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