Skip to content
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.

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

Add support for io.bus and io.cache for shared filesystems with VMs #951

Merged
merged 7 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions doc/api-extensions.md
Original file line number Diff line number Diff line change
Expand Up @@ -2513,3 +2513,7 @@ delete everything inside of the project along with the project itself.
## `resources_cpu_flags`

This exposes the CPU flags/extensions in our resources API to check the CPU features.

## `disk_io_bus_cache_filesystem`

This adds support for both `io.bus` and `io.cache` to disks that are backed by a file system.
26 changes: 23 additions & 3 deletions doc/config_options.txt
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,39 @@ User keys can be used in search.
```

```{config:option} io.bus devices-disk
:default: "`virtio-scsi`"
:default: "`virtio-scsi` for block, `auto` for file system"
:required: "no"
:shortdesc: "Only for VMs: Override the bus for the device (`nvme`, `virtio-blk`, or `virtio-scsi`)"
:shortdesc: "Only for VMs: Override the bus for the device"
:type: "string"
This controls what bus a disk device should be attached to.

For block devices (disks), this is one of:
- `nvme`
- `virtio-blk`
- `virtio-scsi` (default)

For file systems (shared directories or custom volumes), this is one of:
- `9p`
- `auto` (default) (`virtiofs` + `9p`, just `9p` if `virtiofsd` is missing)
- `virtiofs`
```

```{config:option} io.cache devices-disk
:default: "`none`"
:required: "no"
:shortdesc: "Only for VMs: Override the caching mode for the device (`none`, `writeback` or `unsafe`)"
:shortdesc: "Only for VMs: Override the caching mode for the device"
:type: "string"
This controls what bus a disk device should be attached to.

For block devices (disks), this is one of:
- `none` (default)
- `writeback`
- `unsafe`

For file systems (shared directories or custom volumes), this is one of:
- `none` (default)
- `metadata`
- `unsafe`
```

```{config:option} limits.max devices-disk
Expand Down
13 changes: 11 additions & 2 deletions internal/server/device/device_utils_disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ func DiskVMVirtfsProxyStop(pidPath string) error {
// Returns UnsupportedError error if the host system or instance does not support virtiosfd, returns normal error
// type if process cannot be started for other reasons.
// Returns revert function and listener file handle on success.
func DiskVMVirtiofsdStart(execPath string, inst instance.Instance, socketPath string, pidPath string, logPath string, sharePath string, idmaps []idmap.Entry) (func(), net.Listener, error) {
func DiskVMVirtiofsdStart(execPath string, inst instance.Instance, socketPath string, pidPath string, logPath string, sharePath string, idmaps []idmap.Entry, cacheOption string) (func(), net.Listener, error) {
revert := revert.New()
defer revert.Fail()

Expand Down Expand Up @@ -499,8 +499,17 @@ func DiskVMVirtiofsdStart(execPath string, inst instance.Instance, socketPath st

defer func() { _ = unixFile.Close() }()

switch cacheOption {
case "metadata":
cacheOption = "metadata"
case "unsafe":
cacheOption = "always"
default:
cacheOption = "never"
}

// Start the virtiofsd process in non-daemon mode.
args := []string{"--fd=3", "--cache=never", "-o", fmt.Sprintf("source=%s", sharePath)}
args := []string{"--fd=3", fmt.Sprintf("--cache=%s", cacheOption), "-o", fmt.Sprintf("source=%s", sharePath)}
proc, err := subprocess.NewProcess(cmd, args, logPath, logPath)
if err != nil {
return nil, nil, err
Expand Down
75 changes: 67 additions & 8 deletions internal/server/device/disk.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,22 +320,42 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error {
"path": validate.IsAny,

// gendoc:generate(entity=devices, group=disk, key=io.cache)
// This controls what bus a disk device should be attached to.
//
// For block devices (disks), this is one of:
// - `none` (default)
// - `writeback`
// - `unsafe`
//
// For file systems (shared directories or custom volumes), this is one of:
// - `none` (default)
// - `metadata`
// - `unsafe`
// ---
// type: string
// default: `none`
// required: no
// shortdesc: Only for VMs: Override the caching mode for the device (`none`, `writeback` or `unsafe`)
"io.cache": validate.Optional(validate.IsOneOf("none", "writeback", "unsafe")),
// shortdesc: Only for VMs: Override the caching mode for the device
"io.cache": validate.Optional(validate.IsOneOf("none", "metadata", "writeback", "unsafe")),

// gendoc:generate(entity=devices, group=disk, key=io.bus)
// This controls what bus a disk device should be attached to.
//
// For block devices (disks), this is one of:
// - `nvme`
// - `virtio-blk`
// - `virtio-scsi` (default)
//
// For file systems (shared directories or custom volumes), this is one of:
// - `9p`
// - `auto` (default) (`virtiofs` + `9p`, just `9p` if `virtiofsd` is missing)
// - `virtiofs`
// ---
// type: string
// default: `virtio-scsi`
// default: `virtio-scsi` for block, `auto` for file system
// required: no
// shortdesc: Only for VMs: Override the bus for the device (`nvme`, `virtio-blk`, or `virtio-scsi`)
"io.bus": validate.Optional(validate.IsOneOf("nvme", "virtio-blk", "virtio-scsi")),
// shortdesc: Only for VMs: Override the bus for the device
"io.bus": validate.Optional(validate.IsOneOf("nvme", "virtio-blk", "virtio-scsi", "auto", "9p", "virtiofs")),
}

err := d.config.Validate(rules)
Expand Down Expand Up @@ -1102,8 +1122,6 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
},
}
} else {
var err error

// Default to block device or image file passthrough first.
mount := deviceConfig.MountEntryItem{
DevPath: d.config["source"],
Expand Down Expand Up @@ -1193,6 +1211,17 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
// directory sharing feature to mount the directory inside the VM, and as such we need to
// indicate to the VM the target path to mount to.
if internalUtil.IsDir(mount.DevPath) || d.sourceIsCephFs() {
// Confirm we're using filesystem options.
err := validate.Optional(validate.IsOneOf("auto", "9p", "virtiofs"))(d.config["io.bus"])
if err != nil {
return nil, err
}

err = validate.Optional(validate.IsOneOf("none", "metadata", "unsafe"))(d.config["io.cache"])
if err != nil {
return nil, err
}

if d.config["path"] == "" {
return nil, fmt.Errorf(`Missing mount "path" setting`)
}
Expand Down Expand Up @@ -1226,15 +1255,29 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
rawIDMaps.Entries = diskAddRootUserNSEntry(rawIDMaps.Entries, 65534)
}

busOption := d.config["io.bus"]
if busOption == "" {
busOption = "auto"
}

// Start virtiofsd for virtio-fs share. The agent prefers to use this over the
// virtfs-proxy-helper 9p share. The 9p share will only be used as a fallback.
err = func() error {
// Check if we should start virtiofsd.
if busOption != "auto" && busOption != "virtiofs" {
return nil
}

sockPath, pidPath := d.vmVirtiofsdPaths()
logPath := filepath.Join(d.inst.LogPath(), fmt.Sprintf("disk.%s.log", d.name))
_ = os.Remove(logPath) // Remove old log if needed.

revertFunc, unixListener, err := DiskVMVirtiofsdStart(d.state.OS.ExecPath, d.inst, sockPath, pidPath, logPath, mount.DevPath, rawIDMaps.Entries)
revertFunc, unixListener, err := DiskVMVirtiofsdStart(d.state.OS.ExecPath, d.inst, sockPath, pidPath, logPath, mount.DevPath, rawIDMaps.Entries, d.config["io.cache"])
if err != nil {
if busOption == "virtiofs" {
return err
}

var errUnsupported UnsupportedError
if errors.As(err, &errUnsupported) {
d.logger.Warn("Unable to use virtio-fs for device, using 9p as a fallback", logger.Ctx{"err": errUnsupported})
Expand Down Expand Up @@ -1282,6 +1325,11 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
// Start virtfs-proxy-helper for 9p share (this will rewrite mount.DevPath with
// socket FD number so must come after starting virtiofsd).
err = func() error {
// Check if we should start 9p.
if busOption != "auto" && busOption != "9p" {
return nil
}

sockFile, cleanup, err := DiskVMVirtfsProxyStart(d.state.OS.ExecPath, d.vmVirtfsProxyHelperPaths(), mount.DevPath, rawIDMaps.Entries)
if err != nil {
return err
Expand All @@ -1302,6 +1350,17 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) {
}
}
} else {
// Confirm we're dealing with block options.
err := validate.Optional(validate.IsOneOf("nvme", "virtio-blk", "virtio-scsi"))(d.config["io.bus"])
if err != nil {
return nil, err
}

err = validate.Optional(validate.IsOneOf("none", "writeback", "unsafe"))(d.config["io.cache"])
if err != nil {
return nil, err
}

f, err := d.localSourceOpen(mount.DevPath)
if err != nil {
return nil, err
Expand Down
40 changes: 21 additions & 19 deletions internal/server/instance/drivers/driver_qemu.go
Original file line number Diff line number Diff line change
Expand Up @@ -3679,29 +3679,31 @@ func (d *qemu) addDriveDirConfig(cfg *[]cfgSection, bus *qemuBus, fdFiles *[]*os
}

// Add 9p share config.
devBus, devAddr, multi := bus.allocate(busFunctionGroup9p)
if slices.Contains(driveConf.Opts, "bus=auto") || slices.Contains(driveConf.Opts, "bus=9p") {
devBus, devAddr, multi := bus.allocate(busFunctionGroup9p)

fd, err := strconv.Atoi(driveConf.DevPath)
if err != nil {
return fmt.Errorf("Invalid file descriptor %q for drive %q: %w", driveConf.DevPath, driveConf.DevName, err)
}
fd, err := strconv.Atoi(driveConf.DevPath)
if err != nil {
return fmt.Errorf("Invalid file descriptor %q for drive %q: %w", driveConf.DevPath, driveConf.DevName, err)
}

proxyFD := d.addFileDescriptor(fdFiles, os.NewFile(uintptr(fd), driveConf.DevName))
proxyFD := d.addFileDescriptor(fdFiles, os.NewFile(uintptr(fd), driveConf.DevName))

driveDir9pOpts := qemuDriveDirOpts{
dev: qemuDevOpts{
busName: bus.name,
devBus: devBus,
devAddr: devAddr,
multifunction: multi,
},
devName: driveConf.DevName,
mountTag: mountTag,
proxyFD: proxyFD, // Pass by file descriptor
readonly: readonly,
protocol: "9p",
driveDir9pOpts := qemuDriveDirOpts{
dev: qemuDevOpts{
busName: bus.name,
devBus: devBus,
devAddr: devAddr,
multifunction: multi,
},
devName: driveConf.DevName,
mountTag: mountTag,
proxyFD: proxyFD, // Pass by file descriptor
readonly: readonly,
protocol: "9p",
}
*cfg = append(*cfg, qemuDriveDir(&driveDir9pOpts)...)
}
*cfg = append(*cfg, qemuDriveDir(&driveDir9pOpts)...)

return nil
}
Expand Down
10 changes: 5 additions & 5 deletions internal/server/metadata/configuration.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,19 @@
},
{
"io.bus": {
"default": "`virtio-scsi`",
"longdesc": "",
"default": "`virtio-scsi` for block, `auto` for file system",
"longdesc": "This controls what bus a disk device should be attached to.\n\nFor block devices (disks), this is one of:\n- `nvme`\n- `virtio-blk`\n- `virtio-scsi` (default)\n\nFor file systems (shared directories or custom volumes), this is one of:\n- `9p`\n- `auto` (default) (`virtiofs` + `9p`, just `9p` if `virtiofsd` is missing)\n- `virtiofs`",
"required": "no",
"shortdesc": "Only for VMs: Override the bus for the device (`nvme`, `virtio-blk`, or `virtio-scsi`)",
"shortdesc": "Only for VMs: Override the bus for the device",
"type": "string"
}
},
{
"io.cache": {
"default": "`none`",
"longdesc": "",
"longdesc": "This controls what bus a disk device should be attached to.\n\nFor block devices (disks), this is one of:\n- `none` (default)\n- `writeback`\n- `unsafe`\n\nFor file systems (shared directories or custom volumes), this is one of:\n- `none` (default)\n- `metadata`\n- `unsafe`",
"required": "no",
"shortdesc": "Only for VMs: Override the caching mode for the device (`none`, `writeback` or `unsafe`)",
"shortdesc": "Only for VMs: Override the caching mode for the device",
"type": "string"
}
},
Expand Down
1 change: 1 addition & 0 deletions internal/version/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,7 @@ var APIExtensions = []string{
"project_access",
"projects_force_delete",
"resources_cpu_flags",
"disk_io_bus_cache_filesystem",
}

// APIExtensionsCount returns the number of available API extensions.
Expand Down
Loading