Compare commits

...

5 Commits

Author SHA1 Message Date
brynmailer-mdc 9ef1d1ab8d test(compute): cover an unnamed boot device with no medium in the device map 2026-11-03 10:29:13 +10:00
brynmailer-mdc 2765cdb49f 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-03 10:29:13 +10:00
brynmailer-mdc 98dd8c38ff 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-03 10:29:13 +10:00
brynmailer-mdc 767f54e901 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-03 10:29:13 +10:00
brynmailer-mdc 58ff39e111 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-03 10:29:13 +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 // cmdlineReferencesEndpoint checks a NUL-separated /proc/<pid>/cmdline for
// disc's endpoint. A socket mount's path appears verbatim in whichever // disc's endpoint. A socket mount's path appears verbatim in whichever
// argument named it (QEMU's nbd+unix:///?socket=<path> file= or its // argument named it (an EFI -drive's file= or a -blockdev's server.path=). A
// server.path=<path> -blockdev option). A TCP mount's 127.0.0.1:<port> pair // TCP mount's 127.0.0.1:<port> pair may appear joined in one argument (an EFI
// may appear joined in one argument (a boot/EFI drive's nbd://host:port) or // pflash drive's nbd://host:port) or split across server.host=/server.port=
// split across server.host=/server.port= within the same comma-joined // within one -blockdev argument. The match is by substring, so the boot
// -blockdev argument, so both shapes are checked. // volume's file.server.path=/file.server.host=/file.server.port= also match.
func cmdlineReferencesEndpoint(cmdline []byte, disc discoveredNbdkit) bool { func cmdlineReferencesEndpoint(cmdline []byte, disc discoveredNbdkit) bool {
args := bytes.Split(bytes.TrimRight(cmdline, "\x00"), []byte{0}) args := bytes.Split(bytes.TrimRight(cmdline, "\x00"), []byte{0})
@@ -415,7 +415,7 @@ func TestReapOrphanedNbdkit_LiveReferencer_LeftRunningAndLogged(t *testing.T) {
// TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs proves // TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs proves
// endpointReferencer recognizes a TCP endpoint even when QEMU's -blockdev // endpointReferencer recognizes a TCP endpoint even when QEMU's -blockdev
// splits host and port into server.host=/server.port= within one argument, // 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) { func TestReapOrphanedNbdkit_TCPEndpointReferencedAcrossSplitBlockdevArgs(t *testing.T) {
pid := spawnLongRunningProcess(t) pid := spawnLongRunningProcess(t)
procRoot := t.TempDir() 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") 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 // unreadableProcDir creates procRoot/<pid> with a cmdline file inside, then
// strips all permissions from the directory itself, so os.ReadFile on the // strips all permissions from the directory itself, so os.ReadFile on the
// cmdline path inside it fails with a permission error rather than // 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 name := dev.Device
pciIndex := extractPCIIndex(dev.QDev) 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 { if pciIndex < 0 {
// Hot-plugged device: extract name and port from QDev path. // Hot-plugged device: extract name and port from QDev path.
@@ -127,6 +127,23 @@ func TestBuildDeviceMap(t *testing.T) {
}, },
wantLen: 4, 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; // Boot devices returned out of order: lowest PCI index wins /dev/vda;
// hot-plugged devices sort after boot devices. // 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), 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") "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" "slices"
"strconv" "strconv"
"strings" "strings"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/nbd"
) )
// IsMicrovm reports whether machineType is the QEMU microvm machine, with or // IsMicrovm reports whether machineType is the QEMU microvm machine, with or
@@ -162,12 +164,14 @@ type blkDeviceJSON struct {
NumQueues int `json:"num-queues,omitempty"` NumQueues int `json:"num-queues,omitempty"`
BootIndex int `json:"bootindex,omitempty"` BootIndex int `json:"bootindex,omitempty"`
Mapping []iothreadVQMapping `json:"iothread-vq-mapping"` Mapping []iothreadVQMapping `json:"iothread-vq-mapping"`
Werror string `json:"werror"`
Rerror string `json:"rerror"`
} }
// BlkDeviceMapped returns a virtio-blk -device that spreads its virtqueues // BlkDeviceMapped returns a virtio-blk -device that spreads its virtqueues
// across a pool of poolSize IOThreads, round-robin. Falls back to BlkDevice's // 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 // scalar iothread= form when poolSize <= 1, when there is one queue, or on an
// byte-for-byte what it was before this existed. // MMIO machine.
// //
// Every queue is mapped exactly once and the scalar iothread= property is // Every queue is mapped exactly once and the scalar iothread= property is
// absent: QEMU treats the two as alternative assignments and rejects both. // 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, NumQueues: queues,
BootIndex: bootIdx, BootIndex: bootIdx,
Mapping: mapping, Mapping: mapping,
Werror: "stop",
Rerror: "stop",
}) })
if err != nil { if err != nil {
return Device{}, fmt.Errorf("marshal virtio-blk iothread-vq-mapping: %w", err) 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 return Device{Value: string(b)}, nil
} }
// BlkDevice returns the appropriate QEMU virtio-blk device string for // BlkDevice returns the virtio-blk device string for machineType, omitting an
// machineType. iothread and queues are omitted when empty/zero. bootIdx is // empty iothread or zero queues; bootIdx is PCI-only. It sets werror/rerror=stop
// only emitted for PCI machines (MMIO has no bootindex concept). // (see VolumeBlkDeviceArgs), as a -blockdev node has no BlockBackend to hold it.
func BlkDevice(machineType, drive, iothread string, queues int, bootIdx int) Device { func BlkDevice(machineType, drive, iothread string, queues int, bootIdx int) Device {
var b strings.Builder var b strings.Builder
if IsMMIO(machineType) { if IsMMIO(machineType) {
@@ -224,6 +230,7 @@ func BlkDevice(machineType, drive, iothread string, queues int, bootIdx int) Dev
if !IsMMIO(machineType) && bootIdx > 0 { if !IsMMIO(machineType) && bootIdx > 0 {
fmt.Fprintf(&b, ",bootindex=%d", bootIdx) fmt.Fprintf(&b, ",bootindex=%d", bootIdx)
} }
b.WriteString(",werror=stop,rerror=stop")
return Device{Value: b.String()} 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 // (rendered as a joined command-line string) so the two block-graph shapes
// cannot diverge. // cannot diverge.
// //
// werror/rerror mirror the boot drive's on-error policy (see Drive.Werror): // werror=stop,rerror=stop pauses the VM and holds a failed request, so a
// a data volume has no -drive-backed BlockBackend to carry it, so the qdev // transient backend loss is resumed with cont instead of reaching the guest as
// device itself sets werror=stop,rerror=stop. // EIO. A -blockdev node has no BlockBackend to carry it, so the device sets it.
func VolumeBlkDeviceArgs(volumeID, nodeName, iothreadID, bus string) []string { func VolumeBlkDeviceArgs(volumeID, nodeName, iothreadID, bus string) []string {
return []string{ return []string{
fmt.Sprintf("id=%s", VolumeDeviceID(volumeID)), 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, // monitor-owned node that blockdev-del can later remove (unlike -drive,
// which only creates a BlockBackend with an auto-generated node-name). // which only creates a BlockBackend with an auto-generated node-name).
func VolumeBlockdev(nodeName string, server NBDServerOpts) Blockdev { func VolumeBlockdev(nodeName string, server NBDServerOpts) Blockdev {
opts := append([]string{"driver=nbd", fmt.Sprintf("node-name=%s", nodeName)}, server.CommandLineArgs()...) opts := append([]string{"driver=nbd", fmt.Sprintf("node-name=%s", nodeName)}, nbdNodeOpts("", server)...)
opts = append(opts, "export=", fmt.Sprintf("reconnect-delay=%d", NBDReconnectDelaySeconds))
return Blockdev{Value: strings.Join(opts, ",")} 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 // 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 // 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. // 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 // 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 { func VolumeHostDeviceBlockdev(nodeName string, dev HostDeviceOpts) Blockdev {
return Blockdev{Value: fmt.Sprintf( 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", "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) { func TestBlkDevice_PCI(t *testing.T) {
d := BlkDevice("q35", "os", "ioth-os", 4, 1) 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) { func TestBlkDevice_PCI_NoBootIdx(t *testing.T) {
d := BlkDevice("q35", "data", "ioth-data", 2, 0) 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) { func TestBlkDevice_PCI_NoIOThread(t *testing.T) {
d := BlkDevice("q35", "data", "", 0, 0) 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) { func TestBlkDevice_MMIO(t *testing.T) {
d := BlkDevice("microvm", "os", "ioth-os", 4, 1) d := BlkDevice("microvm", "os", "ioth-os", 4, 1)
// MMIO has no bootindex // 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) { func TestBlkDevice_MMIO_NoIOThread(t *testing.T) {
d := BlkDevice("microvm,x-option-roms=off", "data", "", 0, 0) 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) { 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) m.initENIRequests(instance)
instance.EBSRequests.Mu.Lock() 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() instance.EBSRequests.Mu.Unlock()
if err != nil { if err != nil {
return err return err
@@ -1398,16 +1398,24 @@ func directBootLaunchConfig(base Config, requests []EBSRequest, cpuCount int) (C
return cfg, nil 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 // buildDrives converts EBS requests into QEMU drive/iothread/device/blockdev
// configs. Returns an error if any volume has no valid export (see // configs. Returns an error if any volume has no valid export (see
// classifyVolumeExport), or if the EBS hot-plug port pool is exhausted while // classifyVolumeExport), or if the EBS hot-plug port pool is exhausted while
// allocating one for a data volume. // allocating one for a data volume.
// //
// Boot volumes keep the existing anonymous if=none -drive shape exactly as // The boot volume is a -blockdev node named "os" under a bootindex=1 device
// before — they are not detachable (see ErrVolumeNotDetachable), so the // that carries werror/rerror; it is never detached, so it keeps fixed names.
// anonymous-node problem this function otherwise fixes does not apply, and // EFI volumes emit pflash unit=1; Config.Execute adds the CODE blob (unit=0).
// changing the boot drive risks boot order. EFI volumes emit pflash unit=1;
// the readonly CODE blob (unit=0) is added by Config.Execute.
// //
// Every other (data) volume is given the same named node-name/device-id/ // 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 // 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 { switch {
case kind == volumeExportBlock && (v.Boot || v.EFI): case kind == volumeExportBlock && v.EFI:
// Boot and EFI keep the legacy -drive shape, which has no // The VARS store is a pflash -drive, which has no host_device form.
// host_device form here; only data volumes take a block export. return driveConfig{}, fmt.Errorf("volume %s: block device export is not supported for an EFI volume", v.Name)
return driveConfig{}, fmt.Errorf("volume %s: block device export is only supported for data volumes", v.Name)
case v.Boot: case v.Boot:
drive := Drive{ blockdev, err := exportBlockdev(*v, kind, bootNodeName, bootNBDBlockdev)
File: v.NBDURI, if err != nil {
Format: "raw", return driveConfig{}, err
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,
} }
// The boot disk's virtqueues are spread across a pool rather than // 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 // all served by one host thread. A pool of 1 keeps the scalar
// command line this used to, which is every guest below 4 vCPUs. // iothread= form, which is every guest below 4 vCPUs.
pool := BlkIOThreadPoolSize(cpuCount) pool := BlkIOThreadPoolSize(cpuCount)
for i := range pool { for i := range pool {
cfg.IOThreads = append(cfg.IOThreads, IOThread{ID: BlkIOThreadID("ioth-os", i, 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 { if err != nil {
return driveConfig{}, fmt.Errorf("boot volume %s: %w", v.Name, err) return driveConfig{}, fmt.Errorf("boot volume %s: %w", v.Name, err)
} }
cfg.Blockdevs = append(cfg.Blockdevs, blockdev)
cfg.Devices = append(cfg.Devices, dev) cfg.Devices = append(cfg.Devices, dev)
cfg.Drives = append(cfg.Drives, drive)
case v.EFI: case v.EFI:
cfg.Drives = append(cfg.Drives, Drive{ cfg.Drives = append(cfg.Drives, Drive{
@@ -1495,20 +1491,9 @@ func buildDrives(requests []EBSRequest, cpuCount int, machineType string) (drive
nodeName := VolumeNodeName(v.Name) nodeName := VolumeNodeName(v.Name)
iothreadID := VolumeIOThreadID(v.Name) iothreadID := VolumeIOThreadID(v.Name)
var blockdev Blockdev blockdev, err := exportBlockdev(*v, kind, nodeName, VolumeBlockdev)
if kind == volumeExportBlock { if err != nil {
blockdev = VolumeHostDeviceBlockdev(nodeName, HostDeviceOpts{Path: v.BlockDevice, SizeBytes: v.BlockSizeBytes}) return driveConfig{}, err
} 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,
})
} }
cfg.IOThreads = append(cfg.IOThreads, IOThread{ID: iothreadID}) 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}, {Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
}, },
cpuCount: 2, cpuCount: 2,
wantDrives: []Drive{ wantBlockdevs: []Blockdev{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds}, {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"}}, wantIOThreads: []IOThread{{ID: "ioth-os"}},
wantDevices: []Device{ 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}, {Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
}, },
cpuCount: 4, cpuCount: 4,
wantDrives: []Drive{ wantBlockdevs: []Blockdev{
{File: "nbd:unix:/tmp/boot.sock", Format: "raw", If: "none", Media: "disk", ID: "os", Cache: "none", Werror: "stop", Rerror: "stop", ReconnectDelay: NBDReconnectDelaySeconds}, {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"}}, wantIOThreads: []IOThread{{ID: "ioth-os-0"}, {ID: "ioth-os-1"}},
wantDevices: []Device{ 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", 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{ requests: []EBSRequest{
{Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true}, {Name: "vol-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
{Name: "vol-efi", NBDURI: "nbd:unix:/tmp/efi.sock", EFI: 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. // unaffected by the pool either way — they keep one IOThread each.
cpuCount: 2, cpuCount: 2,
wantDrives: []Drive{ 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}, {File: "nbd:unix:/tmp/efi.sock", Format: "raw", If: "pflash", Unit: 1},
}, },
wantIOThreads: []IOThread{ wantIOThreads: []IOThread{
@@ -268,10 +267,11 @@ func TestBuildDrives(t *testing.T) {
{ID: "ioth-vol-data-a"}, {ID: "ioth-vol-data-a"},
}, },
wantBlockdevs: []Blockdev{ 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"}, {Value: "driver=nbd,node-name=nbd-vol-data-a,server.type=unix,server.path=/tmp/data-a.sock,export=,reconnect-delay=30"},
}, },
wantDevices: []Device{ 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"}, {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}, 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-boot", NBDURI: "nbd:unix:/tmp/boot.sock", Boot: true},
{Name: "vol-blk", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20}, {Name: "vol-blk", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20},
}, },
cpuCount: 2, 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},
},
wantIOThreads: []IOThread{{ID: "ioth-os"}, {ID: "ioth-vol-blk"}}, wantIOThreads: []IOThread{{ID: "ioth-os"}, {ID: "ioth-vol-blk"}},
wantBlockdevs: []Blockdev{ 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"}, {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{ 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"}, {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}, wantHotplugPorts: []int{0, 1},
}, },
{ {
name: "block boot volume is rejected", 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: 1 << 20, Boot: true}}, 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, 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", name: "block EFI volume is rejected",
requests: []EBSRequest{{Name: "vol-efi", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20, EFI: true}}, requests: []EBSRequest{{Name: "vol-efi", BlockDevice: "/dev/sdx", BlockSizeBytes: 1 << 20, EFI: true}},
cpuCount: 2, 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", name: "both NBDURI and BlockDevice set is rejected",
@@ -1109,6 +1150,7 @@ func TestStartQEMU_DiskBoot_MgmtTapCreated(t *testing.T) {
InstanceType: "t3.nano", InstanceType: "t3.nano",
MgmtMAC: "02:aa:bb:cc:dd:ee", 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) 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) 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 { func deviceValues(devs []Device) []string {
out := make([]string, len(devs)) out := make([]string, len(devs))
for i, d := range devs { for i, d := range devs {
+7 -28
View File
@@ -371,8 +371,8 @@ type Device struct {
Value string `json:"value"` Value string `json:"value"`
} }
// Drive is one QEMU -drive entry, used for boot, EFI and pflash firmware // Drive is one QEMU -drive entry, used for the pflash firmware drives (the
// drives. Each set field becomes a key=value option. // CODE blob and the EFI volume). Each set field becomes a key=value option.
type Drive struct { type Drive struct {
File string `json:"file"` File string `json:"file"`
Format string `json:"format"` Format string `json:"format"`
@@ -380,15 +380,6 @@ type Drive struct {
Media string `json:"media"` Media string `json:"media"`
ID string `json:"id"` ID string `json:"id"`
Cache string `json:"cache,omitempty"` 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, // 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. // 1 is the per-VM VARS volume. Ignored for non-pflash drives.
Unit int `json:"unit,omitempty"` 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 is a QEMU -blockdev entry: the command-line equivalent of the
// blockdev-add QMP command. Unlike a -drive with if=none, it produces a // blockdev-add QMP command. Unlike a -drive with if=none, it produces a
// monitor-owned node addressable by node-name, which is what makes // 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) // blockdev-del able to remove it later. Used for boot and data volumes, so a
// volumes so a cold-booted one has the same detachable block-graph shape as // cold-booted data volume has the same detachable block-graph shape as a
// a hot-attached one; boot/EFI volumes keep the legacy -drive shape. // hot-attached one; EFI volumes keep the pflash -drive shape.
type Blockdev struct { type Blockdev struct {
Value string `json:"value"` 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)) args = append(args, "-object", fmt.Sprintf("iothread,id=%s", iot.ID))
} }
if len(cfg.Drives) == 0 && cfg.KernelImage == "" { if len(cfg.Drives) == 0 && len(cfg.Blockdevs) == 0 && cfg.KernelImage == "" {
return nil, fmt.Errorf("at least one drive or a kernel image is required") return nil, fmt.Errorf("at least one drive, blockdev or a kernel image is required")
} }
drives := cfg.Drives drives := cfg.Drives
@@ -622,18 +613,6 @@ func (cfg *Config) Execute() (*exec.Cmd, error) {
opts = append(opts, fmt.Sprintf("cache=%s", drive.Cache)) 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, ",")) 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 // Expect error, at least one drive or kernel image required
assert.Error(t, err) 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) assert.Nil(t, cmd)
cfg.Drives = []Drive{ cfg.Drives = []Drive{
@@ -347,6 +347,14 @@ func TestExecute_DriveAndKernelInvariant(t *testing.T) {
assert.NotNil(t, cmd) 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) { t.Run("kernel-only is OK", func(t *testing.T) {
cfg := base cfg := base
cfg.KernelImage = "/boot/vmlinuz" cfg.KernelImage = "/boot/vmlinuz"
@@ -359,7 +367,7 @@ func TestExecute_DriveAndKernelInvariant(t *testing.T) {
cfg := base cfg := base
cmd, err := cfg.Execute() cmd, err := cfg.Execute()
assert.Error(t, err) 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) assert.Nil(t, cmd)
}) })
} }
+2 -3
View File
@@ -11,7 +11,6 @@ import (
"time" "time"
"github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/nbd"
"github.com/mulgadc/spinifex/spinifex/runtime/compute/qmp" "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 { if kind == volumeExportBlock {
blockdevArgs = HostDeviceOpts{Path: ebsRequest.BlockDevice, SizeBytes: ebsRequest.BlockSizeBytes}.QMPArgs(nodeName) blockdevArgs = HostDeviceOpts{Path: ebsRequest.BlockDevice, SizeBytes: ebsRequest.BlockSizeBytes}.QMPArgs(nodeName)
} else { } else {
serverType, socketPath, nbdHost, nbdPort, err := nbd.ParseNBDURI(ebsRequest.NBDURI) server, err := nbdServerOpts(ebsRequest.NBDURI)
if err != nil { if err != nil {
slog.ErrorContext(ctx, "AttachVolume: failed to parse NBDURI", "uri", ebsRequest.NBDURI, "err", err) slog.ErrorContext(ctx, "AttachVolume: failed to parse NBDURI", "uri", ebsRequest.NBDURI, "err", err)
_ = m.rollbackUnmount(ctx, instance.AccountID, ebsRequest) _ = 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{ blockdevArgs = map[string]any{
"node-name": nodeName, "node-name": nodeName,
"driver": "nbd", "driver": "nbd",
"server": NBDServerOpts{Type: serverType, Path: socketPath, Host: nbdHost, Port: nbdPort}.QMPArg(), "server": server.QMPArg(),
"export": "", "export": "",
"read-only": false, "read-only": false,
"reconnect-delay": NBDReconnectDelaySeconds, "reconnect-delay": NBDReconnectDelaySeconds,