Compare commits

...

5 Commits

Author SHA1 Message Date
brynmailer-mdc 2961d73d03 test(compute): cover an unnamed boot device with no medium in the device map 2026-11-02 14:25:25 +10:00
brynmailer-mdc 042666b6ad refactor(compute): share the NBD node options and export dispatch
The data and boot -blockdev builders share one helper for the nbd server,
export and reconnect-delay options, buildDrives dispatches both branches
through one export helper, and hot attach parses its URI with the shared
parser. Emitted arguments are unchanged. The BlkDeviceMapped doc now states
when it falls back to the scalar form.
2026-11-02 14:25:25 +10:00
brynmailer-mdc 3a19e76f6b fix(compute): refuse a disk-boot launch with no boot volume
Data-volume -blockdev nodes satisfy Execute's drive check on their own, so
a disk-boot request list without a boot volume would start a QEMU with
nothing to boot. The disk-boot path now refuses it before launch.
2026-11-02 14:24:18 +10:00
brynmailer-mdc 03cb800f4c test(viperblock): pin the boot -blockdev shape in the nbdkit orphan reaper
The boot volume's NBD server options now carry a file. prefix, which the
referencer scan matches only because it compares by substring. Cover the
unix and TCP boot strings so a stricter match cannot reap a live root
export, and stop describing the joined host:port form as a boot drive.
2026-11-02 14:23:24 +10:00
brynmailer-mdc 6aa989e1ec feat(compute): boot the root volume from a -blockdev node that takes NBD or host_device
The boot volume moves from -drive if=none to a -blockdev node named os, so
a block export reaches QEMU as host_device as data volumes already do. The
NBD boot shape keeps its cache, reconnect, iothread and bootindex settings;
werror and rerror move onto the virtio-blk device. A block EFI volume is
still refused because pflash has no host_device form.
2026-11-02 14:14:07 +10:00
11 changed files with 333 additions and 115 deletions
@@ -363,11 +363,11 @@ func endpointReferencer(procRoot string, disc discoveredNbdkit) (pid int, found
// cmdlineReferencesEndpoint checks a NUL-separated /proc/<pid>/cmdline for
// disc's endpoint. A socket mount's path appears verbatim in whichever
// argument named it (QEMU's nbd+unix:///?socket=<path> file= or its
// server.path=<path> -blockdev option). A TCP mount's 127.0.0.1:<port> pair
// may appear joined in one argument (a boot/EFI drive's nbd://host:port) or
// split across server.host=/server.port= within the same comma-joined
// -blockdev argument, so both shapes are checked.
// argument named it (an EFI -drive's file= or a -blockdev's server.path=). A
// TCP mount's 127.0.0.1:<port> pair may appear joined in one argument (an EFI
// pflash drive's nbd://host:port) or split across server.host=/server.port=
// within one -blockdev argument. The match is by substring, so the boot
// volume's file.server.path=/file.server.host=/file.server.port= also match.
func cmdlineReferencesEndpoint(cmdline []byte, disc discoveredNbdkit) bool {
args := bytes.Split(bytes.TrimRight(cmdline, "\x00"), []byte{0})
@@ -415,7 +415,7 @@ func TestReapOrphanedNbdkit_LiveReferencer_LeftRunningAndLogged(t *testing.T) {
// TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs proves
// endpointReferencer recognizes a TCP endpoint even when QEMU's -blockdev
// splits host and port into server.host=/server.port= within one argument,
// not just the joined host:port form a boot/EFI drive uses.
// not just the joined host:port form an EFI pflash drive uses.
func TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs(t *testing.T) {
pid := spawnLongRunningProcess(t)
procRoot := t.TempDir()
@@ -428,6 +428,46 @@ func TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs(t *test
assert.Never(t, func() bool { return !hostprocess.ProcessAlive(pid) }, 50*time.Millisecond, 5*time.Millisecond, "a TCP endpoint split across server.host=/server.port= must still be recognized as referenced")
}
// TestReapOrphanedNbdkit_BootBlockdevReferencer_LeftRunning pins the boot
// volume's -blockdev, whose server options carry a file. prefix: a live
// guest's root nbdkit named only that way must never be signalled.
func TestReapOrphanedNbdkit_BootBlockdevReferencer_LeftRunning(t *testing.T) {
socket := filepath.Join(t.TempDir(), "vol-boot-root.sock")
tests := []struct {
name string
arg string
disc func(pid int) discoveredNbdkit
}{
{
name: "unix socket",
arg: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=" + socket + ",file.export=,file.reconnect-delay=30",
disc: func(pid int) discoveredNbdkit {
return discoveredNbdkit{PID: pid, Volume: "vol-boot-unix1", Socket: socket}
},
},
{
name: "tcp",
arg: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=inet,file.server.host=127.0.0.1,file.server.port=10897,file.export=,file.reconnect-delay=30",
disc: func(pid int) discoveredNbdkit {
return discoveredNbdkit{PID: pid, Volume: "vol-boot-tcp1", Port: 10897}
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
pid := spawnLongRunningProcess(t)
procRoot := t.TempDir()
const referencerPID = 909092
writeReferencerFixture(t, procRoot, referencerPID, tt.arg)
logs := captureLogs(t)
assert.False(t, reapOrphanedNbdkit(procRoot, tt.disc(pid), reapReasonUnadoptable), "a boot volume's nbdkit must never be signalled")
assert.Never(t, func() bool { return !hostprocess.ProcessAlive(pid) }, 50*time.Millisecond, 5*time.Millisecond, "a boot volume's nbdkit must never be signalled")
assert.Contains(t, logs.String(), fmt.Sprintf("referencing_pid=%d", referencerPID))
})
}
}
// unreadableProcDir creates procRoot/<pid> with a cmdline file inside, then
// strips all permissions from the directory itself, so os.ReadFile on the
// cmdline path inside it fails with a permission error rather than
@@ -84,6 +84,10 @@ func buildDeviceMap(devices []qmp.BlockDevice) map[string]string {
name := dev.Device
pciIndex := extractPCIIndex(dev.QDev)
// A -blockdev boot node has no BlockBackend name; its node-name is "os".
if name == "" && pciIndex >= 0 && dev.Inserted != nil {
name = dev.Inserted.NodeName
}
if pciIndex < 0 {
// Hot-plugged device: extract name and port from QDev path.
@@ -127,6 +127,23 @@ func TestBuildDeviceMap(t *testing.T) {
},
wantLen: 4,
},
{
// A -blockdev boot node leaves the BlockBackend unnamed, so the
// entry is keyed by its node-name instead of "".
name: "blockdev boot device is named by its node-name",
devices: []qmp.BlockDevice{
{Device: "", Inserted: &qmp.BlockInserted{NodeName: "os"}, QDev: "/machine/peripheral-anon/device[0]/virtio-backend"},
{Device: "cloudinit", Inserted: &qmp.BlockInserted{NodeName: "#block123"}, QDev: "/machine/peripheral-anon/device[3]/virtio-backend"},
{Device: "", Inserted: &qmp.BlockInserted{NodeName: "nbd-vol-abc"}, QDev: "/machine/peripheral/vdisk-vol-abc/virtio-backend"},
},
want: map[string]string{
"os": "/dev/vda",
"cloudinit": "/dev/vdb",
"vdisk-vol-abc": "/dev/vdc",
},
absent: []string{"", "nbd-vol-abc"},
wantLen: 3,
},
{
// Boot devices returned out of order: lowest PCI index wins /dev/vda;
// hot-plugged devices sort after boot devices.
@@ -426,3 +443,15 @@ func TestLogGuestDeviceMap_LeavesBlockDeviceMappingsAlone(t *testing.T) {
assert.Equal(t, "/dev/sdf", aws.StringValue(v.Instance.BlockDeviceMappings[1].DeviceName),
"a data mapping must still name the device the caller asked for, not /dev/vdb")
}
// TestBuildDeviceMap_UnnamedBootDeviceWithoutMedium covers an unnamed
// boot-time BlockBackend that reports no inserted medium.
func TestBuildDeviceMap_UnnamedBootDeviceWithoutMedium(t *testing.T) {
devices := []qmp.BlockDevice{
{Device: "", Inserted: nil, QDev: "/machine/peripheral-anon/device[0]/virtio-backend"},
{Device: "cloudinit", Inserted: &qmp.BlockInserted{}, QDev: "/machine/peripheral-anon/device[3]/virtio-backend"},
}
var got map[string]string
require.NotPanics(t, func() { got = buildDeviceMap(devices) })
assert.Equal(t, "/dev/vdb", got["cloudinit"], "the unnamed device still holds its PCI slot")
}
+61 -11
View File
@@ -6,6 +6,8 @@ import (
"slices"
"strconv"
"strings"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/nbd"
)
// IsMicrovm reports whether machineType is the QEMU microvm machine, with or
@@ -162,12 +164,14 @@ type blkDeviceJSON struct {
NumQueues int `json:"num-queues,omitempty"`
BootIndex int `json:"bootindex,omitempty"`
Mapping []iothreadVQMapping `json:"iothread-vq-mapping"`
Werror string `json:"werror"`
Rerror string `json:"rerror"`
}
// BlkDeviceMapped returns a virtio-blk -device that spreads its virtqueues
// across a pool of poolSize IOThreads, round-robin. Falls back to BlkDevice's
// scalar iothread= form when poolSize <= 1, so the common small-guest case is
// byte-for-byte what it was before this existed.
// scalar iothread= form when poolSize <= 1, when there is one queue, or on an
// MMIO machine.
//
// Every queue is mapped exactly once and the scalar iothread= property is
// absent: QEMU treats the two as alternative assignments and rejects both.
@@ -197,6 +201,8 @@ func BlkDeviceMapped(machineType, drive, iothreadBase string, queues, bootIdx, p
NumQueues: queues,
BootIndex: bootIdx,
Mapping: mapping,
Werror: "stop",
Rerror: "stop",
})
if err != nil {
return Device{}, fmt.Errorf("marshal virtio-blk iothread-vq-mapping: %w", err)
@@ -204,9 +210,9 @@ func BlkDeviceMapped(machineType, drive, iothreadBase string, queues, bootIdx, p
return Device{Value: string(b)}, nil
}
// BlkDevice returns the appropriate QEMU virtio-blk device string for
// machineType. iothread and queues are omitted when empty/zero. bootIdx is
// only emitted for PCI machines (MMIO has no bootindex concept).
// BlkDevice returns the virtio-blk device string for machineType, omitting an
// empty iothread or zero queues; bootIdx is PCI-only. It sets werror/rerror=stop
// (see VolumeBlkDeviceArgs), as a -blockdev node has no BlockBackend to hold it.
func BlkDevice(machineType, drive, iothread string, queues int, bootIdx int) Device {
var b strings.Builder
if IsMMIO(machineType) {
@@ -224,6 +230,7 @@ func BlkDevice(machineType, drive, iothread string, queues int, bootIdx int) Dev
if !IsMMIO(machineType) && bootIdx > 0 {
fmt.Fprintf(&b, ",bootindex=%d", bootIdx)
}
b.WriteString(",werror=stop,rerror=stop")
return Device{Value: b.String()}
}
@@ -282,9 +289,9 @@ func HotplugEBSBus(port int) string {
// (rendered as a joined command-line string) so the two block-graph shapes
// cannot diverge.
//
// werror/rerror mirror the boot drive's on-error policy (see Drive.Werror):
// a data volume has no -drive-backed BlockBackend to carry it, so the qdev
// device itself sets werror=stop,rerror=stop.
// werror=stop,rerror=stop pauses the VM and holds a failed request, so a
// transient backend loss is resumed with cont instead of reaching the guest as
// EIO. A -blockdev node has no BlockBackend to carry it, so the device sets it.
func VolumeBlkDeviceArgs(volumeID, nodeName, iothreadID, bus string) []string {
return []string{
fmt.Sprintf("id=%s", VolumeDeviceID(volumeID)),
@@ -383,11 +390,54 @@ func (o NBDServerOpts) CommandLineArgs() []string {
// monitor-owned node that blockdev-del can later remove (unlike -drive,
// which only creates a BlockBackend with an auto-generated node-name).
func VolumeBlockdev(nodeName string, server NBDServerOpts) Blockdev {
opts := append([]string{"driver=nbd", fmt.Sprintf("node-name=%s", nodeName)}, server.CommandLineArgs()...)
opts = append(opts, "export=", fmt.Sprintf("reconnect-delay=%d", NBDReconnectDelaySeconds))
opts := append([]string{"driver=nbd", fmt.Sprintf("node-name=%s", nodeName)}, nbdNodeOpts("", server)...)
return Blockdev{Value: strings.Join(opts, ",")}
}
// nbdNodeOpts returns an nbd node's server, export and reconnect-delay options,
// each prefixed with prefix: "" on the nbd node itself, "file." under raw.
func nbdNodeOpts(prefix string, server NBDServerOpts) []string {
var opts []string
for _, arg := range server.CommandLineArgs() {
opts = append(opts, prefix+arg)
}
return append(opts, prefix+"export=", fmt.Sprintf("%sreconnect-delay=%d", prefix, NBDReconnectDelaySeconds))
}
// bootNodeName is the boot volume's block-node name, which its -device names
// with drive=.
const bootNodeName = "os"
// nbdServerOpts parses an NBD URI into the server options a blockdev takes.
func nbdServerOpts(uri string) (NBDServerOpts, error) {
serverType, socketPath, host, port, err := nbd.ParseNBDURI(uri)
if err != nil {
return NBDServerOpts{}, err
}
return NBDServerOpts{Type: serverType, Path: socketPath, Host: host, Port: port}, nil
}
// bootNBDBlockdev renders the boot volume's -blockdev for an NBD export: a raw
// node over an nbd node, the graph -drive format=raw,cache=none built, with the
// same O_DIRECT setting and reconnect delay.
func bootNBDBlockdev(nodeName string, server NBDServerOpts) Blockdev {
opts := append([]string{"driver=raw", fmt.Sprintf("node-name=%s", nodeName), "cache.direct=on", "file.driver=nbd"}, nbdNodeOpts("file.", server)...)
return Blockdev{Value: strings.Join(opts, ",")}
}
// exportBlockdev renders v's -blockdev as nodeName: a host_device node for a
// block export, otherwise the NBD node nbdBlockdev builds from its URI.
func exportBlockdev(v EBSRequest, kind volumeExportKind, nodeName string, nbdBlockdev func(string, NBDServerOpts) Blockdev) (Blockdev, error) {
if kind == volumeExportBlock {
return VolumeHostDeviceBlockdev(nodeName, HostDeviceOpts{Path: v.BlockDevice, SizeBytes: v.BlockSizeBytes}), nil
}
server, err := nbdServerOpts(v.NBDURI)
if err != nil {
return Blockdev{}, fmt.Errorf("parse NBDURI for volume %s: %w", v.Name, err)
}
return nbdBlockdev(nodeName, server), nil
}
// HostDeviceOpts is a host block device export. QEMU opens Path through the
// host_device protocol driver with O_DIRECT and native AIO, and a raw format
// node over it exposes only the first SizeBytes to the guest.
@@ -413,7 +463,7 @@ func (o HostDeviceOpts) QMPArgs(nodeName string) map[string]any {
}
// VolumeHostDeviceBlockdev renders the -blockdev equivalent of QMPArgs for a
// data volume backed by a host block device.
// volume backed by a host block device.
func VolumeHostDeviceBlockdev(nodeName string, dev HostDeviceOpts) Blockdev {
return Blockdev{Value: fmt.Sprintf(
"driver=raw,node-name=%s,size=%d,file.driver=host_device,file.filename=%s,file.aio=native,file.cache.direct=on",
+5 -5
View File
@@ -130,28 +130,28 @@ func TestNICQueues_ClampedWhenEnabled(t *testing.T) {
func TestBlkDevice_PCI(t *testing.T) {
d := BlkDevice("q35", "os", "ioth-os", 4, 1)
assert.Equal(t, "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=4,bootindex=1", d.Value)
assert.Equal(t, "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=4,bootindex=1,werror=stop,rerror=stop", d.Value)
}
func TestBlkDevice_PCI_NoBootIdx(t *testing.T) {
d := BlkDevice("q35", "data", "ioth-data", 2, 0)
assert.Equal(t, "virtio-blk-pci,drive=data,iothread=ioth-data,num-queues=2", d.Value)
assert.Equal(t, "virtio-blk-pci,drive=data,iothread=ioth-data,num-queues=2,werror=stop,rerror=stop", d.Value)
}
func TestBlkDevice_PCI_NoIOThread(t *testing.T) {
d := BlkDevice("q35", "data", "", 0, 0)
assert.Equal(t, "virtio-blk-pci,drive=data", d.Value)
assert.Equal(t, "virtio-blk-pci,drive=data,werror=stop,rerror=stop", d.Value)
}
func TestBlkDevice_MMIO(t *testing.T) {
d := BlkDevice("microvm", "os", "ioth-os", 4, 1)
// MMIO has no bootindex
assert.Equal(t, "virtio-blk-device,drive=os,iothread=ioth-os,num-queues=4", d.Value)
assert.Equal(t, "virtio-blk-device,drive=os,iothread=ioth-os,num-queues=4,werror=stop,rerror=stop", d.Value)
}
func TestBlkDevice_MMIO_NoIOThread(t *testing.T) {
d := BlkDevice("microvm,x-option-roms=off", "data", "", 0, 0)
assert.Equal(t, "virtio-blk-device,drive=data", d.Value)
assert.Equal(t, "virtio-blk-device,drive=data,werror=stop,rerror=stop", d.Value)
}
func TestRngDevice_PCI(t *testing.T) {
+27 -42
View File
@@ -315,7 +315,7 @@ func (m *Manager) startQEMU(instance *VM, appendConsole bool) error {
m.initENIRequests(instance)
instance.EBSRequests.Mu.Lock()
driveCfg, err := buildDrives(instance.EBSRequests.Requests, spec.VCPUs, instance.Config.MachineType)
driveCfg, err := buildDiskBootDrives(instance.EBSRequests.Requests, spec.VCPUs, instance.Config.MachineType)
instance.EBSRequests.Mu.Unlock()
if err != nil {
return err
@@ -1398,16 +1398,24 @@ func directBootLaunchConfig(base Config, requests []EBSRequest, cpuCount int) (C
return cfg, nil
}
// buildDiskBootDrives is buildDrives for a guest that boots from a volume: a
// request list with no boot volume is refused, as QEMU would start with
// nothing to boot from and the guest would never come up.
func buildDiskBootDrives(requests []EBSRequest, cpuCount int, machineType string) (driveConfig, error) {
if !slices.ContainsFunc(requests, func(r EBSRequest) bool { return r.Boot }) {
return driveConfig{}, errors.New("no boot volume among the instance's EBS requests")
}
return buildDrives(requests, cpuCount, machineType)
}
// buildDrives converts EBS requests into QEMU drive/iothread/device/blockdev
// configs. Returns an error if any volume has no valid export (see
// classifyVolumeExport), or if the EBS hot-plug port pool is exhausted while
// allocating one for a data volume.
//
// Boot volumes keep the existing anonymous if=none -drive shape exactly as
// before — they are not detachable (see ErrVolumeNotDetachable), so the
// anonymous-node problem this function otherwise fixes does not apply, and
// changing the boot drive risks boot order. EFI volumes emit pflash unit=1;
// the readonly CODE blob (unit=0) is added by Config.Execute.
// The boot volume is a -blockdev node named "os" under a bootindex=1 device
// that carries werror/rerror; it is never detached, so it keeps fixed names.
// EFI volumes emit pflash unit=1; Config.Execute adds the CODE blob (unit=0).
//
// Every other (data) volume is given the same named node-name/device-id/
// iothread-id shape AttachVolume's hot-plug path uses, via a -blockdev entry
@@ -1432,41 +1440,29 @@ func buildDrives(requests []EBSRequest, cpuCount int, machineType string) (drive
}
switch {
case kind == volumeExportBlock && (v.Boot || v.EFI):
// Boot and EFI keep the legacy -drive shape, which has no
// host_device form here; only data volumes take a block export.
return driveConfig{}, fmt.Errorf("volume %s: block device export is only supported for data volumes", v.Name)
case kind == volumeExportBlock && v.EFI:
// The VARS store is a pflash -drive, which has no host_device form.
return driveConfig{}, fmt.Errorf("volume %s: block device export is not supported for an EFI volume", v.Name)
case v.Boot:
drive := Drive{
File: v.NBDURI,
Format: "raw",
If: "none",
Media: "disk",
ID: "os",
Cache: "none",
// Pause the VM and hold the failed request rather than
// delivering EIO to the guest, which aborts its journal. A
// transient backend loss is then recovered with cont.
Werror: "stop",
Rerror: "stop",
ReconnectDelay: NBDReconnectDelaySeconds,
blockdev, err := exportBlockdev(*v, kind, bootNodeName, bootNBDBlockdev)
if err != nil {
return driveConfig{}, err
}
// The boot disk's virtqueues are spread across a pool rather than
// all served by one host thread. A pool of 1 emits exactly the
// command line this used to, which is every guest below 4 vCPUs.
// all served by one host thread. A pool of 1 keeps the scalar
// iothread= form, which is every guest below 4 vCPUs.
pool := BlkIOThreadPoolSize(cpuCount)
for i := range pool {
cfg.IOThreads = append(cfg.IOThreads, IOThread{ID: BlkIOThreadID("ioth-os", i, pool)})
}
dev, err := BlkDeviceMapped(machineType, drive.ID, "ioth-os", cpuCount, 1, pool)
dev, err := BlkDeviceMapped(machineType, bootNodeName, "ioth-os", cpuCount, 1, pool)
if err != nil {
return driveConfig{}, fmt.Errorf("boot volume %s: %w", v.Name, err)
}
cfg.Blockdevs = append(cfg.Blockdevs, blockdev)
cfg.Devices = append(cfg.Devices, dev)
cfg.Drives = append(cfg.Drives, drive)
case v.EFI:
cfg.Drives = append(cfg.Drives, Drive{
@@ -1495,20 +1491,9 @@ func buildDrives(requests []EBSRequest, cpuCount int, machineType string) (drive
nodeName := VolumeNodeName(v.Name)
iothreadID := VolumeIOThreadID(v.Name)
var blockdev Blockdev
if kind == volumeExportBlock {
blockdev = VolumeHostDeviceBlockdev(nodeName, HostDeviceOpts{Path: v.BlockDevice, SizeBytes: v.BlockSizeBytes})
} else {
serverType, socketPath, nbdHost, nbdPort, err := nbd.ParseNBDURI(v.NBDURI)
if err != nil {
return driveConfig{}, fmt.Errorf("parse NBDURI for volume %s: %w", v.Name, err)
}
blockdev = VolumeBlockdev(nodeName, NBDServerOpts{
Type: serverType,
Path: socketPath,
Host: nbdHost,
Port: nbdPort,
})
blockdev, err := exportBlockdev(*v, kind, nodeName, VolumeBlockdev)
if err != nil {
return driveConfig{}, err
}
cfg.IOThreads = append(cfg.IOThreads, IOThread{ID: iothreadID})
+142 -18
View File
@@ -132,12 +132,12 @@ func TestBuildDrives(t *testing.T) {
{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
},
cpuCount: 2,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
},
wantIOThreads: []IOThread{{ID: "ioth-os"}},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1"},
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
},
},
{
@@ -148,12 +148,12 @@ func TestBuildDrives(t *testing.T) {
{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
},
cpuCount: 4,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
},
wantIOThreads: []IOThread{{ID: "ioth-os-0"}, {ID: "ioth-os-1"}},
wantDevices: []Device{
{Value: `{"driver":"virtio-blk-pci","drive":"os","num-queues":4,"bootindex":1,"iothread-vq-mapping":[{"iothread":"ioth-os-0","vqs":[0,2]},{"iothread":"ioth-os-1","vqs":[1,3]}]}`},
{Value: `{"driver":"virtio-blk-pci","drive":"os","num-queues":4,"bootindex":1,"iothread-vq-mapping":[{"iothread":"ioth-os-0","vqs":[0,2]},{"iothread":"ioth-os-1","vqs":[1,3]}],"werror":"stop","rerror":"stop"}`},
},
},
{
@@ -249,7 +249,7 @@ func TestBuildDrives(t *testing.T) {
wantErr: "no free EBS hot-plug port for volume vol-overflow",
},
{
name: "mixed boot + EFI + data: boot and EFI keep the legacy -drive shape",
name: "mixed boot + EFI + data: boot is a -blockdev node and EFI keeps the pflash -drive",
requests: []EBSRequest{
{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
{Name: "vol-efi", NBDURI: "nbd:unix:/tmp/efi.sock", EFI: true},
@@ -260,7 +260,6 @@ func TestBuildDrives(t *testing.T) {
// unaffected by the pool either way — they keep one IOThread each.
cpuCount: 2,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
{File: "nbd:unix:/tmp/efi.sock", Format: "raw", If: "pflash", Unit: 1},
},
wantIOThreads: []IOThread{
@@ -268,10 +267,11 @@ func TestBuildDrives(t *testing.T) {
{ID: "ioth-vol-data-a"},
},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
{Value: "driver=nbd,node-name=nbd-vol-data-a,server.type=unix,server.path=/tmp/data-a.sock,export=,reconnect-delay=30"},
},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1"},
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
{Value: "virtio-blk-pci,id=vdisk-vol-data-a,drive=nbd-vol-data-a,iothread=ioth-vol-data-a,serial=voldataa,bus=hotplug-ebs3,werror=stop,rerror=stop"},
},
wantHotplugPorts: []int{0, 0, 3},
@@ -348,31 +348,72 @@ func TestBuildDrives(t *testing.T) {
{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
{Name: "vol-blk", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20},
},
cpuCount: 2,
wantDrives: []Drive{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds},
},
cpuCount: 2,
wantIOThreads: []IOThread{{ID: "ioth-os"}, {ID: "ioth-vol-blk"}},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=unix,file.server.path=/tmp/boot.sock,file.export=,file.reconnect-delay=30"},
{Value: "driver=raw,node-name=nbd-vol-blk,size=1048576,file.driver=host_device,file.filename=/dev/sdx,file.aio=native,file.cache.direct=on"},
},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1"},
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
{Value: "virtio-blk-pci,id=vdisk-vol-blk,drive=nbd-vol-blk,iothread=ioth-vol-blk,serial=volblk,bus=hotplug-ebs1,werror=stop,rerror=stop"},
},
wantHotplugPorts: []int{0, 1},
},
{
name: "block boot volume is rejected",
requests: []EBSRequest{{Name: "vol-boot", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20, Boot: true}},
name: "block boot volume is a raw node over host_device named os under the bootindex device",
requests: []EBSRequest{
{Name: "vol-boot", BlockDevice: "/dev/sdx", BlockSizeBytes: 8 << 30, Boot: true},
},
cpuCount: 2,
wantIOThreads: []IOThread{{ID: "ioth-os"}},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,size=8589934592,file.driver=host_device,file.filename=/dev/sdx,file.aio=native,file.cache.direct=on"},
},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
},
wantHotplugPorts: []int{0},
},
{
name: "block boot volume with four vCPUs keeps the mapped device and error policy",
requests: []EBSRequest{
{Name: "vol-boot", BlockDevice: "/dev/sdx", BlockSizeBytes: 8 << 30, Boot: true},
},
cpuCount: 4,
wantIOThreads: []IOThread{{ID: "ioth-os-0"}, {ID: "ioth-os-1"}},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,size=8589934592,file.driver=host_device,file.filename=/dev/sdx,file.aio=native,file.cache.direct=on"},
},
wantDevices: []Device{
{Value: `{"driver":"virtio-blk-pci","drive":"os","num-queues":4,"bootindex":1,"iothread-vq-mapping":[{"iothread":"ioth-os-0","vqs":[0,2]},{"iothread":"ioth-os-1","vqs":[1,3]}],"werror":"stop","rerror":"stop"}`},
},
},
{
name: "NBD boot volume over TCP renders inet server options under file.",
requests: []EBSRequest{
{Name: "vol-boot", NBDURI: "nbd://127.0.0.1:44801", Boot: true},
},
cpuCount: 2,
wantIOThreads: []IOThread{{ID: "ioth-os"}},
wantBlockdevs: []Blockdev{
{Value: "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=inet,file.server.host=127.0.0.1,file.server.port=44801,file.export=,file.reconnect-delay=30"},
},
wantDevices: []Device{
{Value: "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=2,bootindex=1,werror=stop,rerror=stop"},
},
},
{
name: "unparseable NBD boot URI is rejected",
requests: []EBSRequest{{Name: "vol-boot", NBDURI: "http://example/boot", Boot: true}},
cpuCount: 2,
wantErr: "volume vol-boot: block device export is only supported for data volumes",
wantErr: "parse NBDURI for volume vol-boot",
},
{
name: "block EFI volume is rejected",
requests: []EBSRequest{{Name: "vol-efi", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20, EFI: true}},
cpuCount: 2,
wantErr: "volume vol-efi: block device export is only supported for data volumes",
wantErr: "volume vol-efi: block device export is not supported for an EFI volume",
},
{
name: "both NBDURI and BlockDevice set is rejected",
@@ -1109,6 +1150,7 @@ func TestStartQEMU_DiskBoot_MgmtTapCreated(t *testing.T) {
InstanceType: "t3.nano",
MgmtMAC: "02:aa:bb:cc:dd:ee",
}
instance.EBSRequests.Requests = []EBSRequest{{Name: "vol-root", NBDURI: "nbd:unix:/tmp/root.sock", Boot: true}}
err := m.startQEMU(instance, false)
@@ -2168,6 +2210,88 @@ func TestStartQEMU_DiskBoot_DataVolumeStaysOnThePCIPath(t *testing.T) {
assert.Equal(t, 1, instance.EBSRequests.Requests[1].HotplugPort)
}
// TestStartQEMU_DiskBoot_BootVolumeIsABlockdevNode launches a disk-boot guest
// from each export kind and checks the boot volume reaches QEMU as the "os"
// -blockdev node behind the bootindex=1 device, with no -drive left for it.
func TestStartQEMU_DiskBoot_BootVolumeIsABlockdevNode(t *testing.T) {
tests := []struct {
name string
req func(t *testing.T) EBSRequest
wantBlockdev func(req EBSRequest) string
}{
{
name: "nbd export",
req: func(t *testing.T) EBSRequest {
return EBSRequest{Name: "vol-root", NBDURI: nbdListener(t), Boot: true}
},
wantBlockdev: func(req EBSRequest) string {
hostPort := strings.TrimPrefix(req.NBDURI, "nbd://")
host, port, _ := strings.Cut(hostPort, ":")
return "driver=raw,node-name=os,cache.direct=on,file.driver=nbd,file.server.type=inet,file.server.host=" + host +
",file.server.port=" + port + ",file.export=,file.reconnect-delay=30"
},
},
{
name: "block export",
req: func(t *testing.T) EBSRequest {
dev := filepath.Join(t.TempDir(), "sdx")
require.NoError(t, os.WriteFile(dev, nil, 0o600))
return EBSRequest{Name: "vol-root", BlockDevice: dev, BlockSizeBytes: 8 << 30, Boot: true}
},
wantBlockdev: func(req EBSRequest) string {
return "driver=raw,node-name=os,size=8589934592,file.driver=host_device,file.filename=" + req.BlockDevice +
",file.aio=native,file.cache.direct=on"
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
m := directBootManager(t, &fakeNetworkPlumber{})
launches := withFakeQEMU(t)
req := tt.req(t)
instance := &VM{ID: "i-boot-blockdev", InstanceType: "t3.nano"}
instance.EBSRequests.Requests = []EBSRequest{req}
require.NoError(t, m.startQEMU(instance, false))
got := launches()
require.Len(t, got, 1)
assert.Equal(t, []string{tt.wantBlockdev(req)}, allArgValues(got[0], "-blockdev"))
assert.Contains(t, allArgValues(got[0], "-device"), "virtio-blk-pci,drive=os,iothread=ioth-os,num-queues=1,bootindex=1,werror=stop,rerror=stop")
for _, d := range allArgValues(got[0], "-drive") {
assert.NotContains(t, d, "id=os", "the boot volume must not also be a -drive")
}
})
}
}
// TestStartQEMU_DiskBoot_NoBootVolumeIsNotLaunched covers a disk-boot guest
// whose only volumes are data volumes: their -blockdev nodes would satisfy
// Execute, but QEMU would have nothing to boot from.
func TestStartQEMU_DiskBoot_NoBootVolumeIsNotLaunched(t *testing.T) {
tests := []struct {
name string
requests []EBSRequest
}{
{name: "data volume only", requests: []EBSRequest{{Name: "vol-data-a", NBDURI: "nbd:unix:/tmp/data-a.sock"}}},
{name: "no volumes", requests: nil},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
m := directBootManager(t, &fakeNetworkPlumber{})
launches := withFakeQEMU(t)
instance := &VM{ID: "i-no-boot", InstanceType: "t3.nano"}
instance.EBSRequests.Requests = tt.requests
err := m.startQEMU(instance, false)
require.Error(t, err)
assert.Contains(t, err.Error(), "no boot volume among the instance's EBS requests")
assert.Empty(t, launches(), "QEMU must not start")
})
}
}
func deviceValues(devs []Device) []string {
out := make([]string, len(devs))
for i, d := range devs {
+7 -28
View File
@@ -371,8 +371,8 @@ type Device struct {
Value string `json:"value"`
}
// Drive is one QEMU -drive entry, used for boot, EFI and pflash firmware
// drives. Each set field becomes a key=value option.
// Drive is one QEMU -drive entry, used for the pflash firmware drives (the
// CODE blob and the EFI volume). Each set field becomes a key=value option.
type Drive struct {
File string `json:"file"`
Format string `json:"format"`
@@ -380,15 +380,6 @@ type Drive struct {
Media string `json:"media"`
ID string `json:"id"`
Cache string `json:"cache,omitempty"`
// Werror and Rerror set QEMU's block on-error policy. "stop" pauses the VM
// and holds the failed request, so a transient backend loss is resumed with
// cont instead of reaching the guest as EIO and corrupting its filesystem.
Werror string `json:"werror,omitempty"`
Rerror string `json:"rerror,omitempty"`
// ReconnectDelay emits file.reconnect-delay, the seconds an NBD disconnect
// pauses requests before failing them. The file. prefix is required: the
// bare name reaches the format driver, which has no such option.
ReconnectDelay int `json:"reconnect_delay,omitempty"`
// Unit selects the pflash slot when If=="pflash": 0 is the CODE blob,
// 1 is the per-VM VARS volume. Ignored for non-pflash drives.
Unit int `json:"unit,omitempty"`
@@ -405,9 +396,9 @@ type IOThread struct {
// Blockdev is a QEMU -blockdev entry: the command-line equivalent of the
// blockdev-add QMP command. Unlike a -drive with if=none, it produces a
// monitor-owned node addressable by node-name, which is what makes
// blockdev-del able to remove it later. Used for data (non-boot, non-EFI)
// volumes so a cold-booted one has the same detachable block-graph shape as
// a hot-attached one; boot/EFI volumes keep the legacy -drive shape.
// blockdev-del able to remove it later. Used for boot and data volumes, so a
// cold-booted data volume has the same detachable block-graph shape as a
// hot-attached one; EFI volumes keep the pflash -drive shape.
type Blockdev struct {
Value string `json:"value"`
}
@@ -571,8 +562,8 @@ func (cfg *Config) Execute() (*exec.Cmd, error) {
args = append(args, "-object", fmt.Sprintf("iothread,id=%s", iot.ID))
}
if len(cfg.Drives) == 0 && cfg.KernelImage == "" {
return nil, fmt.Errorf("at least one drive or a kernel image is required")
if len(cfg.Drives) == 0 && len(cfg.Blockdevs) == 0 && cfg.KernelImage == "" {
return nil, fmt.Errorf("at least one drive, blockdev or a kernel image is required")
}
drives := cfg.Drives
@@ -622,18 +613,6 @@ func (cfg *Config) Execute() (*exec.Cmd, error) {
opts = append(opts, fmt.Sprintf("cache=%s", drive.Cache))
}
if drive.Werror != "" {
opts = append(opts, fmt.Sprintf("werror=%s", drive.Werror))
}
if drive.Rerror != "" {
opts = append(opts, fmt.Sprintf("rerror=%s", drive.Rerror))
}
if drive.ReconnectDelay > 0 {
opts = append(opts, fmt.Sprintf("file.reconnect-delay=%d", drive.ReconnectDelay))
}
args = append(args, "-drive", strings.Join(opts, ","))
}
+10 -2
View File
@@ -39,7 +39,7 @@ func TestExecute(t *testing.T) {
// Expect error, at least one drive or kernel image required
assert.Error(t, err)
assert.ErrorContains(t, err, "at least one drive or a kernel image is required")
assert.ErrorContains(t, err, "at least one drive, blockdev or a kernel image is required")
assert.Nil(t, cmd)
cfg.Drives = []Drive{
@@ -347,6 +347,14 @@ func TestExecute_DriveAndKernelInvariant(t *testing.T) {
assert.NotNil(t, cmd)
})
t.Run("blockdevs-only is OK", func(t *testing.T) {
cfg := base
cfg.Blockdevs = []Blockdev{{Value: "driver=raw,node-name=os,file.driver=nbd,file.server.type=unix,file.server.path=/run/os.sock"}}
cmd, err := cfg.Execute()
assert.NoError(t, err)
assert.NotNil(t, cmd)
})
t.Run("kernel-only is OK", func(t *testing.T) {
cfg := base
cfg.KernelImage = "/boot/vmlinuz"
@@ -359,7 +367,7 @@ func TestExecute_DriveAndKernelInvariant(t *testing.T) {
cfg := base
cmd, err := cfg.Execute()
assert.Error(t, err)
assert.ErrorContains(t, err, "at least one drive or a kernel image")
assert.ErrorContains(t, err, "at least one drive, blockdev or a kernel image")
assert.Nil(t, cmd)
})
}
+2 -3
View File
@@ -11,7 +11,6 @@ import (
"time"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/nbd"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/qmp"
)
@@ -143,7 +142,7 @@ func (m *Manager) AttachVolume(ctx context.Context, id, volumeID, device string)
if kind == volumeExportBlock {
blockdevArgs = HostDeviceOpts{Path: ebsRequest.BlockDevice, SizeBytes: ebsRequest.BlockSizeBytes}.QMPArgs(nodeName)
} else {
serverType, socketPath, nbdHost, nbdPort, err := nbd.ParseNBDURI(ebsRequest.NBDURI)
server, err := nbdServerOpts(ebsRequest.NBDURI)
if err != nil {
slog.ErrorContext(ctx, "AttachVolume: failed to parse NBDURI", "uri", ebsRequest.NBDURI, "err", err)
_ = m.rollbackUnmount(ctx, instance.AccountID, ebsRequest)
@@ -152,7 +151,7 @@ func (m *Manager) AttachVolume(ctx context.Context, id, volumeID, device string)
blockdevArgs = map[string]any{
"node-name": nodeName,
"driver": "nbd",
"server": NBDServerOpts{Type: serverType, Path: socketPath, Host: nbdHost, Port: nbdPort}.QMPArg(),
"server": server.QMPArg(),
"export": "",
"read-only": false,
"reconnect-delay": NBDReconnectDelaySeconds,