From 1803737bda2215e81b395e155bcaef81365fcb1e Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Fri, 18 Sep 2026 19:23:29 -0300 Subject: [PATCH 1/3] machine: a disk as a chain of descriptors, and a monitor on one A disk given by path has QEMU open the image and every backing file its header names. So QEMU sees the caller's filesystem, and a header decides what it reads. Disk.Chain is the other form: each image is a descriptor set (Spec.FDSets, -add-fd), and the images are -blockdev nodes, each backed by the next and the last by nothing. QEMU opens nothing by path, and follows no name a header carries. Each descriptor carries an opaque, the caller's path, which query-fdsets gives back. That is how a caller still learns which image a device has open. QMPFD and QMPFD2 put a monitor on a socket the caller already listens on, for a QEMU that may create no socket where the caller would connect. -object monitor-qmp, because QEMU 11.1 warns -mon is deprecated. TestAChainOverDescriptorsIsReadWithoutAPathAndSealedUnderAnOverlay runs in verify:args. It renames the directory away after opening the images, seals the top under an overlay handed over the same way, and reads the opaques back. With the explicit backing reference taken out, QEMU follows the header to a base that is no longer there. One descriptor per image is enough: QEMU 11.1.1 keeps the old top's descriptor when it seals it, rather than reopening it read-only. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019GHRRFvbnZi5q3KSTvyW5a --- Taskfile.yml | 3 +- machine/machine.go | 192 +++++++++++++++++++++++++++++++++++-- machine/machine_test.go | 21 ++++ machine/qemu_chain_test.go | 162 +++++++++++++++++++++++++++++++ 4 files changed, 371 insertions(+), 7 deletions(-) create mode 100644 machine/qemu_chain_test.go diff --git a/Taskfile.yml b/Taskfile.yml index 17cdc1e..61a693d 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -258,7 +258,8 @@ tasks: # a rebuilt QEMU with a device removed would otherwise be met with a cached pass. # -v because what was rewritten for the TCG binary, and what a host without # /dev/vhost-vsock left unchecked, is the part a reader has to see. - - SPIN_MACHINE_OUTPUT={{.OUTPUT_ABS}} go test ./machine -run TestQEMUAcceptsEveryArgument -count=1 -v + # And the chain over descriptors: what QEMU does with it, not only whether it parses. + - SPIN_MACHINE_OUTPUT={{.OUTPUT_ABS}} go test ./machine -run 'TestQEMUAcceptsEveryArgument|TestAChainOverDescriptors' -count=1 -v fingerprint: desc: >- diff --git a/machine/machine.go b/machine/machine.go index 1d6106e..fbddfeb 100644 --- a/machine/machine.go +++ b/machine/machine.go @@ -26,6 +26,7 @@ package machine import ( "crypto/sha256" "encoding/hex" + "encoding/json" "errors" "fmt" "io" @@ -115,8 +116,17 @@ const memGrowthID = "mem.growth" // Disk is one virtio-blk device. type Disk struct { - // Path to the image file. + // Path to the image file. QEMU opens it, and every backing file its header + // names, by path. Set Path or Chain, not both. Path string + // Chain is the image and every image under it, top first, each handed to + // QEMU as a descriptor set (Spec.FDSets): QEMU opens no image by path, and + // follows no backing file a header names — each image's backing is the next + // one here, and the last has none. So a QEMU that is not allowed to see the + // caller's filesystem at all still reads the whole chain, and reads nothing + // a header could point it at. Format and Cache do not apply: each image + // carries its own format, and the chain is cached as QEMU does by default. + Chain []Image // Format as QEMU names it: qcow2 or raw. Required — it is not guessed // from the file name, because a wrong guess is a guest that boots and finds // a disk full of nothing, and because letting QEMU probe the format of a @@ -161,6 +171,87 @@ type Disk struct { DirectOverBacking bool } +// chainArgs is disk i as -blockdev nodes over descriptor sets, the base first: each format +// node is backed by the node below it, and the last by nothing, so QEMU never reads a backing +// name from a header. The top node is blk, as a -drive's id would be, and the rest +// blk-. +// +// The same options as the path form: aio=io_uring, discard=unmap, and the lock only where +// asked for. Every image but the top is read-only, as a backing file is. +// +// JSON, not key=value: "no backing" is JSON null, which key=value cannot say — backing=null +// names a node called null — and an opaque path needs no comma escaping. +func (d Disk) chainArgs(i int) ([]string, error) { + var args []string + for j := len(d.Chain) - 1; j >= 0; j-- { + img := d.Chain[j] + node := fmt.Sprintf("blk%d", i) + if j > 0 { + node = fmt.Sprintf("blk%d-%d", i, j) + } + readOnly := d.Readonly || j > 0 + file := map[string]any{ + "driver": "file", "node-name": node + "-file", + "filename": fmt.Sprintf("/dev/fdset/%d", img.FDSet), + "aio": "io_uring", "discard": "unmap", "read-only": readOnly, + } + if j == 0 && d.Locking { + file["locking"] = "on" + } + if d.DirectOverBacking { + file["cache"] = map[string]any{"direct": j == 0} + } + format := map[string]any{ + "driver": img.Format, "node-name": node, "file": node + "-file", "read-only": readOnly, + } + switch { + case j < len(d.Chain)-1: + format["backing"] = fmt.Sprintf("blk%d-%d", i, j+1) + case img.Format != "raw": + // Or QEMU opens whatever the header names. + format["backing"] = nil + } + for _, n := range []map[string]any{file, format} { + b, err := json.Marshal(n) + if err != nil { + return nil, err + } + args = append(args, "-blockdev", string(b)) + } + } + return args, nil +} + +// Image is one image of a Disk's chain. +type Image struct { + // FDSet is the id of the descriptor set holding the image (Spec.FDSets): one + // descriptor, opened read-write for the top of a writable disk and read-only + // for everything else. Sealing the top under a new overlay (blockdev-snapshot) + // needs nothing more — measured on QEMU 11.1.1, which keeps the old top's + // descriptor rather than reopening it read-only. + FDSet int + // Format as QEMU names it, qcow2 or raw, for the same reasons as Disk.Format. + // Only the last image of a chain may be raw: raw has no backing. + Format string +} + +// FDSet is descriptors QEMU is handed open rather than a path to open: a path +// of the form /dev/fdset/ names the set wherever QEMU takes a file name. +type FDSet struct { + ID int + FDs []FD +} + +// FD is one descriptor in the QEMU process, so 3 or above. +type FD struct { + Num int + // Opaque is what QEMU reports for the descriptor in query-fdsets, and the + // only way back from /dev/fdset/ to what the caller opened: the path, in + // practice, so that a caller asking which image a device has open gets an + // answer it can compare. + Opaque string +} + // NIC is one virtio-net device, backed by a TAP file descriptor the caller has // already opened and passed to the QEMU process. type NIC struct { @@ -328,6 +419,15 @@ type Spec struct { // QMPSocket is a Unix socket path QEMU listens on for QMP. Required to do // anything to a running machine, including shutting it down. QMPSocket string + // QMPFD is QMPSocket already listening, handed to QEMU as a descriptor, for a + // QEMU that may not create a socket where the caller would connect to it. Set + // one or the other; QMPFD2 is the same for QMPSocket2. + QMPFD int + QMPFD2 int + + // FDSets are the descriptor sets the command line names: a Disk's Chain, a + // Serial of the form file:/dev/fdset/. + FDSets []FDSet // QMPSocket2 is a second monitor, on its own socket, for a second thing that drives // this machine. @@ -600,13 +700,62 @@ func (s Spec) Validate() error { case s.HotplugPorts < 0 || s.HotplugPorts > MaxHotplugPorts: return fmt.Errorf("%d root ports for devices arriving later, and the slot range holds %d", s.HotplugPorts, MaxHotplugPorts) + case s.QMPSocket != "" && s.QMPFD != 0, s.QMPSocket2 != "" && s.QMPFD2 != 0: + return fmt.Errorf("a monitor is given both a socket path and a descriptor") + case s.QMPFD != 0 && s.QMPFD < 3, s.QMPFD2 != 0 && s.QMPFD2 < 3: + return fmt.Errorf("a monitor descriptor below 3 is stdin, stdout or stderr") + } + sets, err := s.fdSets() + if err != nil { + return err } for i, d := range s.Disks { - if d.Path == "" { - return fmt.Errorf("disk %d has no path", i) + if err := d.validate(sets); err != nil { + return fmt.Errorf("disk %d: %w", i, err) } - if d.Format == "" { - return fmt.Errorf("disk %d (%s) has no format: it is not guessed", i, d.Path) + } + return nil +} + +// fdSets indexes Spec.FDSets by id, refusing what QEMU would refuse later and less clearly. +func (s Spec) fdSets() (map[int]bool, error) { + sets := make(map[int]bool, len(s.FDSets)) + for _, set := range s.FDSets { + switch { + case sets[set.ID]: + return nil, fmt.Errorf("descriptor set %d is given twice", set.ID) + case len(set.FDs) == 0: + return nil, fmt.Errorf("descriptor set %d holds no descriptor", set.ID) + } + for _, fd := range set.FDs { + if fd.Num < 3 { + return nil, fmt.Errorf("descriptor set %d names descriptor %d, which is stdin, stdout or stderr", set.ID, fd.Num) + } + } + sets[set.ID] = true + } + return sets, nil +} + +func (d Disk) validate(sets map[int]bool) error { + switch { + case d.Path == "" && len(d.Chain) == 0: + return fmt.Errorf("no path and no chain") + case d.Path != "" && len(d.Chain) != 0: + return fmt.Errorf("both a path and a chain: which one the guest reads is not a guess") + case d.Path != "" && d.Format == "": + return fmt.Errorf("%s has no format: it is not guessed", d.Path) + case len(d.Chain) != 0 && (d.Cache != "" || d.Format != ""): + return fmt.Errorf("a chain carries a format per image and takes QEMU's caching") + } + for j, img := range d.Chain { + switch { + case !sets[img.FDSet]: + return fmt.Errorf("image %d is in descriptor set %d, which Spec.FDSets does not have", j, img.FDSet) + case img.Format == "": + return fmt.Errorf("image %d has no format: it is not guessed", j) + case img.Format == "raw" && j != len(d.Chain)-1: + return fmt.Errorf("image %d is raw, which has no backing, and images follow it", j) } } return nil @@ -788,7 +937,16 @@ func (s Spec) Args() ([]string, error) { if d.Serial != "" { dev += ",serial=" + d.Serial } - args = append(args, "-drive", drive, "-device", dev) + if len(d.Chain) != 0 { + chain, err := d.chainArgs(i) + if err != nil { + return nil, err + } + args = append(args, chain...) + } else { + args = append(args, "-drive", drive) + } + args = append(args, "-device", dev) } for i, n := range s.NICs { @@ -825,6 +983,28 @@ func (s Spec) Args() ([]string, error) { args = append(args, "-qmp", fmt.Sprintf("unix:%s,server=on,wait=off", sock)) } + // A monitor on a socket the caller made and listens on: QEMU accepts on the descriptor + // and never needs a place in the filesystem to put a socket. + for i, fd := range []int{s.QMPFD, s.QMPFD2} { + if fd == 0 { + continue + } + // -object monitor-qmp and not -mon, which QEMU 11.1 warns is deprecated. + id := fmt.Sprintf("qmpfd%d", i) + args = append(args, + "-chardev", fmt.Sprintf("socket,id=%s,fd=%d,server=on,wait=off", id, fd), + "-object", fmt.Sprintf("monitor-qmp,id=mon-%s,chardev=%s", id, id)) + } + // Commas are doubled: QEMU splits an option on a single one, and an opaque is a path. + for _, set := range s.FDSets { + for _, fd := range set.FDs { + spec := fmt.Sprintf("fd=%d,set=%d", fd.Num, set.ID) + if fd.Opaque != "" { + spec += ",opaque=" + strings.ReplaceAll(fd.Opaque, ",", ",,") + } + args = append(args, "-add-fd", spec) + } + } // -incoming, in whichever of its two forms this machine was given. Validate has // already refused a spec carrying both. diff --git a/machine/machine_test.go b/machine/machine_test.go index 56baa7a..2ad7dab 100644 --- a/machine/machine_test.go +++ b/machine/machine_test.go @@ -622,6 +622,27 @@ func TestValidateRefuses(t *testing.T) { {"a negative number of root ports", func(s *Spec) { s.HotplugPorts = -1 }}, {"a disk with no path", func(s *Spec) { s.Disks = []Disk{{Format: "qcow2"}} }}, {"a disk with no format", func(s *Spec) { s.Disks = []Disk{{Path: "/a.qcow2"}} }}, + {"a disk with a path and a chain", func(s *Spec) { + s.FDSets = []FDSet{{ID: 1, FDs: []FD{{Num: 3}}}} + s.Disks = []Disk{{Path: "/a.qcow2", Format: "qcow2", Chain: []Image{{FDSet: 1, Format: "qcow2"}}}} + }}, + {"a chain image in a set nobody gave", func(s *Spec) { + s.Disks = []Disk{{Chain: []Image{{FDSet: 9, Format: "qcow2"}}}} + }}, + {"a chain image with no format", func(s *Spec) { + s.FDSets = []FDSet{{ID: 1, FDs: []FD{{Num: 3}}}} + s.Disks = []Disk{{Chain: []Image{{FDSet: 1}}}} + }}, + {"a raw image with images under it", func(s *Spec) { + s.FDSets = []FDSet{{ID: 1, FDs: []FD{{Num: 3}}}, {ID: 2, FDs: []FD{{Num: 4}}}} + s.Disks = []Disk{{Chain: []Image{{FDSet: 1, Format: "raw"}, {FDSet: 2, Format: "qcow2"}}}} + }}, + {"a descriptor set given twice", func(s *Spec) { + s.FDSets = []FDSet{{ID: 1, FDs: []FD{{Num: 3}}}, {ID: 1, FDs: []FD{{Num: 4}}}} + }}, + {"a descriptor set holding stdin", func(s *Spec) { s.FDSets = []FDSet{{ID: 1, FDs: []FD{{Num: 0}}}} }}, + {"a monitor given a path and a descriptor", func(s *Spec) { s.QMPSocket, s.QMPFD = "/qmp", 3 }}, + {"a monitor on stderr", func(s *Spec) { s.QMPFD = 2 }}, // Both forms of -incoming: QEMU takes the flag once, and which source a VM // restores from is not something to guess at on the caller's behalf. {"both forms of -incoming", func(s *Spec) { s.IncomingDefer, s.Incoming = true, "file:/state" }}, diff --git a/machine/qemu_chain_test.go b/machine/qemu_chain_test.go new file mode 100644 index 0000000..a8ac038 --- /dev/null +++ b/machine/qemu_chain_test.go @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: Apache-2.0 + +package machine + +import ( + "encoding/json" + "net" + "os" + "os/exec" + "strings" + "testing" +) + +// A disk handed over as descriptors is read without a path: the chain is opened after the +// directory that held it is gone, so a backing name in a header points nowhere and QEMU +// reads what it was given or nothing. Then the top is sealed under a new overlay handed over +// the same way, with one descriptor per image, and query-fdsets gives back what each +// descriptor was. The monitor is on a socket this test listens on and QEMU was handed, as a +// QEMU that may create no socket needs. +// +// Under the TCG binary, like TestQEMUAcceptsEveryArgument, and skipped where there is none. +func TestAChainOverDescriptorsIsReadWithoutAPathAndSealedUnderAnOverlay(t *testing.T) { + qemu, firmware := qemuTCG(t) + kernel := pvhStub(t) + + dir := diskDir(t) + base := qcow2In(t, qemu, dir, "base.qcow2", "") + top := qcow2In(t, qemu, dir, "top.qcow2", base) + next := qcow2In(t, qemu, dir, "next.qcow2", top) + + open := func(path string, flag int) *os.File { + t.Helper() + f, err := os.OpenFile(path, flag, 0) // #nosec G304 -- a file this test made + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = f.Close() }) + return f + } + // Descriptors 3 onwards, in this order. + files := []*os.File{open(top, os.O_RDWR), open(base, os.O_RDONLY), open(next, os.O_RDWR)} + socket := qmpSocket(t) + listener, err := net.Listen("unix", socket) + if err != nil { + t.Fatal(err) + } + unix := listener.(*net.UnixListener) //nolint:forcetypeassert // Listen("unix") is one + monitor, err := unix.File() + if err != nil { + t.Fatal(err) + } + // QEMU holds the listening socket from here, at the path this test dials: Go unlinks it + // on Close unless told not to. + unix.SetUnlinkOnClose(false) + _ = listener.Close() + files = append(files, monitor) + + // Nothing QEMU could open by name is left where the headers say. + if err := os.Rename(dir, dir+".gone"); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Rename(dir+".gone", dir) }) + + spec := Spec{ + QEMU: qemu, Kernel: kernel, Firmware: firmware, + BootCPUs: 1, Memory: Memory{SizeMB: 512}, Cmdline: DefaultCmdline().String(), + QMPFD: 6, + FDSets: []FDSet{ + {ID: 1, FDs: []FD{{Num: 3, Opaque: top}}}, + {ID: 2, FDs: []FD{{Num: 4, Opaque: base}}}, + {ID: 3, FDs: []FD{{Num: 5, Opaque: next}}}, + }, + Disks: []Disk{{Chain: []Image{{FDSet: 1, Format: "qcow2"}, {FDSet: 2, Format: "qcow2"}}, Serial: "root", Locking: true}}, + } + args, err := spec.Args() + if err != nil { + t.Fatal(err) + } + args, _, err = tcgOnly(args) + if err != nil { + t.Fatal(err) + } + + cmd := exec.Command(qemu, append(args, "-S")...) // #nosec G204 -- the binary and arguments this test built + var out strings.Builder + cmd.Stderr, cmd.Stdout = &out, &out + cmd.ExtraFiles = files + if err := cmd.Start(); err != nil { + t.Fatalf("starting %s: %v", qemu, err) + } + defer func() { + _ = cmd.Process.Kill() + _, _ = cmd.Process.Wait() + }() + + conn := dialQMP(t, socket, &out) + defer func() { _ = conn.Close() }() + enc, dec := json.NewEncoder(conn), json.NewDecoder(conn) + var greeting struct { + QMP *struct{} `json:"QMP"` + } + if err := dec.Decode(&greeting); err != nil || greeting.QMP == nil { + t.Fatalf("no QMP greeting on the handed-over monitor: %v\n\nQEMU said:\n%s", err, out.String()) + } + command := func(what string, req any) json.RawMessage { + t.Helper() + if err := enc.Encode(req); err != nil { + t.Fatalf("sending %s: %v", what, err) + } + for { + var reply struct { + Return json.RawMessage `json:"return"` + Event string `json:"event"` + Error *struct { + Desc string `json:"desc"` + } `json:"error"` + } + if err := dec.Decode(&reply); err != nil { + t.Fatalf("reading the reply to %s: %v\n\nQEMU said:\n%s", what, err, out.String()) + } + if reply.Event != "" { + continue + } + if reply.Error != nil { + t.Fatalf("%s was refused: %s\n\nQEMU said:\n%s", what, reply.Error.Desc, out.String()) + } + return reply.Return + } + } + command("qmp_capabilities", map[string]string{"execute": "qmp_capabilities"}) + + command("blockdev-add next's file", map[string]any{"execute": "blockdev-add", "arguments": map[string]any{ + "driver": "file", "node-name": "next-file", "filename": "/dev/fdset/3", "aio": "io_uring", "locking": "on", + }}) + command("blockdev-add next", map[string]any{"execute": "blockdev-add", "arguments": map[string]any{ + "driver": "qcow2", "node-name": "next", "file": "next-file", "backing": nil, + }}) + command("blockdev-snapshot", map[string]any{"execute": "blockdev-snapshot", "arguments": map[string]any{ + "node": "blk0", "overlay": "next", + }}) + + var sets []struct { + ID int `json:"fdset-id"` + FDs []struct { + Opaque string `json:"opaque"` + } `json:"fds"` + } + if err := json.Unmarshal(command("query-fdsets", map[string]string{"execute": "query-fdsets"}), &sets); err != nil { + t.Fatal(err) + } + opaque := map[int]string{} + for _, s := range sets { + for _, fd := range s.FDs { + opaque[s.ID] = fd.Opaque + } + } + for id, want := range map[int]string{1: top, 2: base, 3: next} { + if opaque[id] != want { + t.Errorf("descriptor set %d says it is %q, want %q", id, opaque[id], want) + } + } +} From b7f1302c8b6c33c99d65963038c864b6fb18af3c Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Fri, 18 Sep 2026 19:29:50 -0300 Subject: [PATCH 2/3] machine: the console on a descriptor set A QEMU that may open nothing by path needs its console handed over too, and the usual console is the write end of a FIFO. "-serial file:/dev/fdset/" fails on one with EINVAL: QEMU truncates what it opens. SerialFDSet puts the console on a descriptor set through "-chardev file,append=on" instead. It counts as a serial port in the fingerprint, as Serial does, so a template taken with one form restores into the other. TestAChainOverDescriptors writes its console into a FIFO this way, and the fingerprint test puts a machine given descriptors beside the same machine given paths. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019GHRRFvbnZi5q3KSTvyW5a --- machine/machine.go | 28 +++++++++++++++++++++++----- machine/machine_test.go | 10 ++++++++-- machine/qemu_chain_test.go | 17 ++++++++++++++--- 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/machine/machine.go b/machine/machine.go index fbddfeb..f304ff9 100644 --- a/machine/machine.go +++ b/machine/machine.go @@ -425,10 +425,17 @@ type Spec struct { QMPFD int QMPFD2 int - // FDSets are the descriptor sets the command line names: a Disk's Chain, a - // Serial of the form file:/dev/fdset/. + // FDSets are the descriptor sets the command line names: a Disk's Chain, + // SerialFDSet. FDSets []FDSet + // SerialFDSet is the console as a descriptor set QEMU writes to, instead of + // Serial, for a QEMU that may open nothing by path. It is opened append-only: + // QEMU otherwise truncates what it opens, and the usual console descriptor is + // a FIFO, which cannot be truncated — "-serial file:/dev/fdset/" fails + // with EINVAL on one (measured, QEMU 11.1.1). + SerialFDSet int + // QMPSocket2 is a second monitor, on its own socket, for a second thing that drives // this machine. // @@ -709,6 +716,12 @@ func (s Spec) Validate() error { if err != nil { return err } + switch { + case s.SerialFDSet != 0 && s.Serial != "": + return fmt.Errorf("a console given both as a chardev and as a descriptor set") + case s.SerialFDSet != 0 && !sets[s.SerialFDSet]: + return fmt.Errorf("the console is descriptor set %d, which Spec.FDSets does not have", s.SerialFDSet) + } for i, d := range s.Disks { if err := d.validate(sets); err != nil { return fmt.Errorf("disk %d: %w", i, err) @@ -970,9 +983,14 @@ func (s Spec) Args() ([]string, error) { "-device", dev) } - if s.Serial != "" { + switch { + case s.SerialFDSet != 0: + args = append(args, + "-chardev", fmt.Sprintf("file,id=serial0,path=/dev/fdset/%d,append=on", s.SerialFDSet), + "-serial", "chardev:serial0") + case s.Serial != "": args = append(args, "-serial", s.Serial) - } else { + default: args = append(args, "-serial", "none") } @@ -1173,7 +1191,7 @@ func (s Spec) topology() string { if s.Memory.MaxMB > s.Memory.SizeMB { fmt.Fprintf(&b, ";virtio-mem-pci@%#x", SlotMem) } - if s.Serial != "" { + if s.Serial != "" || s.SerialFDSet != 0 { b.WriteString(";isa-serial") } // The empty root ports, which are devices present when the state is loaded even diff --git a/machine/machine_test.go b/machine/machine_test.go index 2ad7dab..56a7fed 100644 --- a/machine/machine_test.go +++ b/machine/machine_test.go @@ -441,10 +441,16 @@ func TestFingerprintIgnoresWhatIsBehindTheDevices(t *testing.T) { a.NICs = []NIC{{TapFD: 3, MAC: "52:54:00:00:00:01"}} a.VsockCID = 7 + a.Serial = "file:/var/log/console" + + // The same machine handed its disk, monitor and console as descriptors. b := a - b.Disks = []Disk{{Path: "/another.qcow2", Format: "qcow2", Serial: "bbb"}} + b.FDSets = []FDSet{{ID: 1, FDs: []FD{{Num: 10}}}, {ID: 2, FDs: []FD{{Num: 11}}}} + b.Disks = []Disk{{Chain: []Image{{FDSet: 1, Format: "qcow2"}}, Serial: "bbb"}} b.NICs = []NIC{{TapFD: 9, MAC: "52:54:00:00:00:02"}} b.VsockCID = 42 + b.Serial, b.SerialFDSet = "", 2 + b.QMPSocket, b.QMPFD = "", 12 fa, err := a.Fingerprint() if err != nil { @@ -455,7 +461,7 @@ func TestFingerprintIgnoresWhatIsBehindTheDevices(t *testing.T) { t.Fatal(err) } if fa != fb { - t.Error("different paths, MACs and context ids made it a different machine") + t.Error("different paths, MACs, context ids and descriptors made it a different machine") } } diff --git a/machine/qemu_chain_test.go b/machine/qemu_chain_test.go index a8ac038..060a724 100644 --- a/machine/qemu_chain_test.go +++ b/machine/qemu_chain_test.go @@ -7,7 +7,9 @@ import ( "net" "os" "os/exec" + "path/filepath" "strings" + "syscall" "testing" ) @@ -39,6 +41,13 @@ func TestAChainOverDescriptorsIsReadWithoutAPathAndSealedUnderAnOverlay(t *testi } // Descriptors 3 onwards, in this order. files := []*os.File{open(top, os.O_RDWR), open(base, os.O_RDONLY), open(next, os.O_RDWR)} + // The console, a FIFO as a runner keeps one: its reader first, or opening the writer blocks. + fifo := filepath.Join(t.TempDir(), "console") + if err := syscall.Mkfifo(fifo, 0o600); err != nil { + t.Fatal(err) + } + _ = open(fifo, os.O_RDONLY|syscall.O_NONBLOCK) + files = append(files, open(fifo, os.O_WRONLY)) socket := qmpSocket(t) listener, err := net.Listen("unix", socket) if err != nil { @@ -64,13 +73,15 @@ func TestAChainOverDescriptorsIsReadWithoutAPathAndSealedUnderAnOverlay(t *testi spec := Spec{ QEMU: qemu, Kernel: kernel, Firmware: firmware, BootCPUs: 1, Memory: Memory{SizeMB: 512}, Cmdline: DefaultCmdline().String(), - QMPFD: 6, + QMPFD: 7, FDSets: []FDSet{ {ID: 1, FDs: []FD{{Num: 3, Opaque: top}}}, {ID: 2, FDs: []FD{{Num: 4, Opaque: base}}}, {ID: 3, FDs: []FD{{Num: 5, Opaque: next}}}, + {ID: 4, FDs: []FD{{Num: 6, Opaque: fifo}}}, }, - Disks: []Disk{{Chain: []Image{{FDSet: 1, Format: "qcow2"}, {FDSet: 2, Format: "qcow2"}}, Serial: "root", Locking: true}}, + SerialFDSet: 4, + Disks: []Disk{{Chain: []Image{{FDSet: 1, Format: "qcow2"}, {FDSet: 2, Format: "qcow2"}}, Serial: "root", Locking: true}}, } args, err := spec.Args() if err != nil { @@ -154,7 +165,7 @@ func TestAChainOverDescriptorsIsReadWithoutAPathAndSealedUnderAnOverlay(t *testi opaque[s.ID] = fd.Opaque } } - for id, want := range map[int]string{1: top, 2: base, 3: next} { + for id, want := range map[int]string{1: top, 2: base, 3: next, 4: fifo} { if opaque[id] != want { t.Errorf("descriptor set %d says it is %q, want %q", id, opaque[id], want) } From b162b869eed47c572cf8e7ada1297b3782159291 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Fri, 18 Sep 2026 20:45:21 -0300 Subject: [PATCH 3/3] machine: /dev/kvm, vhost-vsock and vhost-net as descriptors A QEMU whose root holds no /dev can still run a machine: KVMFDSet moves the accelerator to -accel kvm,device=/dev/fdset/N, VsockFD and NIC.VhostFD become vhostfd=. QEMU refuses -accel beside -machine accel= (11.1.1), so the machine argument drops it; the shape, and so the fingerprint, still says accel=kvm. Proven against the release build: /dev/null in the place of either device fails the start, the devices themselves answer on QMP. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_019GHRRFvbnZi5q3KSTvyW5a --- machine/machine.go | 60 ++++++++++++++--- machine/machine_test.go | 55 ++++++++++++++++ machine/qemu_devices_test.go | 122 +++++++++++++++++++++++++++++++++++ 3 files changed, 229 insertions(+), 8 deletions(-) create mode 100644 machine/qemu_devices_test.go diff --git a/machine/machine.go b/machine/machine.go index f304ff9..055b833 100644 --- a/machine/machine.go +++ b/machine/machine.go @@ -257,7 +257,10 @@ type FD struct { type NIC struct { // TapFD is the descriptor number in the QEMU process, so 3 or above. TapFD int - MAC string + // VhostFD is /dev/vhost-net already open, handed to QEMU as a descriptor, for a + // QEMU that may open no device node. Zero has QEMU open the node itself. + VhostFD int + MAC string // MTU is the largest frame the guest may send, announced through // VIRTIO_NET_F_MTU. Zero leaves the guest at its own default, 1500. @@ -415,6 +418,15 @@ type Spec struct { // has no serial port for a caller to drive and no network it is required to // have. VsockCID int + // VsockFD is /dev/vhost-vsock already open, handed to QEMU as a descriptor, for a + // QEMU that may open no device node: one whose root has no /dev. Zero has QEMU open + // the node itself. + VsockFD int + + // KVMFDSet is /dev/kvm as a descriptor set, for the same QEMU. Only under KVM, which + // is the accelerator that opens a device. The shape still says accel=kvm: where the + // accelerator's descriptor came from is not the machine a template is loaded into. + KVMFDSet int // QMPSocket is a Unix socket path QEMU listens on for QMP. Required to do // anything to a running machine, including shutting it down. @@ -612,6 +624,16 @@ func (s Spec) Shape() Shape { } } +// machineArg is the shape's machine string as -machine takes it. With KVMFDSet the +// accelerator moves to its own -accel, which carries the descriptor: QEMU refuses the +// two together — "The -accel and "-machine accel=" options are incompatible" (11.1.1). +func (s Spec) machineArg(shape Shape) string { + if s.KVMFDSet == 0 { + return shape.Machine + } + return strings.Replace(shape.Machine, ",accel=kvm", "", 1) +} + // derivedFromHost reports whether a CPU model takes its feature set from the // silicon it runs on, which is what makes a template built with it unusable on // another machine — and what migratable=on applies to. @@ -721,6 +743,19 @@ func (s Spec) Validate() error { return fmt.Errorf("a console given both as a chardev and as a descriptor set") case s.SerialFDSet != 0 && !sets[s.SerialFDSet]: return fmt.Errorf("the console is descriptor set %d, which Spec.FDSets does not have", s.SerialFDSet) + case s.KVMFDSet != 0 && s.Accel != "" && s.Accel != "kvm": + return fmt.Errorf("a /dev/kvm descriptor for accel=%s, which opens no device", s.Accel) + case s.KVMFDSet != 0 && !sets[s.KVMFDSet]: + return fmt.Errorf("/dev/kvm is descriptor set %d, which Spec.FDSets does not have", s.KVMFDSet) + case s.VsockFD != 0 && s.VsockCID == 0: + return fmt.Errorf("a /dev/vhost-vsock descriptor for a machine with no vsock") + case s.VsockFD != 0 && s.VsockFD < 3: + return fmt.Errorf("a /dev/vhost-vsock descriptor below 3 is stdin, stdout or stderr") + } + for i, n := range s.NICs { + if n.VhostFD != 0 && n.VhostFD < 3 { + return fmt.Errorf("NIC %d: a /dev/vhost-net descriptor below 3 is stdin, stdout or stderr", i) + } } for i, d := range s.Disks { if err := d.validate(sets); err != nil { @@ -798,12 +833,16 @@ func (s Spec) Args() ([]string, error) { // done from outside rather than from inside QEMU. "-sandbox", "on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny", - "-machine", shape.Machine, + "-machine", s.machineArg(shape), "-cpu", shape.CPU, "-smp", shape.SMP, "-m", shape.Memory, } + if s.KVMFDSet != 0 { + args = append(args, "-accel", fmt.Sprintf("kvm,device=/dev/fdset/%d", s.KVMFDSet)) + } + if s.Memory.File != "" { // A restore opens the file read-only. QEMU otherwise opens it read-write even to // map it private, so every VM restored from a template could write the template @@ -884,9 +923,12 @@ func (s Spec) Args() ([]string, error) { virtioModern, SlotBalloon)) if s.VsockCID != 0 { - args = append(args, "-device", - fmt.Sprintf("vhost-vsock-pci,guest-cid=%d,%s,addr=0x%x", - s.VsockCID, virtioModern, SlotVsock)) + dev := fmt.Sprintf("vhost-vsock-pci,guest-cid=%d,%s,addr=0x%x", + s.VsockCID, virtioModern, SlotVsock) + if s.VsockFD != 0 { + dev += fmt.Sprintf(",vhostfd=%d", s.VsockFD) + } + args = append(args, "-device", dev) } // virtio-mem, when this VM is allowed to grow: the region between its boot @@ -978,9 +1020,11 @@ func (s Spec) Args() ([]string, error) { if n.MTU > 0 { dev += fmt.Sprintf(",host_mtu=%d", n.MTU) } - args = append(args, - "-netdev", fmt.Sprintf("tap,id=net%d,fd=%d,vhost=on", i, n.TapFD), - "-device", dev) + netdev := fmt.Sprintf("tap,id=net%d,fd=%d,vhost=on", i, n.TapFD) + if n.VhostFD != 0 { + netdev += fmt.Sprintf(",vhostfd=%d", n.VhostFD) + } + args = append(args, "-netdev", netdev, "-device", dev) } switch { diff --git a/machine/machine_test.go b/machine/machine_test.go index 56a7fed..48a615e 100644 --- a/machine/machine_test.go +++ b/machine/machine_test.go @@ -139,6 +139,50 @@ func TestTheGuestIsToldItsMTUOnTheDevice(t *testing.T) { } } +// A device handed over as a descriptor is named by it on the command line, and one that +// is not is left for QEMU to open: a node that exists only where it runs. +func TestADeviceHandedOverIsNamedByItsDescriptor(t *testing.T) { + for _, tc := range []struct { + name string + given bool + want []string + absent []string + }{ + {"opened by QEMU", false, + []string{"q35,accel=kvm,", "vhost-vsock-pci,guest-cid=7,disable-legacy=on,addr=0x2 ", "tap,id=net0,fd=3,vhost=on "}, + []string{"-accel", "vhostfd"}}, + // -machine loses accel=: QEMU refuses it beside -accel. + {"handed over", true, + []string{"-machine q35,kernel-irqchip=on,", "-accel kvm,device=/dev/fdset/1 ", "addr=0x2,vhostfd=5 ", "tap,id=net0,fd=3,vhost=on,vhostfd=6 "}, + []string{"accel=kvm"}}, + } { + t.Run(tc.name, func(t *testing.T) { + s := spec(t) + s.VsockCID = 7 + s.NICs = []NIC{{TapFD: 3, MAC: "52:54:00:00:00:01"}} + if tc.given { + s.FDSets = []FDSet{{ID: 1, FDs: []FD{{Num: 4}}}} + s.KVMFDSet, s.VsockFD, s.NICs[0].VhostFD = 1, 5, 6 + } + args, err := s.Args() + if err != nil { + t.Fatal(err) + } + joined := strings.Join(args, " ") + " " + for _, want := range tc.want { + if !strings.Contains(joined, want) { + t.Errorf("missing %q in %s", want, joined) + } + } + for _, absent := range tc.absent { + if strings.Contains(joined, absent) { + t.Errorf("%q in %s", absent, joined) + } + } + }) + } +} + // A disk's format is stated, never guessed: a wrong guess is a guest that boots // and finds a disk full of nothing. func TestDiskFormatIsRequired(t *testing.T) { @@ -451,6 +495,9 @@ func TestFingerprintIgnoresWhatIsBehindTheDevices(t *testing.T) { b.VsockCID = 42 b.Serial, b.SerialFDSet = "", 2 b.QMPSocket, b.QMPFD = "", 12 + b.FDSets = append(b.FDSets, FDSet{ID: 3, FDs: []FD{{Num: 13}}}) + b.KVMFDSet, b.VsockFD = 3, 14 + b.NICs[0].VhostFD = 15 fa, err := a.Fingerprint() if err != nil { @@ -649,6 +696,14 @@ func TestValidateRefuses(t *testing.T) { {"a descriptor set holding stdin", func(s *Spec) { s.FDSets = []FDSet{{ID: 1, FDs: []FD{{Num: 0}}}} }}, {"a monitor given a path and a descriptor", func(s *Spec) { s.QMPSocket, s.QMPFD = "/qmp", 3 }}, {"a monitor on stderr", func(s *Spec) { s.QMPFD = 2 }}, + {"/dev/kvm in a set nobody gave", func(s *Spec) { s.KVMFDSet = 1 }}, + {"/dev/kvm for an emulator", func(s *Spec) { + s.Accel, s.KVMFDSet = "tcg", 1 + s.FDSets = []FDSet{{ID: 1, FDs: []FD{{Num: 3}}}} + }}, + {"/dev/vhost-vsock with no vsock", func(s *Spec) { s.VsockCID, s.VsockFD = 0, 3 }}, + {"/dev/vhost-vsock on stdout", func(s *Spec) { s.VsockCID, s.VsockFD = 3, 1 }}, + {"/dev/vhost-net on stderr", func(s *Spec) { s.NICs = []NIC{{TapFD: 3, VhostFD: 2, MAC: "52:54:00:00:00:01"}} }}, // Both forms of -incoming: QEMU takes the flag once, and which source a VM // restores from is not something to guess at on the caller's behalf. {"both forms of -incoming", func(s *Spec) { s.IncomingDefer, s.Incoming = true, "file:/state" }}, diff --git a/machine/qemu_devices_test.go b/machine/qemu_devices_test.go new file mode 100644 index 0000000..2adfc72 --- /dev/null +++ b/machine/qemu_devices_test.go @@ -0,0 +1,122 @@ +// SPDX-License-Identifier: Apache-2.0 + +package machine + +import ( + "cmp" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +// A machine handed /dev/kvm and /dev/vhost-vsock as descriptors uses those and opens +// neither node: given /dev/null in the place of one it fails, and given the devices it +// answers on its monitor. A QEMU whose root has no /dev depends on exactly that. +// +// Under the ordinary build and KVM, the only accelerator that opens a device; skipped on +// a host that will not give both up. +func TestAMachineTakesItsDevicesAsDescriptors(t *testing.T) { + out, err := filepath.Abs(cmp.Or(os.Getenv("SPIN_MACHINE_OUTPUT"), "../_output")) + if err != nil { + t.Fatal(err) + } + qemu := filepath.Join(out, qemuName) + if _, err := os.Stat(qemu); err != nil { + t.Skipf("no QEMU at %s: %v", qemu, err) + } + for _, dev := range []string{"/dev/kvm", "/dev/vhost-vsock"} { + f, err := os.OpenFile(dev, os.O_RDWR, 0) + if err != nil { + t.Skipf("this host will not give up %s: %v", dev, err) + } + _ = f.Close() + } + kernel := pvhStub(t) + + for _, tc := range []struct { + name string + kvm, vsock string + refused bool + }{ + {"both devices", "/dev/kvm", "/dev/vhost-vsock", false}, + {"/dev/null for /dev/kvm", os.DevNull, "/dev/vhost-vsock", true}, + {"/dev/null for /dev/vhost-vsock", "/dev/kvm", os.DevNull, true}, + } { + t.Run(tc.name, func(t *testing.T) { + var files []*os.File + for _, path := range []string{tc.kvm, tc.vsock} { + f, err := os.OpenFile(path, os.O_RDWR, 0) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = f.Close() }) + files = append(files, f) + } + socket := qmpSocket(t) + spec := Spec{ + QEMU: qemu, Kernel: kernel, Firmware: filepath.Join(out, firmwareDir), + BootCPUs: 1, Memory: Memory{SizeMB: 256}, Cmdline: DefaultCmdline().String(), + QMPSocket: socket, + // Descriptors 3 and 4, in the order above. + FDSets: []FDSet{{ID: 1, FDs: []FD{{Num: 3, Opaque: tc.kvm}}}}, + KVMFDSet: 1, + VsockCID: 12346, VsockFD: 4, + } + args, err := spec.Args() + if err != nil { + t.Fatal(err) + } + + cmd := exec.Command(qemu, append(args, "-S")...) // #nosec G204 -- the binary and arguments this test built + var stderr strings.Builder + cmd.Stderr, cmd.Stdout = &stderr, &stderr + cmd.ExtraFiles = files + if err := cmd.Start(); err != nil { + t.Fatalf("starting %s: %v", qemu, err) + } + var exit error + done := make(chan struct{}) + go func() { exit = cmd.Wait(); close(done) }() + defer func() { + _ = cmd.Process.Kill() + <-done + }() + + if tc.refused { + // A QEMU that opened the nodes itself runs, stopped at -S, and is still + // running when this gives up on it. + select { + case <-done: + case <-time.After(20 * time.Second): + t.Fatalf("QEMU is running on %s and %s: it opened the devices itself", tc.kvm, tc.vsock) + } + if exit == nil { + t.Fatalf("QEMU started on %s and %s: it opened the devices itself", tc.kvm, tc.vsock) + } + t.Logf("refused: %s", strings.TrimSpace(stderr.String())) + return + } + conn := dialQMP(t, socket, &stderr) + defer func() { _ = conn.Close() }() + enc, dec := json.NewEncoder(conn), json.NewDecoder(conn) + for _, command := range []string{"", "qmp_capabilities", "query-status"} { + if command != "" { + if err := enc.Encode(map[string]string{"execute": command}); err != nil { + t.Fatal(err) + } + } + var reply map[string]json.RawMessage + if err := dec.Decode(&reply); err != nil { + t.Fatalf("no answer to %q on the monitor: %v\n\nQEMU said:\n%s", command, err, stderr.String()) + } + if e, ok := reply["error"]; ok { + t.Fatalf("%s was refused: %s", command, e) + } + } + }) + } +}