From 651e5e7fcb3462b47c2b6192151f30fb06dd1b37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Wed, 26 Jun 2024 21:17:25 -0400 Subject: [PATCH 1/7] api: disk_io_bus_cache_filesystem MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #599 Signed-off-by: Stéphane Graber --- doc/api-extensions.md | 4 ++++ internal/version/api.go | 1 + 2 files changed, 5 insertions(+) diff --git a/doc/api-extensions.md b/doc/api-extensions.md index 15cd4267cea..e0407d22c9c 100644 --- a/doc/api-extensions.md +++ b/doc/api-extensions.md @@ -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. diff --git a/internal/version/api.go b/internal/version/api.go index d1391aea00c..89240033443 100644 --- a/internal/version/api.go +++ b/internal/version/api.go @@ -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. From 417d94bb7f9e5aff3773dff9becfea20bb0ed940 Mon Sep 17 00:00:00 2001 From: Ruihua Wen Date: Wed, 26 Jun 2024 15:51:51 -0400 Subject: [PATCH 2/7] incusd/device/disk: Extend io.bus option Signed-off-by: Ruihua Wen --- internal/server/device/disk.go | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/internal/server/device/disk.go b/internal/server/device/disk.go index 32af83ef318..7f81c7fd5da 100644 --- a/internal/server/device/disk.go +++ b/internal/server/device/disk.go @@ -329,13 +329,23 @@ func (d *disk) validateConfig(instConf instance.ConfigReader) error { "io.cache": validate.Optional(validate.IsOneOf("none", "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) @@ -1102,8 +1112,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"], @@ -1193,6 +1201,12 @@ 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 + } + if d.config["path"] == "" { return nil, fmt.Errorf(`Missing mount "path" setting`) } @@ -1302,6 +1316,12 @@ 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 + } + f, err := d.localSourceOpen(mount.DevPath) if err != nil { return nil, err From 4e891330626967dd2548219f61df9b12b76f53d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Wed, 26 Jun 2024 22:38:16 -0400 Subject: [PATCH 3/7] incusd/device/disk: Extend io.cache option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- internal/server/device/disk.go | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/internal/server/device/disk.go b/internal/server/device/disk.go index 7f81c7fd5da..5fd8283aa0f 100644 --- a/internal/server/device/disk.go +++ b/internal/server/device/disk.go @@ -320,13 +320,23 @@ 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. @@ -1207,6 +1217,11 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) { 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`) } @@ -1322,6 +1337,11 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) { 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 From b4e888c920bc40226094c003ed0ea8efcfc29b01 Mon Sep 17 00:00:00 2001 From: Ruihua Wen Date: Wed, 26 Jun 2024 21:14:29 -0400 Subject: [PATCH 4/7] incusd/device/disk: Add support for io.cache on virtiofs Signed-off-by: Ruihua Wen --- internal/server/device/device_utils_disk.go | 13 +++++++++++-- internal/server/device/disk.go | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/internal/server/device/device_utils_disk.go b/internal/server/device/device_utils_disk.go index e836cf711cb..c38ef451c6f 100644 --- a/internal/server/device/device_utils_disk.go +++ b/internal/server/device/device_utils_disk.go @@ -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() @@ -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 diff --git a/internal/server/device/disk.go b/internal/server/device/disk.go index 5fd8283aa0f..965044ddfd9 100644 --- a/internal/server/device/disk.go +++ b/internal/server/device/disk.go @@ -1262,7 +1262,7 @@ func (d *disk) startVM() (*deviceConfig.RunConfig, error) { 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 { var errUnsupported UnsupportedError if errors.As(err, &errUnsupported) { From 911e5a3c4532dbc4e5d0647c1dd5f4b2f9194016 Mon Sep 17 00:00:00 2001 From: Ruihua Wen Date: Wed, 26 Jun 2024 21:14:41 -0400 Subject: [PATCH 5/7] incusd/device/disk: Add support for io.bus on filesystems Signed-off-by: Ruihua Wen --- internal/server/device/disk.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/internal/server/device/disk.go b/internal/server/device/disk.go index 965044ddfd9..714dca64807 100644 --- a/internal/server/device/disk.go +++ b/internal/server/device/disk.go @@ -1255,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, 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}) @@ -1311,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 From b378d59ebe5fcf68fa5f3e0609d6fe59480d1542 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Wed, 26 Jun 2024 22:54:45 -0400 Subject: [PATCH 6/7] incusd/instance/driver_qemu: Handle 9p being disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- .../server/instance/drivers/driver_qemu.go | 40 ++++++++++--------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/internal/server/instance/drivers/driver_qemu.go b/internal/server/instance/drivers/driver_qemu.go index 41e2c470545..71963bedb09 100644 --- a/internal/server/instance/drivers/driver_qemu.go +++ b/internal/server/instance/drivers/driver_qemu.go @@ -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 } From 0dab18af7cfc1dfdf2b11ecd2328b86832504a5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Graber?= Date: Wed, 26 Jun 2024 15:52:39 -0400 Subject: [PATCH 7/7] doc: Update configs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stéphane Graber --- doc/config_options.txt | 26 ++++++++++++++++++--- internal/server/metadata/configuration.json | 10 ++++---- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/doc/config_options.txt b/doc/config_options.txt index d33b8cf97da..63c522046a5 100644 --- a/doc/config_options.txt +++ b/doc/config_options.txt @@ -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 diff --git a/internal/server/metadata/configuration.json b/internal/server/metadata/configuration.json index febfbc006b5..fc541caa2f6 100644 --- a/internal/server/metadata/configuration.json +++ b/internal/server/metadata/configuration.json @@ -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" } },