From cf9a3a922100ec264f795d625b808d5896f14f6a Mon Sep 17 00:00:00 2001 From: Fedor Korotkov Date: Wed, 12 Jul 2023 15:08:52 -0400 Subject: [PATCH] Allow mounting a single directory without a name (#555) * 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 --- Sources/tart/Commands/Run.swift | 97 +++++++++++++++-------- Tests/TartTests/DirecotryShareTests.swift | 32 ++++++++ 2 files changed, 95 insertions(+), 34 deletions(-) create mode 100644 Tests/TartTests/DirecotryShareTests.swift diff --git a/Sources/tart/Commands/Run.swift b/Sources/tart/Commands/Run.swift index 4f3f5bf6..dd3a5476 100644 --- a/Sources/tart/Commands/Run.swift +++ b/Sources/tart/Commands/Run.swift @@ -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(""" @@ -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] } @@ -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) + } +} diff --git a/Tests/TartTests/DirecotryShareTests.swift b/Tests/TartTests/DirecotryShareTests.swift new file mode 100644 index 00000000..6c28d7f6 --- /dev/null +++ b/Tests/TartTests/DirecotryShareTests.swift @@ -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) + } +}