From 4a11bfcb63fc38a867e62db740c8731499e6a16e Mon Sep 17 00:00:00 2001 From: Nicola Sella Date: Mon, 1 Jun 2026 15:16:21 +0200 Subject: [PATCH] machine: share virtiofs systemd unit generation Make GenerateSystemDFilesForVirtiofsMounts an agnostic function and move the code from apple to pkg/machine/volume_systemd.go. MountVolumesToVM is now a no-op matching the AppleHV and LibKrun behaviour. Continues from PR: https://github.com/podman-container-tools/podman/pull/28736 Fixes: https://redhat.atlassian.net/browse/RUN-4742 Signed-off-by: Nicola Sella --- pkg/machine/apple/apple.go | 77 ------------------------------ pkg/machine/applehv/stubber.go | 2 +- pkg/machine/libkrun/stubber.go | 2 +- pkg/machine/qemu/stubber.go | 58 ++++++----------------- pkg/machine/volume_systemd.go | 86 ++++++++++++++++++++++++++++++++++ 5 files changed, 102 insertions(+), 123 deletions(-) create mode 100644 pkg/machine/volume_systemd.go diff --git a/pkg/machine/apple/apple.go b/pkg/machine/apple/apple.go index 092854127f1..329dc160755 100644 --- a/pkg/machine/apple/apple.go +++ b/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 diff --git a/pkg/machine/applehv/stubber.go b/pkg/machine/applehv/stubber.go index 117975d4d7d..1831f64d0f9 100644 --- a/pkg/machine/applehv/stubber.go +++ b/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 } diff --git a/pkg/machine/libkrun/stubber.go b/pkg/machine/libkrun/stubber.go index 4ace7021b15..ecffe3e25fa 100644 --- a/pkg/machine/libkrun/stubber.go +++ b/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 } diff --git a/pkg/machine/qemu/stubber.go b/pkg/machine/qemu/stubber.go index 3ef60aadbcc..17b872101e5 100644 --- a/pkg/machine/qemu/stubber.go +++ b/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 } diff --git a/pkg/machine/volume_systemd.go b/pkg/machine/volume_systemd.go new file mode 100644 index 00000000000..28dcc08934e --- /dev/null +++ b/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 +}