Skip to content

Commit

Permalink
tart run: support specifying disk caching mode (#953)
Browse files Browse the repository at this point in the history
  • Loading branch information
edigaryev authored Nov 19, 2024
1 parent b1e88e1 commit 589d489
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 15 deletions.
44 changes: 36 additions & 8 deletions Sources/tart/Commands/Run.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,23 @@ extension VZDiskImageSynchronizationMode {
}
}

extension VZDiskImageCachingMode {
public init?(_ description: String) throws {
switch description {
case "automatic":
self = .automatic
case "cached":
self = .cached
case "uncached":
self = .uncached
case "":
return nil
default:
throw RuntimeError.VMConfigurationError("unsupported disk image caching mode: \"\(description)\"")
}
}
}

struct Run: AsyncParsableCommand {
static var configuration = CommandConfiguration(abstract: "Run a VM")

Expand Down Expand Up @@ -176,7 +193,7 @@ struct Run: AsyncParsableCommand {
@Flag(help: ArgumentHelp("Restrict network access to the host-only network"))
var netHost: Bool = false

@Option(help: ArgumentHelp("Set the root disk options (e.g. --root-disk-opts=\"ro\" or --root-disk-opts=\"sync=none\")",
@Option(help: ArgumentHelp("Set the root disk options (e.g. --root-disk-opts=\"ro\" or --root-disk-opts=\"caching=cached,sync=none\")",
discussion: """
Options are comma-separated and are as follows:
Expand All @@ -187,6 +204,12 @@ struct Run: AsyncParsableCommand {
* sync=fsync — enable data synchronization with the permanent storage, but don't ensure that it was actually written (e.g. --root-disk-opts="sync=fsync")
* sync=full — enable data synchronization with the permanent storage and ensure that it was actually written (e.g. --root-disk-opts="sync=full")
* caching=automatic — allows the virtualization framework to automatically determine whether to enable data caching
* caching=cached — enabled data caching
* caching=uncached — disables data caching
""", valueName: "options"))
var rootDiskOpts: String = ""

Expand Down Expand Up @@ -308,7 +331,8 @@ struct Run: AsyncParsableCommand {
nested: nested,
audio: !noAudio,
clipboard: !noClipboard,
sync: VZDiskImageSynchronizationMode(diskOptions.syncModeRaw)
sync: VZDiskImageSynchronizationMode(diskOptions.syncModeRaw),
caching: VZDiskImageCachingMode(diskOptions.cachingModeRaw)
)

let vncImpl: VNC? = try {
Expand Down Expand Up @@ -761,12 +785,12 @@ struct AdditionalDisk {
let configuration: VZStorageDeviceConfiguration

init(parseFrom: String) throws {
let (diskPath, readOnly, syncModeRaw) = Self.parseOptions(parseFrom)
let (diskPath, readOnly, syncModeRaw, cachingModeRaw) = Self.parseOptions(parseFrom)

self.configuration = try Self.craft(diskPath, readOnly: readOnly, syncModeRaw: syncModeRaw)
self.configuration = try Self.craft(diskPath, readOnly: readOnly, syncModeRaw: syncModeRaw, cachingModeRaw: cachingModeRaw)
}

static func craft(_ diskPath: String, readOnly diskReadOnly: Bool, syncModeRaw: String) throws -> VZStorageDeviceConfiguration {
static func craft(_ diskPath: String, readOnly diskReadOnly: Bool, syncModeRaw: String, cachingModeRaw: String) throws -> VZStorageDeviceConfiguration {
let diskURL = URL(string: diskPath)

if (["nbd", "nbds", "nbd+unix", "nbds+unix"].contains(diskURL?.scheme)) {
Expand Down Expand Up @@ -843,28 +867,29 @@ struct AdditionalDisk {
let diskImageAttachment = try VZDiskImageStorageDeviceAttachment(
url: diskFileURL,
readOnly: diskReadOnly,
cachingMode: .automatic,
cachingMode: try VZDiskImageCachingMode(cachingModeRaw) ?? .automatic,
synchronizationMode: try VZDiskImageSynchronizationMode(syncModeRaw)
)

return VZVirtioBlockDeviceConfiguration(attachment: diskImageAttachment)
}

static func parseOptions(_ parseFrom: String) -> (String, Bool, String) {
static func parseOptions(_ parseFrom: String) -> (String, Bool, String, String) {
var arguments = parseFrom.split(separator: ":")

let options = DiskOptions(String(arguments.last!))
if options.foundAtLeastOneOption {
arguments.removeLast()
}

return (arguments.joined(separator: ":"), options.readOnly, options.syncModeRaw)
return (arguments.joined(separator: ":"), options.readOnly, options.syncModeRaw, options.cachingModeRaw)
}
}

struct DiskOptions {
var readOnly: Bool = false
var syncModeRaw: String = ""
var cachingModeRaw: String = ""
var foundAtLeastOneOption: Bool = false

init(_ parseFrom: String) {
Expand All @@ -878,6 +903,9 @@ struct DiskOptions {
case option.hasPrefix("sync="):
self.syncModeRaw = String(option.dropFirst("sync=".count))
self.foundAtLeastOneOption = true
case option.hasPrefix("caching="):
self.cachingModeRaw = String(option.dropFirst("caching=".count))
self.foundAtLeastOneOption = true
default:
continue
}
Expand Down
22 changes: 15 additions & 7 deletions Sources/tart/VM.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ class VM: NSObject, VZVirtualMachineDelegate, ObservableObject {
nested: Bool = false,
audio: Bool = true,
clipboard: Bool = true,
sync: VZDiskImageSynchronizationMode = .full
sync: VZDiskImageSynchronizationMode = .full,
caching: VZDiskImageCachingMode? = nil
) throws {
name = vmDir.name
config = try VMConfig.init(fromURL: vmDir.configURL)
Expand All @@ -70,7 +71,8 @@ class VM: NSObject, VZVirtualMachineDelegate, ObservableObject {
nested: nested,
audio: audio,
clipboard: clipboard,
sync: sync
sync: sync,
caching: caching
)
virtualMachine = VZVirtualMachine(configuration: configuration)

Expand Down Expand Up @@ -300,7 +302,8 @@ class VM: NSObject, VZVirtualMachineDelegate, ObservableObject {
nested: Bool = false,
audio: Bool = true,
clipboard: Bool = true,
sync: VZDiskImageSynchronizationMode = .full
sync: VZDiskImageSynchronizationMode = .full,
caching: VZDiskImageCachingMode? = nil
) throws -> VZVirtualMachineConfiguration {
let configuration = VZVirtualMachineConfiguration()

Expand Down Expand Up @@ -363,10 +366,15 @@ class VM: NSObject, VZVirtualMachineDelegate, ObservableObject {
}

// Storage
let attachment: VZDiskImageStorageDeviceAttachment = vmConfig.os == .linux ?
// Use "cached" caching mode for virtio drive to prevent fs corruption on linux
try VZDiskImageStorageDeviceAttachment(url: diskURL, readOnly: false, cachingMode: .cached, synchronizationMode: sync) :
try VZDiskImageStorageDeviceAttachment(url: diskURL, readOnly: false, cachingMode: .automatic, synchronizationMode: sync)
let attachment: VZDiskImageStorageDeviceAttachment = try VZDiskImageStorageDeviceAttachment(
url: diskURL,
readOnly: false,
// When not specified, use "cached" caching mode for Linux VMs to prevent file-system corruption[1]
//
// [1]: https://github.com/cirruslabs/tart/pull/675
cachingMode: caching ?? (vmConfig.os == .linux ? .cached : .automatic),
synchronizationMode: sync
)

var devices: [VZStorageDeviceConfiguration] = [VZVirtioBlockDeviceConfiguration(attachment: attachment)]
devices.append(contentsOf: additionalStorageDevices)
Expand Down

0 comments on commit 589d489

Please sign in to comment.