Skip to content

Commit

Permalink
apple virtiofs: fix racy mount setup
Browse files Browse the repository at this point in the history
One problem on FCOS is that the root directory is immutable, as such in
order to mount arbitrary paths from the host we must make it mutable
again and create these dir on boot in order to be able to mount there.

The current logic was racy as it used one unit for each path and they
all did chattr -i /; mkdir -p $path; chattr -i / and systemd can run
these units in parallel. That means it was possible for another unit to
make / immutable before the unit could do the mkdir. I pointed this out
on the original PR[1] but we never followed up on it...

Now this here changes several things. First have one unit that does the
chattr -i / (immutable-root-off.service), it is hooked into
remote-fs-pre.target which means it is executed before the network
mounts (virtiofs) are done.

Then we have another unit that does chattr +i /
(immutable-root-on.service) which turn the immutable root back on after
remote-fs.target which means all mount are done at this point.

Additionally the automount unit is removed because it does not add any
value for us and it was borken anyway as it used the virtiofs tag as
path so systemd just ignored it.

[1] #20612 (comment)

Fixes #22569

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
  • Loading branch information
Luap99 committed Jun 27, 2024
1 parent 67df6d6 commit fdb736d
Showing 1 changed file with 38 additions and 41 deletions.
79 changes: 38 additions & 41 deletions pkg/machine/apple/apple.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,7 @@ func GenerateSystemDFilesForVirtiofsMounts(mounts []machine.VirtIoFs) ([]ignitio

unitFiles := make([]ignition.Unit, 0, len(mounts))
for _, mnt := range mounts {
// Here we are looping the mounts and for each mount, we are adding two unit files
// for virtiofs. One unit file is the mount itself and the second is to automount it
// on boot.
autoMountUnit := parser.NewUnitFile()
autoMountUnit.Add("Automount", "Where", "%s")
autoMountUnit.Add("Install", "WantedBy", "multi-user.target")
autoMountUnit.Add("Unit", "Description", "Mount virtiofs volume %s")
autoMountUnitFile, err := autoMountUnit.ToString()
if err != nil {
return nil, err
}

// Create mount unit for each mount
mountUnit := parser.NewUnitFile()
mountUnit.Add("Mount", "What", "%s")
mountUnit.Add("Mount", "Where", "%s")
Expand All @@ -95,49 +84,57 @@ func GenerateSystemDFilesForVirtiofsMounts(mounts []machine.VirtIoFs) ([]ignitio
return nil, err
}

virtiofsAutomount := ignition.Unit{
Enabled: ignition.BoolToPtr(true),
Name: fmt.Sprintf("%s.automount", parser.PathEscape(mnt.Target)),
Contents: ignition.StrToPtr(fmt.Sprintf(autoMountUnitFile, mnt.Tag, mnt.Target)),
}
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)),
}

// This "unit" simulates something like systemctl enable virtiofs-mount-prepare@
enablePrep := ignition.Unit{
Enabled: ignition.BoolToPtr(true),
Name: fmt.Sprintf("virtiofs-mount-prepare@%s.service", parser.PathEscape(mnt.Target)),
}

unitFiles = append(unitFiles, virtiofsAutomount, virtiofsMount, enablePrep)
unitFiles = append(unitFiles, virtiofsMount)
}

// mount prep is a way to workaround the FCOS limitation of creating directories
// This is a way to workaround the FCOS limitation of creating directories
// at the rootfs / and then mounting to them.
mountPrep := parser.NewUnitFile()
mountPrep.Add("Unit", "Description", "Allow virtios to mount to /")
mountPrep.Add("Unit", "DefaultDependencies", "no")
mountPrep.Add("Unit", "ConditionPathExists", "!%f")

mountPrep.Add("Service", "Type", "oneshot")
mountPrep.Add("Service", "ExecStartPre", "chattr -i /")
mountPrep.Add("Service", "ExecStart", "mkdir -p '%f'")
mountPrep.Add("Service", "ExecStopPost", "chattr +i /")

mountPrep.Add("Install", "WantedBy", "remote-fs.target")
mountPrepFile, err := mountPrep.ToString()
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", "remote-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", "remote-fs.target")

immutableRootOn.Add("Service", "Type", "oneshot")
immutableRootOn.Add("Service", "ExecStart", "chattr +i /")

immutableRootOn.Add("Install", "WantedBy", "remote-fs.target")
immutableRootOnFile, err := immutableRootOn.ToString()
if err != nil {
return nil, err
}

virtioFSChattr := ignition.Unit{
Contents: ignition.StrToPtr(mountPrepFile),
Name: "virtiofs-mount-prepare@.service",
immutableRootOnUnit := ignition.Unit{
Contents: ignition.StrToPtr(immutableRootOnFile),
Name: "immutable-root-on.service",
Enabled: ignition.BoolToPtr(true),
}
unitFiles = append(unitFiles, virtioFSChattr)
unitFiles = append(unitFiles, immutableRootOnUnit)

return unitFiles, nil
}
Expand Down

0 comments on commit fdb736d

Please sign in to comment.