machine/linux: Switch to virtiofs by default

Switch to using virtiofs by default, and delete the 9p code. This is structured as a separate patch to make it easier to revert if need be. Signed-off-by: Colin Walters <[email protected]>
cgwalters · Jun 7, 2024 · 4b29c9d · 4b29c9d
1 parent bf541c6
commit 4b29c9d
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 92 deletions.
diff --git a/pkg/machine/qemu/command/command.go b/pkg/machine/qemu/command/command.go
@@ -99,15 +99,6 @@ func (q *QemuCmd) SetSerialPort(readySocket, vmPidFile define.VMFile, name strin
 		"-pidfile", vmPidFile.GetPath())
 }
 
-// SetVirtfsMount adds a virtfs mount to the machine
-func (q *QemuCmd) SetVirtfsMount(source, tag, securityModel string, readonly bool) {
-	virtfsOptions := fmt.Sprintf("local,path=%s,mount_tag=%s,security_model=%s", source, tag, securityModel)
-	if readonly {
-		virtfsOptions += ",readonly"
-	}
-	*q = append(*q, "-virtfs", virtfsOptions)
-}
-
 // SetBootableImage specifies the image the machine will use to boot
 func (q *QemuCmd) SetBootableImage(image string) {
 	*q = append(*q, "-drive", "if=virtio,file="+image)

diff --git a/pkg/machine/qemu/command/qemu_command_test.go b/pkg/machine/qemu/command/qemu_command_test.go
@@ -47,7 +47,6 @@ func TestQemuCmd(t *testing.T) {
 	err = cmd.SetNetwork(vlanSocket)
 	assert.NoError(t, err)
 	cmd.SetSerialPort(*readySocket, *vmPidFile, "test-machine")
-	cmd.SetVirtfsMount("/tmp/path", "vol10", "none", true)
 	cmd.SetBootableImage(bootableImagePath)
 	cmd.SetDisplay("none")
 
@@ -65,7 +64,6 @@ func TestQemuCmd(t *testing.T) {
 		"-chardev", fmt.Sprintf("socket,path=%s,server=on,wait=off,id=atest-machine_ready", readySocketPath),
 		"-device", "virtserialport,chardev=atest-machine_ready,name=org.fedoraproject.port.0",
 		"-pidfile", vmPidFilePath,
-		"-virtfs", "local,path=/tmp/path,mount_tag=vol10,security_model=none,readonly",
 		"-drive", fmt.Sprintf("if=virtio,file=%s", bootableImagePath),
 		"-display", "none"}
 

diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go
@@ -24,27 +24,8 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-const (
-	MountType9p       = "9p"
-	MountTypeVirtiofs = "virtiofs"
-)
-
 func NewStubber() (*QEMUStubber, error) {
-	var mountType string
-	if v, ok := os.LookupEnv("PODMAN_MACHINE_VIRTFS"); ok {
-		logrus.Debugf("using virtiofs %q", v)
-		switch v {
-		case MountType9p, MountTypeVirtiofs:
-			mountType = v
-		default:
-			return nil, fmt.Errorf("failed to parse PODMAN_MACHINE_VIRTFS=%s", v)
-		}
-	} else {
-		mountType = MountType9p
-	}
-	return &QEMUStubber{
-		mountType: mountType,
-	}, nil
+	return &QEMUStubber{}, nil
 }
 
 // qemuPid returns -1 or the PID of the running QEMU instance.

diff --git a/pkg/machine/qemu/stubber.go b/pkg/machine/qemu/stubber.go
@@ -32,8 +32,6 @@ type QEMUStubber struct {
 	// Command describes the final QEMU command line
 	Command command.QemuCmd
 
-	// mountType is one of virtiofs or 9p
-	mountType string
 	// virtiofsHelpers are virtiofsd child processes
 	virtiofsHelpers []virtiofsdHelperCmd
 }
@@ -166,31 +164,22 @@ func (q *QEMUStubber) StartVM(mc *vmconfigs.MachineConfig) (func() error, func()
 	defer dnr.Close()
 	defer dnw.Close()
 
-	if q.mountType == MountTypeVirtiofs {
-		runtime, err := mc.RuntimeDir()
-		if err != nil {
-			return nil, nil, err
-		}
-		spawner, err := newVirtiofsdSpawner(runtime)
-		if err != nil {
-			return nil, nil, err
-		}
+	runtime, err := mc.RuntimeDir()
+	if err != nil {
+		return nil, nil, err
+	}
+	spawner, err := newVirtiofsdSpawner(runtime)
+	if err != nil {
+		return nil, nil, err
+	}
 
-		for _, hostmnt := range mc.Mounts {
-			qemuArgs, virtiofsdHelper, err := spawner.spawnForMount(hostmnt)
-			if err != nil {
-				return nil, nil, fmt.Errorf("failed to init virtiofsd for mount %s: %w", hostmnt.Source, err)
-			}
-			q.Command = append(q.Command, qemuArgs...)
-			q.virtiofsHelpers = append(q.virtiofsHelpers, *virtiofsdHelper)
-		}
-	} else {
-		// Add volumes to qemu command line
-		for _, mount := range mc.Mounts {
-			// the index provided in this case is thrown away
-			_, _, _, _, securityModel := vmconfigs.SplitVolume(0, mount.OriginalInput)
-			q.Command.SetVirtfsMount(mount.Source, mount.Tag, securityModel, mount.ReadOnly)
+	for _, hostmnt := range mc.Mounts {
+		qemuArgs, virtiofsdHelper, err := spawner.spawnForMount(hostmnt)
+		if err != nil {
+			return nil, nil, fmt.Errorf("failed to init virtiofsd for mount %s: %w", hostmnt.Source, err)
 		}
+		q.Command = append(q.Command, qemuArgs...)
+		q.virtiofsHelpers = append(q.virtiofsHelpers, *virtiofsdHelper)
 	}
 
 	cmdLine := q.Command
@@ -362,48 +351,27 @@ func (q *QEMUStubber) MountVolumesToVM(mc *vmconfigs.MachineConfig, quiet bool)
 		if err != nil {
 			return err
 		}
-		// NOTE: We ignore q.Type here (and it should eventually be dropped from being serialized); we
-		// want the mount type to be dynamic, not static.
-		switch q.mountType {
-		case MountType9p:
-			mountOptions := []string{"-t", "9p"}
-			mountOptions = append(mountOptions, []string{"-o", "trans=virtio", mount.Tag, mount.Target}...)
-			mountOptions = append(mountOptions, []string{"-o", "version=9p2000.L,msize=131072,cache=mmap"}...)
-			if mount.ReadOnly {
-				mountOptions = append(mountOptions, []string{"-o", "ro"}...)
-			}
-			err = machine.CommonSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, append([]string{"sudo", "mount"}, mountOptions...))
-			if err != nil {
-				return err
-			}
-		case MountTypeVirtiofs:
-			mountOptions := []string{"-t", "virtiofs"}
-			mountOptions = append(mountOptions, []string{mount.Tag, mount.Target}...)
-			mountFlags := fmt.Sprintf("context=\"%s\"", machine.NFSSELinuxContext)
-			if mount.ReadOnly {
-				mountFlags += ",ro"
-			}
-			mountOptions = append(mountOptions, "-o", mountFlags)
-			err = machine.CommonSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, append([]string{"sudo", "mount"}, mountOptions...))
-			if err != nil {
-				return err
-			}
-		default:
-			return fmt.Errorf("unknown mount type: %s", mount.Type)
+		// NOTE: The mount type q.Type was previously serialized as 9p for older Linux versions,
+		// but we ignore it now because we want the mount type to be dynamic, not static.  Or
+		// in other words we don't want to make people unnecessarily reprovision their machines
+		// to upgrade from 9p to virtiofs.
+		mountOptions := []string{"-t", "virtiofs"}
+		mountOptions = append(mountOptions, []string{mount.Tag, mount.Target}...)
+		mountFlags := fmt.Sprintf("context=\"%s\"", machine.NFSSELinuxContext)
+		if mount.ReadOnly {
+			mountFlags += ",ro"
+		}
+		mountOptions = append(mountOptions, "-o", mountFlags)
+		err = machine.CommonSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, append([]string{"sudo", "mount"}, mountOptions...))
+		if err != nil {
+			return err
 		}
 	}
 	return nil
 }
 
 func (q *QEMUStubber) MountType() vmconfigs.VolumeMountType {
-	switch q.mountType {
-	case MountType9p:
-		return vmconfigs.NineP
-	case MountTypeVirtiofs:
-		return vmconfigs.VirtIOFS
-	default:
-		panic(fmt.Errorf("unexpected mount type %q", q.mountType))
-	}
+	return vmconfigs.VirtIOFS
 }
 
 func (q *QEMUStubber) PostStartNetworking(mc *vmconfigs.MachineConfig, noInfo bool) error {