Merge pull request #28823 from inknos/run-4472

machine: share virtiofs systemd unit generation

pkg/machine/apple/apple.go

@@ -17,12 +17,9 @@ import (
 	"github.com/sirupsen/logrus"
 	"go.podman.io/common/pkg/config"
 	"go.podman.io/common/pkg/strongunits"
-	"go.podman.io/podman/v6/pkg/machine"
 	"go.podman.io/podman/v6/pkg/machine/define"
-	"go.podman.io/podman/v6/pkg/machine/ignition"
 	"go.podman.io/podman/v6/pkg/machine/sockets"
 	"go.podman.io/podman/v6/pkg/machine/vmconfigs"
-	"go.podman.io/podman/v6/pkg/systemd/parser"
 )
 
 const applehvMACAddress = "5a:94:ef:e4:0c:ee"
@@ -65,80 +62,6 @@ func SetProviderAttrs(mc *vmconfigs.MachineConfig, opts define.SetOptions, state
 	return nil
 }
 
-func GenerateSystemDFilesForVirtiofsMounts(mounts []machine.VirtIoFs) ([]ignition.Unit, error) {
-	// mounting in fcos with virtiofs is a bit of a dance.  we need a unit file for the mount, a unit file
-	// for automatic mounting on boot, and a "preparatory" service file that disables FCOS security, performs
-	// the mkdir of the mount point, and then re-enables security.  This must be done for each mount.
-
-	unitFiles := make([]ignition.Unit, 0, len(mounts))
-	for _, mnt := range mounts {
-		// Create mount unit for each mount
-		mountUnit := parser.NewUnitFile()
-		mountUnit.Add("Mount", "What", "%s")
-		mountUnit.Add("Mount", "Where", "%s")
-		mountUnit.Add("Mount", "Type", "virtiofs")
-		mountUnit.Add("Mount", "Options", fmt.Sprintf("context=\"%s\"", machine.NFSSELinuxContext))
-		mountUnit.Add("Install", "WantedBy", "local-fs.target")
-		mountUnitFile, err := mountUnit.ToString()
-		if err != nil {
-			return nil, err
-		}
-
-		virtiofsMount := ignition.Unit{
-			Enabled:  ignition.BoolToPtr(true),
-			Name:     fmt.Sprintf("%s.mount", parser.PathEscape(mnt.Target)),
-			Contents: ignition.StrToPtr(fmt.Sprintf(mountUnitFile, mnt.Tag, mnt.Target)),
-		}
-
-		unitFiles = append(unitFiles, virtiofsMount)
-	}
-
-	// This is a way to workaround the FCOS limitation of creating directories
-	// at the rootfs / and then mounting to them.
-	immutableRootOff := parser.NewUnitFile()
-	immutableRootOff.Add("Unit", "Description", "Allow systemd to create mount points on /")
-	immutableRootOff.Add("Unit", "DefaultDependencies", "no")
-
-	immutableRootOff.Add("Service", "Type", "oneshot")
-	immutableRootOff.Add("Service", "ExecStart", "chattr -i /")
-
-	immutableRootOff.Add("Install", "WantedBy", "local-fs-pre.target")
-	immutableRootOffFile, err := immutableRootOff.ToString()
-	if err != nil {
-		return nil, err
-	}
-
-	immutableRootOffUnit := ignition.Unit{
-		Contents: ignition.StrToPtr(immutableRootOffFile),
-		Name:     "immutable-root-off.service",
-		Enabled:  ignition.BoolToPtr(true),
-	}
-	unitFiles = append(unitFiles, immutableRootOffUnit)
-
-	immutableRootOn := parser.NewUnitFile()
-	immutableRootOn.Add("Unit", "Description", "Set / back to immutable after mounts are done")
-	immutableRootOn.Add("Unit", "DefaultDependencies", "no")
-	immutableRootOn.Add("Unit", "After", "local-fs.target")
-
-	immutableRootOn.Add("Service", "Type", "oneshot")
-	immutableRootOn.Add("Service", "ExecStart", "chattr +i /")
-
-	immutableRootOn.Add("Install", "WantedBy", "local-fs.target")
-	immutableRootOnFile, err := immutableRootOn.ToString()
-	if err != nil {
-		return nil, err
-	}
-
-	immutableRootOnUnit := ignition.Unit{
-		Contents: ignition.StrToPtr(immutableRootOnFile),
-		Name:     "immutable-root-on.service",
-		Enabled:  ignition.BoolToPtr(true),
-	}
-	unitFiles = append(unitFiles, immutableRootOnUnit)
-
-	return unitFiles, nil
-}
-
 // StartGenericAppleVM is wrapped by apple provider methods and starts the vm
 func StartGenericAppleVM(mc *vmconfigs.MachineConfig, cmdBinary string, bootloader vfConfig.Bootloader, endpoint string) (func() error, func() error, error) {
 	var ignitionSocket *define.VMFile

pkg/machine/applehv/stubber.go

@@ -58,7 +58,7 @@ func (a *AppleHVStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.Machin
 	}
 
 	// Populate the ignition file with virtiofs stuff
-	virtIOIgnitionMounts, err := apple.GenerateSystemDFilesForVirtiofsMounts(virtiofsMounts)
+	virtIOIgnitionMounts, err := machine.GenerateSystemDFilesForVirtiofsMounts(virtiofsMounts)
 	if err != nil {
 		return err
 	}

pkg/machine/libkrun/stubber.go

@@ -45,7 +45,7 @@ func (l *LibKrunStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.Machin
 	}
 
 	// Populate the ignition file with virtiofs stuff
-	virtIOIgnitionMounts, err := apple.GenerateSystemDFilesForVirtiofsMounts(virtiofsMounts)
+	virtIOIgnitionMounts, err := machine.GenerateSystemDFilesForVirtiofsMounts(virtiofsMounts)
 	if err != nil {
 		return err
 	}

pkg/machine/qemu/stubber.go

@@ -10,7 +10,6 @@ import (
 	"os"
 	"os/exec"
 	"strconv"
-	"strings"
 	"time"
 
 	gvproxy "github.com/containers/gvisor-tap-vsock/pkg/types"
@@ -89,7 +88,7 @@ func (q *QEMUStubber) setQEMUCommandLine(mc *vmconfigs.MachineConfig) error {
 	return nil
 }
 
-func (q *QEMUStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.MachineConfig, _ *ignition.IgnitionBuilder) error {
+func (q *QEMUStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.MachineConfig, ignBuilder *ignition.IgnitionBuilder) error {
 	monitor, err := command.NewQMPMonitor(opts.Name, opts.Dirs.RuntimeDir)
 	if err != nil {
 		return err
@@ -110,6 +109,17 @@ func (q *QEMUStubber) CreateVM(opts define.CreateVMOpts, mc *vmconfigs.MachineCo
 
 	mc.QEMUHypervisor = &qemuConfig
 	mc.QEMUHypervisor.QEMUPidPath = qemuPidPath
+
+	virtiofsMounts := make([]machine.VirtIoFs, 0, len(mc.Mounts))
+	for _, mnt := range mc.Mounts {
+		virtiofsMounts = append(virtiofsMounts, machine.MountToVirtIOFs(mnt))
+	}
+	virtIOIgnitionMounts, err := machine.GenerateSystemDFilesForVirtiofsMounts(virtiofsMounts)
+	if err != nil {
+		return err
+	}
+	ignBuilder.WithUnit(virtIOIgnitionMounts...)
+
 	return q.resizeDisk(mc.Resources.DiskSize, mc.ImagePath)
 }
 
@@ -334,48 +344,8 @@ func (q *QEMUStubber) RemoveAndCleanMachines(_ *define.MachineDirs) error {
 	return nil
 }
 
-// mountVolumesToVM iterates through the machine's volumes and mounts them to the
-// machine
-// TODO this should probably be temporary; mount code should probably be its own package and shared completely
-func (q *QEMUStubber) MountVolumesToVM(mc *vmconfigs.MachineConfig, quiet bool) error {
-	for _, mount := range mc.Mounts {
-		if !quiet {
-			fmt.Printf("Mounting volume... %s:%s\n", mount.Source, mount.Target)
-		}
-		// create mountpoint directory if it doesn't exist
-		// because / is immutable, we have to monkey around with permissions
-		// if we dont mount in /home or /mnt
-		var args []string
-		if !strings.HasPrefix(mount.Target, "/home") && !strings.HasPrefix(mount.Target, "/mnt") {
-			args = append(args, "sudo", "chattr", "-i", "/", ";")
-		}
-		args = append(args, "sudo", "mkdir", "-p", strconv.Quote(mount.Target))
-		if !strings.HasPrefix(mount.Target, "/home") && !strings.HasPrefix(mount.Target, "/mnt") {
-			args = append(args, ";", "sudo", "chattr", "+i", "/", ";")
-		}
-		err := machine.LocalhostSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, args)
-		if err != nil {
-			return err
-		}
-
-		mountFlags := fmt.Sprintf("context=\"%s\"", machine.NFSSELinuxContext)
-		if mount.ReadOnly {
-			mountFlags += ",ro"
-		}
-		// 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",
-			mount.Tag, strconv.Quote(mount.Target),
-			"-o", mountFlags,
-		}
-		err = machine.LocalhostSSH(mc.SSH.RemoteUsername, mc.SSH.IdentityPath, mc.Name, mc.SSH.Port, append([]string{"sudo", "mount"}, mountOptions...))
-		if err != nil {
-			return err
-		}
-	}
+func (q *QEMUStubber) MountVolumesToVM(_ *vmconfigs.MachineConfig, _ bool) error {
+	// virtiofs: mounts are handled by systemd units baked into ignition at init time
 	return nil
 }
 

pkg/machine/volume_systemd.go

@@ -0,0 +1,86 @@
+package machine
+
+import (
+	"fmt"
+
+	"go.podman.io/podman/v6/pkg/machine/ignition"
+	"go.podman.io/podman/v6/pkg/systemd/parser"
+)
+
+// GenerateSystemDFilesForVirtiofsMounts generates the systemd unit files needed
+// to mount virtiofs volumes inside a FCOS guest VM. It is shared between the
+// AppleHV, LibKrun, and QEMU providers.
+//
+// Mounting in FCOS with virtiofs is a bit of a dance.  we need a unit file for
+// the mount, a unit file for automatic mounting on boot, and a "preparatory"
+// service file that disables FCOS security, performs the mkdir of the mount
+// point, and then re-enables security. This must be done for each mount.
+func GenerateSystemDFilesForVirtiofsMounts(mounts []VirtIoFs) ([]ignition.Unit, error) {
+	unitFiles := make([]ignition.Unit, 0, len(mounts)+2)
+	for _, mnt := range mounts {
+		// Create mount unit for each mount
+		mountUnit := parser.NewUnitFile()
+		mountUnit.Add("Mount", "What", "%s")
+		mountUnit.Add("Mount", "Where", "%s")
+		mountUnit.Add("Mount", "Type", "virtiofs")
+		mountUnit.Add("Mount", "Options", fmt.Sprintf("context=\"%s\"", NFSSELinuxContext))
+		mountUnit.Add("Install", "WantedBy", "local-fs.target")
+		mountUnitFile, err := mountUnit.ToString()
+		if err != nil {
+			return nil, err
+		}
+
+		virtiofsMount := ignition.Unit{
+			Enabled:  ignition.BoolToPtr(true),
+			Name:     fmt.Sprintf("%s.mount", parser.PathEscape(mnt.Target)),
+			Contents: ignition.StrToPtr(fmt.Sprintf(mountUnitFile, mnt.Tag, mnt.Target)),
+		}
+
+		unitFiles = append(unitFiles, virtiofsMount)
+	}
+
+	// This is a way to workaround the FCOS limitation of creating directories
+	// at the rootfs / and then mounting to them.
+	immutableRootOff := parser.NewUnitFile()
+	immutableRootOff.Add("Unit", "Description", "Allow systemd to create mount points on /")
+	immutableRootOff.Add("Unit", "DefaultDependencies", "no")
+
+	immutableRootOff.Add("Service", "Type", "oneshot")
+	immutableRootOff.Add("Service", "ExecStart", "chattr -i /")
+
+	immutableRootOff.Add("Install", "WantedBy", "local-fs-pre.target")
+	immutableRootOffFile, err := immutableRootOff.ToString()
+	if err != nil {
+		return nil, err
+	}
+
+	immutableRootOffUnit := ignition.Unit{
+		Contents: ignition.StrToPtr(immutableRootOffFile),
+		Name:     "immutable-root-off.service",
+		Enabled:  ignition.BoolToPtr(true),
+	}
+	unitFiles = append(unitFiles, immutableRootOffUnit)
+
+	immutableRootOn := parser.NewUnitFile()
+	immutableRootOn.Add("Unit", "Description", "Set / back to immutable after mounts are done")
+	immutableRootOn.Add("Unit", "DefaultDependencies", "no")
+	immutableRootOn.Add("Unit", "After", "local-fs.target")
+
+	immutableRootOn.Add("Service", "Type", "oneshot")
+	immutableRootOn.Add("Service", "ExecStart", "chattr +i /")
+
+	immutableRootOn.Add("Install", "WantedBy", "local-fs.target")
+	immutableRootOnFile, err := immutableRootOn.ToString()
+	if err != nil {
+		return nil, err
+	}
+
+	immutableRootOnUnit := ignition.Unit{
+		Contents: ignition.StrToPtr(immutableRootOnFile),
+		Name:     "immutable-root-on.service",
+		Enabled:  ignition.BoolToPtr(true),
+	}
+	unitFiles = append(unitFiles, immutableRootOnUnit)
+
+	return unitFiles, nil
+}