From a58f31afce0815096bc9079c45dc4b983c95dfca Mon Sep 17 00:00:00 2001 From: Anastassios Nanos Date: Fri, 9 Oct 2026 14:30:24 +0300 Subject: [PATCH] feat(darwin): Pass an ordered list of disks to the VMM An overlay root on a block device needs two disks: a read-only lower built from the image and a writable upper per instance. ExecArgs had a single BlockDevPath, so the darwin builders could attach one disk only. On vz that disk also replaced the root share. ExecArgs gains BlockDevs, a list of BlockDevSpec{Path, ReadOnly} in command-line order. The VMM gives each disk the serial disk by its position, and the guest finds its disks by serial. BlockDevPath stays as the single-disk form. It counts as one writable disk when BlockDevs is empty. EffectiveBlockDevs returns either form as a list. Only the darwin builders read BlockDevs; the Linux hvi and the QEMU builders still read BlockDevPath. - hvi: emit --disk or --disk-ro per entry, in order. BlockDevPath alone still gives one --disk. - vz: BlockDevPath alone still goes out as --rootfs and replaces the root share. Entries in BlockDevs go out as --disk or --disk-ro after the root share. The root share stays on the command line, both the Sharedfs one and the RootfsPath one. - Both builders reject an entry with no path and a path listed twice, as they do for shares. A missing or repeated disk would shift the serials of the disks after it. - docs/vz-runner-contract.md: document --disk, --disk-ro and the disk serial, counted over those two flags only. Add the missing --share-ro row. The Linux hvi builder is unchanged. TestHviDarwinDisks and TestVzDarwinDisks cover the legacy path and a read-only lower followed by a writable upper. The *DisksRejected tests cover the empty and duplicate path errors on both builders. TestVzDarwinDisksKeepRootShare and TestVzDarwinDisksKeepRootfsPathShare check that BlockDevs keep the root share on vz, and that BlockDevPath still drops it. The BlockDevs cases fail against the previous builders. Signed-off-by: Anastassios Nanos --- docs/vz-runner-contract.md | 32 +++- pkg/unikontainers/hypervisors/hvi_darwin.go | 31 +++- .../hypervisors/hvi_darwin_test.go | 125 ++++++++++++++ pkg/unikontainers/hypervisors/vz_darwin.go | 20 ++- .../hypervisors/vz_darwin_test.go | 155 ++++++++++++++++++ pkg/unikontainers/types/types.go | 26 +++ 6 files changed, 382 insertions(+), 7 deletions(-) diff --git a/docs/vz-runner-contract.md b/docs/vz-runner-contract.md index 60fcd2cd1..9dfc471df 100644 --- a/docs/vz-runner-contract.md +++ b/docs/vz-runner-contract.md @@ -24,12 +24,42 @@ vz-runner --kernel [options] | `--cpus ` | yes | Number of vCPUs | | `--initrd ` | no | Initial ramdisk | | `--cmdline ` | no | Kernel command line | -| `--rootfs ` | no | Disk image attached as a virtio block device (`/dev/vda`) | +| `--rootfs ` | no | Disk image attached as a virtio block device (`/dev/vda`). The legacy single root disk; it carries no serial | +| `--disk ` | no, repeatable | Disk image attached read-write as a virtio block device, after the `--rootfs` disk if any. See [Disks and serials](#disks-and-serials) | +| `--disk-ro ` | no, repeatable | As `--disk`, attached read-only (`VZDiskImageStorageDeviceAttachment(readOnly: true)`) | | `--share ` | no, repeatable | Host directory exported over virtiofs with the given tag. The backend uses tag `rootfs` for the root filesystem and `shared` for user-shared directories | +| `--share-ro ` | no, repeatable | As `--share`, exported read-only. The mode is carried by the flag, so the arguments keep the same shape | | `--mac
` | no | MAC address for the NAT network device (colon-separated hex). When omitted the runner may use a random address; the backend passes a deterministic one so the guest's DHCP lease can be located on the host by MAC | | `--net-fd ` | no | Inherited file descriptor of a connected datagram socket; the runner backs the guest's single virtio-net device with it (`VZFileHandleNetworkDeviceAttachment`) **instead of** the NAT attachment. Used by orchestration layers to place guests on a user-mode gateway network, since the NAT attachment isolates guests from each other. The guest still sees exactly one interface | | `--qmp ` | no | Unix socket path on which the runner serves a minimal QMP endpoint (used for graceful shutdown) | +## Disks and serials + +`--disk` and `--disk-ro` may be given any number of times, in any +interleaving. The runner appends them to `storageDevices` in command-line +order, after the `--rootfs` disk (or the EFI boot disk) if there is one, and +sets each one's `blockDeviceIdentifier` to `disk`. N is the 0-based +position of the disk among the `--disk` and `--disk-ro` flags only; the +`--rootfs` disk is not counted and gets no serial. The guest reads the serial +from `/sys/block/vd*/serial` and never relies on `/dev/vdX` order. Serials are +at most 20 bytes. + +The backend emits `--rootfs` only for the legacy single disk +(`ExecArgs.BlockDevPath` with `ExecArgs.BlockDevs` empty), and that disk +replaces the root share. Disks in `ExecArgs.BlockDevs` go out over +`--disk`/`--disk-ro` in order and leave the root share (`--share` or +`--share-ro` with tag `fs0` unless set) in place. For example, a read-only +lower followed by a writable upper: + +``` +vz-runner ... --share-ro fs0 --disk-ro lower.ext4 --disk upper.ext4 +``` + +gives `lower.ext4` serial `disk0` and `upper.ext4` serial `disk1`. + +When checkpointing, the runner clones only the writable disks and records the +read-only ones by path. + ## Runtime behavior - The guest serial console must be attached to the runner's **stdin/stdout**; diff --git a/pkg/unikontainers/hypervisors/hvi_darwin.go b/pkg/unikontainers/hypervisors/hvi_darwin.go index 42807bb7a..e085686b9 100644 --- a/pkg/unikontainers/hypervisors/hvi_darwin.go +++ b/pkg/unikontainers/hypervisors/hvi_darwin.go @@ -66,9 +66,13 @@ func (h *HviDarwin) BuildExecCmd(args types.ExecArgs, _ types.Unikernel) ([]stri if args.Command != "" { cmd = append(cmd, "--cmdline", args.Command) } - if args.BlockDevPath != "" { - cmd = append(cmd, "--disk", args.BlockDevPath) + // hvi gives each disk the serial disk by its position across --disk + // and --disk-ro, and the guest finds its disks by serial. + disks, err := blockDevArgs("hvi", args.EffectiveBlockDevs()) + if err != nil { + return nil, err } + cmd = append(cmd, disks...) seenTags := make(map[string]bool) appendShare := func(path, tag string, readOnly bool) error { if path == "" || tag == "" { @@ -119,6 +123,29 @@ func (h *HviDarwin) BuildExecCmd(args types.ExecArgs, _ types.Unikernel) ([]stri return cmd, nil } +// blockDevArgs returns one --disk or --disk-ro flag per disk, in order. The +// position of a disk in the list is its serial, so an entry with no path or a +// path listed twice is an error. backend names the builder in the error. +func blockDevArgs(backend string, disks []types.BlockDevSpec) ([]string, error) { + var out []string + seen := make(map[string]bool) + for i, disk := range disks { + if disk.Path == "" { + return nil, fmt.Errorf("%s: block device %d has no path", backend, i) + } + if seen[disk.Path] { + return nil, fmt.Errorf("%s: duplicate block device %q", backend, disk.Path) + } + seen[disk.Path] = true + flag := "--disk" + if disk.ReadOnly { + flag = "--disk-ro" + } + out = append(out, flag, disk.Path) + } + return out, nil +} + func (h *HviDarwin) PreExec(_ types.ExecArgs) error { return nil } func (h *HviDarwin) Signal(pid int, signal unix.Signal) error { diff --git a/pkg/unikontainers/hypervisors/hvi_darwin_test.go b/pkg/unikontainers/hypervisors/hvi_darwin_test.go index f964b7d98..de693447f 100644 --- a/pkg/unikontainers/hypervisors/hvi_darwin_test.go +++ b/pkg/unikontainers/hypervisors/hvi_darwin_test.go @@ -103,3 +103,128 @@ func TestHviDarwinGatewayNetwork(t *testing.T) { t.Fatalf("built-in network must not accompany gateway network: %s", got) } } + +// diskArgs returns the disk flags in argv with their values, in order, so a +// test can check both the mode of each disk and the order the serials follow. +func diskArgs(argv []string) []string { + var out []string + for i := 0; i < len(argv)-1; i++ { + switch argv[i] { + case "--disk", "--disk-ro", "--rootfs": + out = append(out, argv[i], argv[i+1]) + i++ + } + } + return out +} + +func TestHviDarwinDisks(t *testing.T) { + cases := []struct { + name string + args types.ExecArgs + want []string + }{ + { + name: "no disk", + args: types.ExecArgs{}, + want: nil, + }, + { + name: "legacy BlockDevPath is one writable disk", + args: types.ExecArgs{BlockDevPath: "/instance/root.ext4"}, + want: []string{"--disk", "/instance/root.ext4"}, + }, + { + // The serial is the position across both flags, so a read-only + // lower first and a writable upper second must stay in that order. + name: "BlockDevs keep their order and mode", + args: types.ExecArgs{BlockDevs: []types.BlockDevSpec{ + {Path: "/store/lower.ext4", ReadOnly: true}, + {Path: "/instance/upper.ext4"}, + }}, + want: []string{"--disk-ro", "/store/lower.ext4", "--disk", "/instance/upper.ext4"}, + }, + { + name: "BlockDevs win over BlockDevPath", + args: types.ExecArgs{ + BlockDevPath: "/instance/root.ext4", + BlockDevs: []types.BlockDevSpec{{Path: "/store/lower.ext4", ReadOnly: true}}, + }, + want: []string{"--disk-ro", "/store/lower.ext4"}, + }, + } + hvi := NewHviDarwin("/opt/hvi") + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tc.args.KernelPath = "/host/Image" + argv, err := hvi.BuildExecCmd(tc.args, &fakeUnikernel{}) + if err != nil { + t.Fatal(err) + } + got := diskArgs(argv) + if strings.Join(got, " ") != strings.Join(tc.want, " ") { + t.Fatalf("disks: got %q, want %q\nargv: %v", got, tc.want, argv) + } + }) + } +} + +func TestHviDarwinDisksRejected(t *testing.T) { + cases := []struct { + name string + disks []types.BlockDevSpec + want string + }{ + { + name: "empty path", + disks: []types.BlockDevSpec{{Path: "/store/lower.ext4", ReadOnly: true}, {}}, + want: "hvi: block device 1 has no path", + }, + { + name: "duplicate path", + disks: []types.BlockDevSpec{ + {Path: "/instance/disk.ext4", ReadOnly: true}, + {Path: "/instance/disk.ext4"}, + }, + want: `hvi: duplicate block device "/instance/disk.ext4"`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := NewHviDarwin("/opt/hvi").BuildExecCmd(types.ExecArgs{ + KernelPath: "/host/Image", + BlockDevs: tc.disks, + }, &fakeUnikernel{}) + if err == nil || err.Error() != tc.want { + t.Fatalf("got error %v, want %q", err, tc.want) + } + }) + } +} + +// Disks and the root share are independent on hvi: an overlay boot passes a +// read-only lower and a writable upper next to the virtio-fs shares. +func TestHviDarwinDisksWithShares(t *testing.T) { + argv, err := NewHviDarwin("/opt/hvi").BuildExecCmd(types.ExecArgs{ + KernelPath: "/host/Image", + Sharedfs: types.SharedfsParams{Path: "/store/rootfs", Tag: "rootfs", ReadOnly: true}, + SharedDirs: []types.SharedDirParams{{Path: "/host/home", Tag: "share0"}}, + BlockDevs: []types.BlockDevSpec{ + {Path: "/store/lower.ext4", ReadOnly: true}, + {Path: "/instance/upper.ext4"}, + }, + }, &fakeUnikernel{}) + if err != nil { + t.Fatal(err) + } + joined := strings.Join(argv, " ") + for _, want := range []string{ + "--disk-ro /store/lower.ext4 --disk /instance/upper.ext4", + "--share-ro /store/rootfs rootfs", + "--share-rw /host/home share0", + } { + if !strings.Contains(joined, want) { + t.Errorf("expected %q in HVI command:\n%s", want, joined) + } + } +} diff --git a/pkg/unikontainers/hypervisors/vz_darwin.go b/pkg/unikontainers/hypervisors/vz_darwin.go index 024b1c208..a6bb3504a 100644 --- a/pkg/unikontainers/hypervisors/vz_darwin.go +++ b/pkg/unikontainers/hypervisors/vz_darwin.go @@ -64,10 +64,13 @@ func (v *VzDarwin) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([ cmdArgs = append(cmdArgs, "--mac", args.Net.MAC) } - // Block device: attach ext4 image as virtio-blk disk - if args.BlockDevPath != "" { + // Legacy single block device: attach the ext4 image as the root disk. It + // takes the place of any root share. Only when BlockDevs is empty; disks + // listed there go out over --disk below and leave the root share alone. + switch { + case len(args.BlockDevs) == 0 && args.BlockDevPath != "": cmdArgs = append(cmdArgs, "--rootfs", args.BlockDevPath) - } else if args.Sharedfs.Path != "" { + case args.Sharedfs.Path != "": // Root filesystem over virtiofs. Mount tag fs0 matches the shared Linux // unikernel builder's cmdline (root=fs0 rootfstype=virtiofs), the same // convention the QEMU backend uses, so both monitors boot the identical @@ -81,7 +84,7 @@ func (v *VzDarwin) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([ tag = "fs0" } cmdArgs = append(cmdArgs, flag, args.Sharedfs.Path, tag) - } else if args.RootfsPath != "" && args.InitrdPath == "" { + case args.RootfsPath != "" && args.InitrdPath == "": // Legacy root share: a caller that builds its own kernel command line // (e.g. the macOS product with root=rootfs) passes RootfsPath instead // of Sharedfs; share it under mount tag "rootfs" to match, mirroring @@ -92,6 +95,15 @@ func (v *VzDarwin) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([ } } + // Additional disks, one flag per disk in order. vz-runner gives each the + // serial disk by its position across --disk and --disk-ro (a --rootfs + // disk is not counted), and the guest finds its disks by serial. + disks, err := blockDevArgs("vz", args.BlockDevs) + if err != nil { + return nil, err + } + cmdArgs = append(cmdArgs, disks...) + // Additional tagged shares (one --share per directory). A read-only share // goes over --share-ro, a separate flag rather than a third positional, so // the argument shape stays fixed and an older runner cannot mistake a mode diff --git a/pkg/unikontainers/hypervisors/vz_darwin_test.go b/pkg/unikontainers/hypervisors/vz_darwin_test.go index 11ec7055b..bfa4816c5 100644 --- a/pkg/unikontainers/hypervisors/vz_darwin_test.go +++ b/pkg/unikontainers/hypervisors/vz_darwin_test.go @@ -87,3 +87,158 @@ func TestVzDarwinContainerBootRootShareIsTaggedReadOnly(t *testing.T) { } } } + +func TestVzDarwinDisks(t *testing.T) { + cases := []struct { + name string + args types.ExecArgs + want []string + }{ + { + name: "no disk", + args: types.ExecArgs{}, + want: nil, + }, + { + name: "legacy BlockDevPath is the root disk", + args: types.ExecArgs{BlockDevPath: "/instance/root.ext4"}, + want: []string{"--rootfs", "/instance/root.ext4"}, + }, + { + // vz-runner numbers the serials over --disk and --disk-ro only, + // so the order here is the order of disk0, disk1. + name: "BlockDevs keep their order and mode", + args: types.ExecArgs{BlockDevs: []types.BlockDevSpec{ + {Path: "/store/lower.ext4", ReadOnly: true}, + {Path: "/instance/upper.ext4"}, + }}, + want: []string{"--disk-ro", "/store/lower.ext4", "--disk", "/instance/upper.ext4"}, + }, + { + name: "BlockDevs win over BlockDevPath", + args: types.ExecArgs{ + BlockDevPath: "/instance/root.ext4", + BlockDevs: []types.BlockDevSpec{{Path: "/store/lower.ext4", ReadOnly: true}}, + }, + want: []string{"--disk-ro", "/store/lower.ext4"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + tc.args.KernelPath = "/host/Image" + argv, err := NewVzDarwin().BuildExecCmd(tc.args, &fakeUnikernel{}) + if err != nil { + t.Fatal(err) + } + got := diskArgs(argv) + if strings.Join(got, " ") != strings.Join(tc.want, " ") { + t.Fatalf("disks: got %q, want %q\nargv: %v", got, tc.want, argv) + } + }) + } +} + +func TestVzDarwinDisksRejected(t *testing.T) { + cases := []struct { + name string + disks []types.BlockDevSpec + want string + }{ + { + name: "empty path", + disks: []types.BlockDevSpec{{Path: "/store/lower.ext4", ReadOnly: true}, {}}, + want: "vz: block device 1 has no path", + }, + { + name: "duplicate path", + disks: []types.BlockDevSpec{ + {Path: "/instance/disk.ext4", ReadOnly: true}, + {Path: "/instance/disk.ext4"}, + }, + want: `vz: duplicate block device "/instance/disk.ext4"`, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := NewVzDarwin().BuildExecCmd(types.ExecArgs{ + KernelPath: "/host/Image", + BlockDevs: tc.disks, + }, &fakeUnikernel{}) + if err == nil || err.Error() != tc.want { + t.Fatalf("got error %v, want %q", err, tc.want) + } + }) + } +} + +// Disks in BlockDevs leave the RootfsPath share in place, as they do the +// Sharedfs root share. +func TestVzDarwinDisksKeepRootfsPathShare(t *testing.T) { + dir := t.TempDir() + out, err := NewVzDarwin().BuildExecCmd(types.ExecArgs{ + KernelPath: "/host/Image", + RootfsPath: dir, + BlockDevs: []types.BlockDevSpec{{Path: "/instance/upper.ext4"}}, + }, &fakeUnikernel{}) + if err != nil { + t.Fatal(err) + } + joined := strings.Join(out, " ") + for _, want := range []string{ + "--share " + dir + " rootfs", + "--disk /instance/upper.ext4", + } { + if !strings.Contains(joined, want) { + t.Errorf("expected %q in Vz command:\n%s", want, joined) + } + } + if strings.Contains(joined, "--rootfs") { + t.Errorf("BlockDevs must not emit --rootfs:\n%s", joined) + } +} + +// The legacy root disk replaces the root share. Disks in BlockDevs do not: +// an overlay boot needs both the share and the disks on the command line. +func TestVzDarwinDisksKeepRootShare(t *testing.T) { + share := types.SharedfsParams{Path: "/store/rootfs", ReadOnly: true} + + out, err := NewVzDarwin().BuildExecCmd(types.ExecArgs{ + KernelPath: "/host/Image", + Sharedfs: share, + BlockDevs: []types.BlockDevSpec{ + {Path: "/store/lower.ext4", ReadOnly: true}, + {Path: "/instance/upper.ext4"}, + }, + }, &fakeUnikernel{}) + if err != nil { + t.Fatal(err) + } + joined := strings.Join(out, " ") + for _, want := range []string{ + "--share-ro /store/rootfs fs0", + "--disk-ro /store/lower.ext4 --disk /instance/upper.ext4", + } { + if !strings.Contains(joined, want) { + t.Errorf("expected %q in Vz command:\n%s", want, joined) + } + } + if strings.Contains(joined, "--rootfs") { + t.Errorf("BlockDevs must not emit --rootfs:\n%s", joined) + } + + out, err = NewVzDarwin().BuildExecCmd(types.ExecArgs{ + KernelPath: "/host/Image", + Sharedfs: share, + BlockDevPath: "/instance/root.ext4", + }, &fakeUnikernel{}) + if err != nil { + t.Fatal(err) + } + joined = strings.Join(out, " ") + if !strings.Contains(joined, "--rootfs /instance/root.ext4") { + t.Errorf("legacy root disk missing:\n%s", joined) + } + if strings.Contains(joined, "fs0") { + t.Errorf("legacy root disk must replace the root share:\n%s", joined) + } +} diff --git a/pkg/unikontainers/types/types.go b/pkg/unikontainers/types/types.go index 24f90e3f7..804ac84d2 100644 --- a/pkg/unikontainers/types/types.go +++ b/pkg/unikontainers/types/types.go @@ -115,6 +115,12 @@ type UnikernelParams struct { ContainerBoot bool // Use the generic initrd /init before entering a block rootfs } +// BlockDevSpec describes one disk image attached to the guest as virtio-blk. +type BlockDevSpec struct { + Path string // The path to the disk image on the host + ReadOnly bool // Attach the disk read-only +} + // ExecArgs holds the data required by Execve to start the VMM // FIXME: add extra fields if required by additional VMM's type ExecArgs struct { @@ -140,6 +146,14 @@ type ExecArgs struct { Net NetDevParams Sharedfs SharedfsParams SharedDirs []SharedDirParams // additional tagged virtiofs shares (Vz) + // BlockDevs are the disks to attach, in the order they go on the VMM's + // command line. The guest finds each one by its serial, disk, where N + // is its position here, so the order matters. BlockDevPath is the older + // single-disk form; it counts as one writable disk when BlockDevs is + // empty, and is ignored otherwise. Use EffectiveBlockDevs to read both. + // Read by the darwin builders only (hvi, Vz). The Linux hvi and the QEMU + // builders still read BlockDevPath. + BlockDevs []BlockDevSpec // GuestUID and GuestGID are who the workload runs as inside the guest. // // A file server sharing a host directory has to present it as owned by @@ -152,6 +166,18 @@ type ExecArgs struct { GuestGID uint32 } +// EffectiveBlockDevs returns the disks to attach, in order: BlockDevs when +// it is set, else BlockDevPath as a single writable disk, else nil. +func (a ExecArgs) EffectiveBlockDevs() []BlockDevSpec { + if len(a.BlockDevs) > 0 { + return a.BlockDevs + } + if a.BlockDevPath != "" { + return []BlockDevSpec{{Path: a.BlockDevPath}} + } + return nil +} + type MonitorCliArgs struct { ExtraInitrd string OtherArgs []string