Skip to content

Commit

Permalink
Merge pull request #951 from SpiffyEight77/feat/shared-filesystems
Browse files Browse the repository at this point in the history
Add support for io.bus and io.cache for shared filesystems with VMs
  • Loading branch information
stgraber authored Jun 27, 2024
2 parents 37c70e3 + 0dab18a commit 48bf032
Show file tree
Hide file tree
Showing 7 changed files with 132 additions and 37 deletions.
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

0 comments on commit 48bf032

Please sign in to comment.