machine: Disk.Cache goes; nothing sets it

cmd/spin-machine/commands_test.go

@@ -220,7 +220,7 @@ func TestFlagsAreTheSpec(t *testing.T) {
 	every.Incoming = "file:/s"
 	every.Serial = ""
 	every.Disks = []machine.Disk{{Path: "/d", Format: "raw", Readonly: true, Serial: "ser",
-		Cache: "none", DirectOverBacking: true}}
+		DirectOverBacking: true}}
 	every.Cmdline = machine.DefaultCmdline()
 	every.Cmdline.Init, every.Cmdline.Root, every.Cmdline.RootReadonly = "/bin/sh", "/dev/vdb", true
 	every.Cmdline = every.Cmdline.Profiling()
@@ -245,7 +245,7 @@ func TestFlagsAreTheSpec(t *testing.T) {
 	}{
 		{"nothing given", nil, defaults, true},
 		{"everything given", []string{"--qemu", "/q", "--kernel", "/k", "--initrd", "/i", "--firmware", "/f", "--bios", "bios-256k.bin",
-			"--disk", "/d", "--disk-format", "raw", "--disk-readonly", "--disk-serial", "ser", "--disk-cache", "none",
+			"--disk", "/d", "--disk-format", "raw", "--disk-readonly", "--disk-serial", "ser",
 			"--disk-direct-over-backing", "--accel", "tcg", "--memory", "1024", "--max-memory", "4096", "--cpu", "Skylake-Server-v4",
 			"--cpus", "3", "--max-cpus", "8", "--hotplug-disks", "2",
 			"--vsock-cid", "7", "--qmp", "/qmp", "--incoming", "file:/s", "--console", "", "--init", "/bin/sh",

cmd/spin-machine/main.go

@@ -180,8 +180,7 @@ type machineFlags struct {
 	diskFormat string
 	readonly   bool
 	serial     string
-	// diskCache and directOverBacking are machine.Disk's Cache and DirectOverBacking.
-	diskCache         string
+	// directOverBacking is machine.Disk's DirectOverBacking.
 	directOverBacking bool
 
 	memoryMB     int
@@ -215,7 +214,6 @@ func (o *machineFlags) register(fs *flag.FlagSet) {
 	fs.StringVar(&o.diskFormat, "disk-format", "qcow2", "format of the disk image; never guessed")
 	fs.BoolVar(&o.readonly, "disk-readonly", false, "open the disk read-only, and mount root ro")
 	fs.StringVar(&o.serial, "disk-serial", "", "serial the guest can resolve the disk by")
-	fs.StringVar(&o.diskCache, "disk-cache", "", "QEMU cache mode for the disk (default: QEMU's, which is writeback)")
 	fs.BoolVar(&o.directOverBacking, "disk-direct-over-backing", false,
 		"open the disk O_DIRECT and its backing chain through the host page cache (the disk must have a backing file)")
 
@@ -309,7 +307,6 @@ func (o *machineFlags) spec() (machine.Spec, error) {
 			Format:            o.diskFormat,
 			Readonly:          o.readonly,
 			Serial:            o.serial,
-			Cache:             o.diskCache,
 			DirectOverBacking: o.directOverBacking,
 		}}
 	}

machine/machine.go

@@ -134,9 +134,6 @@ func qemuOpt(v string) string { return strings.ReplaceAll(v, ",", ",,") }
 // nobody here has thought about the safety of.
 var diskFormats = []string{"qcow2", "raw"}
 
-// cacheModes are QEMU's -drive cache= values.
-var cacheModes = []string{"none", "writeback", "writethrough", "directsync", "unsafe"}
-
 // memGrowthID names the memory a virtio-mem device hands out. It is a separate
 // region from pc.ram, the memory the guest boots with: this one is empty until
 // somebody asks for it.
@@ -176,20 +173,6 @@ type Disk struct {
 	// writable disk, and only worth setting when something outside QEMU takes
 	// the same lock to find out whether a VM is running on the image.
 	Locking bool
-	// Cache overrides how the host caches this disk. Empty leaves QEMU's default,
-	// which is what this machine wants and is worth saying why.
-	//
-	// The usual advice for a production VM is cache=none — bypass the host page
-	// cache, because the guest caches the same blocks and holding them twice
-	// wastes RAM. It is the wrong advice here. The read-only base image is one
-	// file that *every* VM on the host maps through a backing chain, so one copy
-	// in the host's page cache is one copy shared by all of them; with O_DIRECT
-	// each VM would fault its own. The overlay is the part that would benefit,
-	// and it shares a -drive with the base.
-	//
-	// cache=none also fails outright on a filesystem with no O_DIRECT — tmpfs,
-	// which is where a scratch overlay usually lands.
-	Cache string
 	// DirectOverBacking opens this image with O_DIRECT and leaves every image
 	// under it in its backing chain on the host page cache: cache.direct=on for
 	// this node, and backing.cache.direct=off, which the deeper backing files
@@ -335,12 +318,14 @@ func encodeNode(n any) string {
 // without one then fails with "Could not open backing file: Must specify either driver
 // or file". A restore that wants every layer left alone declares its chain; see
 // Disk.Chain.
+//
+// No cache=: QEMU's default, writeback, is what this machine wants. The read-only base is
+// one file every VM on the host maps through a backing chain, so one copy in the host's
+// page cache serves all of them, where O_DIRECT would have each VM fault its own; and
+// cache=none fails outright on tmpfs. The overlay alone is DirectOverBacking's to decide.
 func (d Disk) driveArg(i int) string {
 	drive := fmt.Sprintf("file=%s,if=none,id=blk%d,format=%s,aio=io_uring,discard=unmap,file.drop-cache=off",
 		qemuOpt(d.Path), i, d.Format)
-	if d.Cache != "" {
-		drive += ",cache=" + d.Cache
-	}
 	if d.DirectOverBacking {
 		drive += ",cache.direct=on,backing.cache.direct=off"
 	}
@@ -940,14 +925,8 @@ func (d Disk) validate(sets map[int]bool) error {
 		return fmt.Errorf("%s has no format: it is not guessed", d.Path)
 	case d.Path != "" && !slices.Contains(diskFormats, d.Format):
 		return fmt.Errorf("%s: format %q is not one of %v", d.Path, d.Format, diskFormats)
-	case len(d.Chain) != 0 && (d.Cache != "" || d.Format != ""):
-		return errors.New("a chain carries a format per image and takes QEMU's caching")
-	case d.Cache != "" && !slices.Contains(cacheModes, d.Cache):
-		return fmt.Errorf("cache mode %q is not one of %v", d.Cache, cacheModes)
-	// cache= is itself a setting of cache.direct, so the two together are one option
-	// given twice with nothing to say which wins.
-	case d.Cache != "" && d.DirectOverBacking:
-		return fmt.Errorf("cache=%s and DirectOverBacking both decide cache.direct", d.Cache)
+	case len(d.Chain) != 0 && d.Format != "":
+		return errors.New("a chain carries a format per image")
 	}
 	for j, img := range d.Chain {
 		switch {

machine/machine_test.go

@@ -663,6 +663,10 @@ func TestValidateRefuses(t *testing.T) {
 			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 with a format of its own", func(s *Spec) {
+			s.FDSets = []FDSet{{ID: 1, FDs: []FD{{Num: 3}}}}
+			s.Disks = []Disk{{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"}}}}
 		}},
@@ -688,10 +692,6 @@ func TestValidateRefuses(t *testing.T) {
 			s.FDSets = []FDSet{{ID: 1, FDs: []FD{{Num: 3}}}}
 			s.Disks = []Disk{{Chain: []Image{{FDSet: 1, Format: "vmdk"}}}}
 		}},
-		{"a cache mode that is not one", func(s *Spec) { s.Disks = []Disk{{Path: "/a", Format: "raw", Cache: "none,aio=threads"}} }},
-		{"a cache mode and O_DIRECT over backing", func(s *Spec) {
-			s.Disks = []Disk{{Path: "/a", Format: "qcow2", Cache: "none", DirectOverBacking: true}}
-		}},
 		{"a CPU model with options", func(s *Spec) { s.CPU = "Skylake-Server-v4,enforce=off" }},
 		{"an accelerator this machine does not run under", func(s *Spec) { s.Accel = "xen" }},
 		// The kernel command line: each of these boots a guest that is told something

machine/qemu_accepts_test.go

@@ -175,7 +175,7 @@ func TestQEMUAcceptsEveryArgument(t *testing.T) {
 			s.Disks = []Disk{
 				{Path: rawDisk(t, "base.raw"), Format: "raw", Readonly: true, Serial: "base"},
 				{Path: qcow2In(t, qemu, t.TempDir(), "overlay.qcow2", ""), Format: "qcow2",
-					Serial: "overlay", Locking: true, Cache: "writeback"},
+					Serial: "overlay", Locking: true},
 			}
 			return s
 		},