Skip to content

cache sync options #447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cfergeau wants to merge 4 commits into
base: main
Choose a base branch
from
Open

cache sync options #447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6ac25da
Add virtio-blk cache and sync mode configuration options
cfergeau Mar 25, 2026
51edc9d
Extend cache and sync mode options to NVMe and USB mass storage
cfergeau Mar 25, 2026
13e1760
Add validation to prevent cache/sync options on block devices
cfergeau Mar 25, 2026
22cd5ad
Add sync mode support for block devices
cfergeau Mar 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 84 additions & 3 deletions doc/usage.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,19 @@ See https://cloudinit.readthedocs.io/en/latest/reference/datasources/nocloud.htm
- `path`: the absolute path to the disk image file or block device.
- `type`: the backing type. Use `image` (default) for a disk image file, or `dev` to attach a host block device (for example, /dev/disk1 or /dev/disk1s1). Attaching a block device may require root privileges; use with care.
- `deviceId`: `/dev/disk/by-id/` identifier to use for this device.
- `cache`: disk image caching mode (only for `type=image`, not supported for block devices). Valid values:
- `automatic` (default): allows the virtualization framework to automatically determine whether to enable data caching
- `cached`: enables data caching
- `uncached`: disables data caching
- `sync`: synchronization mode. Valid values differ by backing type:
- For disk images (`type=image`):
- `full`: synchronizes data to the permanent storage
- `fsync` (default): synchronizes data using the system's best-effort synchronization mode
- `none`: disables data synchronization
- For block devices (`type=dev`):
- `full` (default): synchronizes data to the permanent storage
- `none`: disables data synchronization
- Note: `fsync` is not supported for block devices
Comment on lines +213 to +225
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

The documented cache default does not match the current implementation.

These sections say image-backed disks default to automatic, but pkg/vf/virtio.go currently maps an omitted cache mode to cached. Either document cached as the current backward-compatible default, or initialize CachingModeAutomatic when the option is omitted so the runtime behavior matches the docs.

Also applies to: 274-286, 316-328

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@doc/usage.md` around lines 213 - 225, The docs claim image-backed disks
default to CachingModeAutomatic but the runtime maps an omitted cache option to
cached; update pkg/vf/virtio.go so the cache-parsing/initialization code (the
function that maps disk options to CachingMode values — where omitted values are
currently set to CachingModeCached) instead defaults to CachingModeAutomatic for
type=image, leaving block-device defaults unchanged; ensure the symbol
CachingModeAutomatic is used for the omitted-case path and add/update any tests
or validation that assert the new default behavior.


#### Example

Expand All @@ -223,12 +236,27 @@ Attach a host block device instead (may require root privileges):
--device virtio-blk,path=/dev/disk2,type=dev
```

Block device with sync mode disabled for maximum performance:
```
--device virtio-blk,path=/dev/disk2,type=dev,sync=none
```

For ephemeral VMs where data persistence is not critical (maximize performance):
```
--device virtio-blk,path=/Users/virtuser/vfkit.img,cache=cached,sync=none
```

For database or critical workloads (maximize data safety):
```
--device virtio-blk,path=/Users/virtuser/vfkit.img,cache=uncached,sync=full
```
Comment on lines +239 to +252
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a language to the new fenced examples.

These blocks are the ones currently tripping markdownlint MD040. Marking them as bash will keep the docs lint-clean.

Also applies to: 295-302, 337-349

🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 240-240: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

[warning] 245-245: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

[warning] 250-250: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@doc/usage.md` around lines 239 - 252, The three fenced code blocks showing
example --device commands (e.g., "--device
virtio-blk,path=/dev/disk2,type=dev,sync=none", "--device
virtio-blk,path=/Users/virtuser/vfkit.img,cache=cached,sync=none", and "--device
virtio-blk,path=/Users/virtuser/vfkit.img,cache=uncached,sync=full") should
include a language tag to satisfy markdownlint MD040; update each opening
triple-backtick to "```bash" in the doc/usage.md examples at the shown ranges
and also add "```bash" to the other affected blocks referenced (around lines
295-302 and 337-349) so all three fenced blocks are explicitly marked as bash.


To also provide the cloud-init configuration you can add an additional virtio-blk device backed by an image containing the cloud-init configuration files
```
--device virtio-blk,path=/Users/virtuser/cloudinit.img
```

If you prefer to use the automatic ISO creation
If you prefer to use the automatic ISO creation
```
--cloud-init /Users/virtuser/user-data,/Users/virtuser/meta-data
```
Expand All @@ -241,7 +269,21 @@ If you prefer to use the automatic ISO creation
The `--device nvme` option adds a NVMe device to the virtual machine. The disk is backed by an image file on the host machine. This file is a raw image file.

#### Arguments
- `path`: the absolute path to the disk image file.
- `path`: the absolute path to the disk image file or block device.
- `type`: the backing type. Use `image` (default) for a disk image file, or `dev` to attach a host block device. Attaching a block device may require root privileges; use with care.
Comment on lines +272 to +273
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update the NVMe and USB prose to mention block-device backends.

These new path/type entries document type=dev, but the Description text above both sections still says those devices are backed by image files only. The docs now contradict themselves on whether block devices are supported.

Also applies to: 313-314

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@doc/usage.md` around lines 272 - 273, The NVMe and USB device description
text currently states devices are backed by image files only, which contradicts
the new `path`/`type` entries that document `type=dev`; update the prose in the
NVMe and USB sections to mention that backends can be either disk image files or
host block devices and include a short note about `type=dev` (and potential root
privileges) to match the `path`/`type` bullets and avoid the contradiction
(refer to the `path` and `type` fields and the `type=dev` example when editing
those section descriptions).

- `cache`: disk image caching mode (only for `type=image`, not supported for block devices). Valid values:
- `automatic` (default): allows the virtualization framework to automatically determine whether to enable data caching
- `cached`: enables data caching
- `uncached`: disables data caching
- `sync`: synchronization mode. Valid values differ by backing type:
- For disk images (`type=image`):
- `full`: synchronizes data to the permanent storage
- `fsync` (default): synchronizes data using the system's best-effort synchronization mode
- `none`: disables data synchronization
- For block devices (`type=dev`):
- `full` (default): synchronizes data to the permanent storage
- `none`: disables data synchronization
- Note: `fsync` is not supported for block devices

#### Example

Expand All @@ -250,6 +292,16 @@ This adds a NVMe device to the VM which will be backed by the disk image at `/Us
--device nvme,path=/Users/virtuser/image.img
```

For high-performance ephemeral storage:
```
--device nvme,path=/Users/virtuser/image.img,cache=cached,sync=none
```

For database or critical workloads:
```
--device nvme,path=/Users/virtuser/image.img,cache=uncached,sync=full
```


### USB Mass Storage

Expand All @@ -258,8 +310,22 @@ This adds a NVMe device to the VM which will be backed by the disk image at `/Us
The `--device usb-mass-storage` option adds a USB mass storage device to the virtual machine. The disk is backed by an image file on the host machine. This file is a raw image file or an ISO image.

#### Arguments
- `path`: the absolute path to the disk image file.
- `path`: the absolute path to the disk image file or block device.
- `type`: the backing type. Use `image` (default) for a disk image file, or `dev` to attach a host block device. Attaching a block device may require root privileges; use with care.
- `readonly`: if specified the device will be read only.
- `cache`: disk image caching mode (only for `type=image`, not supported for block devices). Valid values:
- `automatic` (default): allows the virtualization framework to automatically determine whether to enable data caching
- `cached`: enables data caching
- `uncached`: disables data caching
- `sync`: synchronization mode. Valid values differ by backing type:
- For disk images (`type=image`):
- `full`: synchronizes data to the permanent storage
- `fsync` (default): synchronizes data using the system's best-effort synchronization mode
- `none`: disables data synchronization
- For block devices (`type=dev`):
- `full` (default): synchronizes data to the permanent storage
- `none`: disables data synchronization
- Note: `fsync` is not supported for block devices

#### Example

Expand All @@ -268,6 +334,21 @@ This adds a read only USB mass storage device to the VM which will be backed by
--device usb-mass-storage,path=/Users/virtuser/distro.iso,readonly
```

For a read-only device with caching enabled:
```
--device usb-mass-storage,path=/Users/virtuser/distro.iso,readonly,cache=cached
```

For a writable device with full data safety:
```
--device usb-mass-storage,path=/Users/virtuser/data.img,cache=uncached,sync=full
```

When using a block device, you can specify the sync mode (cache is not supported):
```
--device usb-mass-storage,path=/dev/disk2,type=dev,sync=none
```

### Network Block Device

#### Description
Expand Down
6 changes: 3 additions & 3 deletions pkg/config/json_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ var jsonStabilityTests = map[string]jsonStabilityTest{
return blk
},

skipFields: []string{"DevName", "URI", "Type"},
skipFields: []string{"DevName", "URI", "Type", "CachingMode", "SynchronizationMode"},
expectedJSON: `{"kind":"virtioblk","devName":"virtio-blk","imagePath":"ImagePath","readOnly":true,"type":"image","deviceIdentifier":"DeviceIdentifier"}`,
},
"USBMassStorage": {
Expand All @@ -291,7 +291,7 @@ var jsonStabilityTests = map[string]jsonStabilityTest{
usb.Type = DiskBackendImage
return usb
},
skipFields: []string{"DevName", "URI", "Type"},
skipFields: []string{"DevName", "URI", "Type", "CachingMode", "SynchronizationMode"},
expectedJSON: `{"kind":"usbmassstorage","devName":"usb-mass-storage","imagePath":"ImagePath","readOnly":true,"type":"image"}`,
},
"NVMExpressController": {
Expand All @@ -301,7 +301,7 @@ var jsonStabilityTests = map[string]jsonStabilityTest{
nvme.Type = DiskBackendImage
return nvme
},
skipFields: []string{"DevName", "URI", "Type"},
skipFields: []string{"DevName", "URI", "Type", "CachingMode", "SynchronizationMode"},
expectedJSON: `{"kind":"nvme","devName":"nvme","imagePath":"ImagePath","readOnly":true,"type":"image"}`,
},
"LinuxBootloader": {
Expand Down
98 changes: 96 additions & 2 deletions pkg/config/virtio.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,17 @@ func (dev *VirtioBlk) ToCmdLine() ([]string, error) {
}

func (dev *VirtioBlk) validate() error {
// First validate the disk storage config (cache/sync mode constraints)
if err := dev.DiskStorageConfig.validate(); err != nil {
return err
}

// Skip qcow2 check for block devices (they can't be qcow2 format)
if dev.Type == DiskBackendBlockDevice {
return nil
}
Comment on lines +648 to +656
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep the block-device fast path from skipping required-path validation.

This early return means virtio-blk,type=dev now parses successfully even when no path was provided, and the error is deferred until much later. Skipping the qcow2 probe for block devices makes sense, but the mandatory path check should still happen before returning.

One small way to preserve the old validation behavior 
 func (dev *VirtioBlk) validate() error {
 	// First validate the disk storage config (cache/sync mode constraints)
 	if err := dev.DiskStorageConfig.validate(); err != nil {
 		return err
 	}
+	if dev.ImagePath == "" {
+		return fmt.Errorf("virtio-blk devices need the path to a disk image or block device")
+	}
 
 	// Skip qcow2 check for block devices (they can't be qcow2 format)
 	if dev.Type == DiskBackendBlockDevice {
 		return nil
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// First validate the disk storage config (cache/sync mode constraints)
if err := dev.DiskStorageConfig.validate(); err != nil {
return err
}
// Skip qcow2 check for block devices (they can't be qcow2 format)
if dev.Type == DiskBackendBlockDevice {
return nil
}
// First validate the disk storage config (cache/sync mode constraints)
if err := dev.DiskStorageConfig.validate(); err != nil {
return err
}
if dev.ImagePath == "" {
return fmt.Errorf("virtio-blk devices need the path to a disk image or block device")
}
// Skip qcow2 check for block devices (they can't be qcow2 format)
if dev.Type == DiskBackendBlockDevice {
return nil
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/config/virtio.go` around lines 648 - 656, The early return for block
devices (check against DiskBackendBlockDevice) skips required-path validation
and allows virtio-blk,type=dev to pass without a path; before returning for
block devices, ensure the mandatory path is validated (e.g., reuse or call
whatever path/required-fields check used elsewhere for Disk config, or
explicitly validate dev.Path) so the missing path is rejected early; keep the
qcow2 probe skipped but perform the path validation prior to the return in the
same block where dev.DiskStorageConfig.validate() and DiskBackendBlockDevice are
checked.


// Then check for qcow2 images (only for disk image files)
imgPath := dev.ImagePath
file, err := os.Open(imgPath)
if err != nil {
Expand Down Expand Up @@ -930,10 +941,57 @@ func (typ DiskBackendType) IsValid() bool {
}
}

// DiskImageCachingMode describes the disk image caching mode.
// See: https://developer.apple.com/documentation/virtualization/vzdiskimagecachingmode
type DiskImageCachingMode string

const (
// CachingModeAutomatic allows the virtualization framework to automatically
// determine whether to enable data caching.
CachingModeAutomatic DiskImageCachingMode = "automatic"
// CachingModeCached enables data caching.
CachingModeCached DiskImageCachingMode = "cached"
// CachingModeUncached disables data caching.
CachingModeUncached DiskImageCachingMode = "uncached"
)

func (mode DiskImageCachingMode) IsValid() bool {
switch mode {
case CachingModeAutomatic, CachingModeCached, CachingModeUncached, "":
return true
default:
return false
}
}

// DiskImageSynchronizationMode describes the disk image synchronization mode.
// See: https://developer.apple.com/documentation/virtualization/vzdiskimagesynchronizationmode
type DiskImageSynchronizationMode string

const (
// SyncModeFull synchronizes data to the permanent storage holding the disk image.
SyncModeFull DiskImageSynchronizationMode = "full"
// SyncModeFsync synchronizes data to the drive using the system's best-effort synchronization mode.
SyncModeFsync DiskImageSynchronizationMode = "fsync"
// SyncModeNone disables data synchronization with the permanent storage.
SyncModeNone DiskImageSynchronizationMode = "none"
)

func (mode DiskImageSynchronizationMode) IsValid() bool {
switch mode {
case SyncModeFull, SyncModeFsync, SyncModeNone, "":
return true
default:
return false
}
}

type DiskStorageConfig struct {
StorageConfig
ImagePath string `json:"imagePath,omitempty"`
Type DiskBackendType `json:"type,omitempty"`
ImagePath string `json:"imagePath,omitempty"`
Type DiskBackendType `json:"type,omitempty"`
CachingMode DiskImageCachingMode `json:"cachingMode,omitempty"`
SynchronizationMode DiskImageSynchronizationMode `json:"synchronizationMode,omitempty"`
}

type NetworkBlockStorageConfig struct {
Expand All @@ -955,6 +1013,15 @@ func (config *DiskStorageConfig) ToCmdLine() ([]string, error) {
if config.ReadOnly {
value += ",readonly"
}

if config.CachingMode != "" {
value += fmt.Sprintf(",cache=%s", string(config.CachingMode))
}

if config.SynchronizationMode != "" {
value += fmt.Sprintf(",sync=%s", string(config.SynchronizationMode))
}

return []string{"--device", value}, nil
}

Expand All @@ -974,10 +1041,37 @@ func (config *DiskStorageConfig) FromOptions(options []option) error {
return fmt.Errorf("unexpected value for virtio-blk 'readonly' option: %s", option.value)
}
config.ReadOnly = true
case "cache":
mode := DiskImageCachingMode(option.value)
if !mode.IsValid() {
return fmt.Errorf("unexpected value for disk 'cache' option: %s (valid values: automatic, cached, uncached)", option.value)
}
config.CachingMode = mode
case "sync":
mode := DiskImageSynchronizationMode(option.value)
if !mode.IsValid() {
return fmt.Errorf("unexpected value for disk 'sync' option: %s (valid values: full, fsync, none)", option.value)
}
config.SynchronizationMode = mode
Comment on lines +1044 to +1055
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reject explicit empty cache / sync values.

Because IsValid() accepts "", inputs like cache= and sync= are parsed as if the option was omitted. An empty shell variable can therefore silently change the storage mode instead of failing fast.

Minimal fix 
 		case "cache":
+			if option.value == "" {
+				return fmt.Errorf("disk 'cache' option requires a value")
+			}
 			mode := DiskImageCachingMode(option.value)
 			if !mode.IsValid() {
 				return fmt.Errorf("unexpected value for disk 'cache' option: %s (valid values: automatic, cached, uncached)", option.value)
 			}
 			config.CachingMode = mode
 		case "sync":
+			if option.value == "" {
+				return fmt.Errorf("disk 'sync' option requires a value")
+			}
 			mode := DiskImageSynchronizationMode(option.value)
 			if !mode.IsValid() {
 				return fmt.Errorf("unexpected value for disk 'sync' option: %s (valid values: full, fsync, none)", option.value)
 			}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
case "cache":
mode := DiskImageCachingMode(option.value)
if !mode.IsValid() {
return fmt.Errorf("unexpected value for disk 'cache' option: %s (valid values: automatic, cached, uncached)", option.value)
}
config.CachingMode = mode
case "sync":
mode := DiskImageSynchronizationMode(option.value)
if !mode.IsValid() {
return fmt.Errorf("unexpected value for disk 'sync' option: %s (valid values: full, fsync, none)", option.value)
}
config.SynchronizationMode = mode
case "cache":
if option.value == "" {
return fmt.Errorf("disk 'cache' option requires a value")
}
mode := DiskImageCachingMode(option.value)
if !mode.IsValid() {
return fmt.Errorf("unexpected value for disk 'cache' option: %s (valid values: automatic, cached, uncached)", option.value)
}
config.CachingMode = mode
case "sync":
if option.value == "" {
return fmt.Errorf("disk 'sync' option requires a value")
}
mode := DiskImageSynchronizationMode(option.value)
if !mode.IsValid() {
return fmt.Errorf("unexpected value for disk 'sync' option: %s (valid values: full, fsync, none)", option.value)
}
config.SynchronizationMode = mode
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/config/virtio.go` around lines 1044 - 1055, The parsing for disk options
"cache" and "sync" currently allows empty strings because DiskImageCachingMode/
DiskImageSynchronizationMode .IsValid() treats "" as valid; update the switch
arms that handle option.value (the "cache" and "sync" cases) to explicitly
reject an empty option.value (e.g., if option.value == "" return an error
stating empty value is not allowed for disk 'cache' or 'sync') before converting
with DiskImageCachingMode/DiskImageSynchronizationMode and assigning to
config.CachingMode/config.SynchronizationMode so empty assignments like "cache="
or "sync=" fail fast.

default:
return fmt.Errorf("unknown option for %s devices: %s", config.DevName, option.key)
}
}
return config.validate()
}

func (config *DiskStorageConfig) validate() error {
// Validate options for block devices (type=dev)
if config.Type == DiskBackendBlockDevice {
// Cache mode is not supported for block devices
if config.CachingMode != "" {
return fmt.Errorf("cache mode is not supported for block devices (type=dev)")
}
// Block devices only support 'full' and 'none' sync modes, not 'fsync'
if config.SynchronizationMode == SyncModeFsync {
return fmt.Errorf("sync mode 'fsync' is not supported for block devices (type=dev), use 'full' or 'none'")
}
}
return nil
}

Expand Down
Loading
Loading