Skip to content

Commit

Permalink
Allow mounting a single directory without a name (#555)
Browse files Browse the repository at this point in the history
* Allow mounting a single directory without a name

To utilize `VZSingleDirectoryShare` which seems more stable than `VZMultipleDirectoryShare`.

We've been having reports from users that mounted directories occasionally return "no such file" errors when building large projects. I took a stab at reproducing the issue by running https://github.com/devMEremenko/XcodeBenchmark in a mounted directory:

```bash
tart run --dir=workdir-test:~/workspace-temp/XcodeBenchmark ventura-xcode
```

And I was able to reproduce the "no such file" error on the first try! After looking into the issue I decided to try `VZSingleDirectoryShare` as this PR changes and to my pleasant surprise it all worked like a charm the next run. So it seems there is a bug in `VZMultipleDirectoryShare` integration with virtiofs. Since in most cases users only mount a single directory it makes sense to allow doing it wihtout providing a `name`.

So now it will be possible to run the following command:

```bash
tart run --dir=~/workspace-temp/XcodeBenchmark ventura-xcode
```

Which will make `~/workspace-temp/XcodeBenchmark` available under `/Volumes/My Shared Files/` without any intermediate directories.

* Reformat

* Updated description
  • Loading branch information
fkorotkov authored Jul 12, 2023
1 parent ad566fa commit cf9a3a9
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 34 deletions.
97 changes: 63 additions & 34 deletions Sources/tart/Commands/Run.swift
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,17 @@ struct Run: AsyncParsableCommand {
var rosettaTag: String?

@Option(help: ArgumentHelp("""
Additional directory shares with an optional read-only specifier\n(e.g. --dir=\"build:~/src/build\" --dir=\"sources:~/src/sources:ro\")
Additional directory shares with an optional read-only specifier\n(e.g. --dir=\"~/src/build\" or --dir=\"~/src/sources:ro\")
""", discussion: """
Requires host to be macOS 13.0 (Ventura) or newer.
All shared directories are automatically mounted to "/Volumes/My Shared Files" directory on macOS,
A shared directory is automatically mounted to "/Volumes/My Shared Files" directory on macOS,
while on Linux you have to do it manually: "mount -t virtiofs com.apple.virtio-fs.automount /mount/point".
For macOS guests, they must be running macOS 13.0 (Ventura) or newer.
""", valueName: "name:path[:ro]"))
In case of passing multiple directories it is required to prefix them with names e.g. --dir=\"build:~/src/build\" --dir=\"sources:~/src/sources:ro\"
These names will be used as directory names under the mounting point inside guests. For the example above it will be
"/Volumes/My Shared Files/build" and "/Volumes/My Shared Files/sources" respectively.
""", valueName: "[name:]path[:ro]"))
var dir: [String] = []

@Option(help: ArgumentHelp("""
Expand Down Expand Up @@ -390,46 +394,31 @@ struct Run: AsyncParsableCommand {
throw UnsupportedOSError("directory sharing", "is")
}

struct DirectoryShare {
let name: String
let path: URL
let readOnly: Bool
}

var directoryShares: [DirectoryShare] = []

var allNamedShares = true
for rawDir in dir {
let splits = rawDir.split(maxSplits: 2) { $0 == ":" }

if splits.count < 2 {
throw ValidationError("invalid --dir syntax: should at least include name and path, colon-separated")
let directoryShare = try DirectoryShare(parseFrom: rawDir)
if (directoryShare.name == nil) {
allNamedShares = false
}

var readOnly: Bool = false

if splits.count == 3 {
if splits[2] == "ro" {
readOnly = true
} else {
throw ValidationError("invalid --dir syntax: optional read-only specifier can only be \"ro\"")
}
}

let (name, path) = (String(splits[0]), String(splits[1]))

directoryShares.append(DirectoryShare(
name: name,
path: URL(fileURLWithPath: NSString(string: path).expandingTildeInPath),
readOnly: readOnly)
)
directoryShares.append(directoryShare)
}

var directories: [String : VZSharedDirectory] = Dictionary()
directoryShares.forEach { directories[$0.name] = VZSharedDirectory(url: $0.path, readOnly: $0.readOnly) }

let automountTag = VZVirtioFileSystemDeviceConfiguration.macOSGuestAutomountTag
let sharingDevice = VZVirtioFileSystemDeviceConfiguration(tag: automountTag)
sharingDevice.share = VZMultipleDirectoryShare(directories: directories)
if allNamedShares {
var directories: [String : VZSharedDirectory] = Dictionary()
directoryShares.forEach { directories[$0.name!] = VZSharedDirectory(url: $0.path, readOnly: $0.readOnly) }
sharingDevice.share = VZMultipleDirectoryShare(directories: directories)
} else if dir.count > 1 {
throw ValidationError("invalid --dir syntax: for multiple directory shares each one of them should be named")
} else if dir.count == 1 {
let directoryShare = directoryShares.first!
let singleDirectoryShare = VZSingleDirectoryShare(directory: VZSharedDirectory(url: directoryShare.path, readOnly: directoryShare.readOnly))
sharingDevice.share = singleDirectoryShare
}

return [sharingDevice]
}
Expand Down Expand Up @@ -595,3 +584,43 @@ struct VMView: NSViewRepresentable {
nsView.virtualMachine = vm.virtualMachine
}
}

struct DirectoryShare {
let name: String?
let path: URL
let readOnly: Bool

init(parseFrom: String) throws {
let splits = parseFrom.split(maxSplits: 2) { $0 == ":" }

if splits.count == 3 {
if splits[2] == "ro" {
readOnly = true
} else {
throw ValidationError("invalid --dir syntax: optional read-only specifier can only be \"ro\"")
}
name = String(splits[0])
path = String(splits[1]).toFilePathURL()
} else if splits.count == 2 {
if splits[1] == "ro" {
name = nil
path = String(splits[0]).toFilePathURL()
readOnly = true
} else {
name = String(splits[0])
path = String(splits[1]).toFilePathURL()
readOnly = false
}
} else {
name = nil
path = String(splits[0]).toFilePathURL()
readOnly = false
}
}
}

extension String {
func toFilePathURL() -> URL {
URL(fileURLWithPath: NSString(string: self).expandingTildeInPath)
}
}
32 changes: 32 additions & 0 deletions Tests/TartTests/DirecotryShareTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import XCTest
@testable import tart

final class DirectoryShareTests: XCTestCase {
func testNamedParsing() throws {
let share = try DirectoryShare(parseFrom: "build:/Users/admin/build")
XCTAssertEqual(share.name, "build")
XCTAssertEqual(share.path, URL(filePath: "/Users/admin/build"))
XCTAssertFalse(share.readOnly)
}

func testNamedReadOnlyParsing() throws {
let share = try DirectoryShare(parseFrom: "build:/Users/admin/build:ro")
XCTAssertEqual(share.name, "build")
XCTAssertEqual(share.path, URL(filePath: "/Users/admin/build"))
XCTAssertTrue(share.readOnly)
}

func testOptionalNameParsing() throws {
let share = try DirectoryShare(parseFrom: "/Users/admin/build")
XCTAssertNil(share.name)
XCTAssertEqual(share.path, URL(filePath: "/Users/admin/build"))
XCTAssertFalse(share.readOnly)
}

func testOptionalNameReadOnlyParsing() throws {
let share = try DirectoryShare(parseFrom: "/Users/admin/build:ro")
XCTAssertNil(share.name)
XCTAssertEqual(share.path, URL(filePath: "/Users/admin/build"))
XCTAssertTrue(share.readOnly)
}
}

0 comments on commit cf9a3a9

Please sign in to comment.