From 4b29c9dd731aafec70425c0a1a547620c6dc6155 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 6 Jun 2024 10:53:29 -0400 Subject: [PATCH] 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 --- pkg/machine/qemu/command/command.go | 9 -- pkg/machine/qemu/command/qemu_command_test.go | 2 - pkg/machine/qemu/machine.go | 21 +---- pkg/machine/qemu/stubber.go | 90 ++++++------------- 4 files changed, 30 insertions(+), 92 deletions(-) diff --git a/pkg/machine/qemu/command/command.go b/pkg/machine/qemu/command/command.go index 2102a79ab21f..11994f85f1f2 100644 --- 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 index 4860dacaf782..0c70f4af0229 100644 --- 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 index be33d627f30f..64fb04a73da9 100644 --- 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 index 0a0aa15bf631..576d30bbccdb 100644 --- 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 {