Skip to content

Commit

Permalink
disk: extract getDevicesNoRename() and tweak getDevices()
Browse files Browse the repository at this point in the history
We currently do a lot of:
```
lastDevice := stageDevices[lastName]
delete(stageDevices, lastName)
stageDevices["device"] = lastDevice
```
which this commit consolidates into the `getDevices()` helper.
The only place that needs the "old" behavior is
`GenMountsDevicesFromPT` and here we can use the new
`getDevicesNoRename()` helper.

Note that this is still pretty messy and needs further refactor.
  • Loading branch information
mvo5 committed Nov 28, 2024
1 parent d9448fe commit fa11558
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 54 deletions.
49 changes: 20 additions & 29 deletions pkg/osbuild/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,7 @@ func GenDeviceCreationStages(pt *disk.PartitionTable, filename string) []*Stage
switch ent := e.(type) {
case *disk.LUKSContainer:
// do not include us when getting the devices
stageDevices, lastName := getDevices(path[:len(path)-1], filename, true)

// "org.osbuild.luks2.format" expects a "device" to create the VG on,
// thus rename the last device to "device"
lastDevice := stageDevices[lastName]
delete(stageDevices, lastName)
stageDevices["device"] = lastDevice
stageDevices := getDevices(path[:len(path)-1], filename, true)

stage := NewLUKS2CreateStage(
&LUKS2CreateStageOptions{
Expand Down Expand Up @@ -64,13 +58,7 @@ func GenDeviceCreationStages(pt *disk.PartitionTable, filename string) []*Stage

case *disk.LVMVolumeGroup:
// do not include us when getting the devices
stageDevices, lastName := getDevices(path[:len(path)-1], filename, true)

// "org.osbuild.lvm2.create" expects a "device" to create the VG on,
// thus rename the last device to "device"
lastDevice := stageDevices[lastName]
delete(stageDevices, lastName)
stageDevices["device"] = lastDevice
stageDevices := getDevices(path[:len(path)-1], filename, true)

volumes := make([]LogicalVolume, len(ent.LogicalVolumes))
for idx, lv := range ent.LogicalVolumes {
Expand Down Expand Up @@ -104,11 +92,7 @@ func GenDeviceFinishStages(pt *disk.PartitionTable, filename string) []*Stage {
switch ent := e.(type) {
case *disk.LUKSContainer:
// do not include us when getting the devices
stageDevices, lastName := getDevices(path[:len(path)-1], filename, true)

lastDevice := stageDevices[lastName]
delete(stageDevices, lastName)
stageDevices["device"] = lastDevice
stageDevices := getDevices(path[:len(path)-1], filename, true)

if ent.Clevis != nil {
if ent.Clevis.RemovePassphrase {
Expand All @@ -119,13 +103,7 @@ func GenDeviceFinishStages(pt *disk.PartitionTable, filename string) []*Stage {
}
case *disk.LVMVolumeGroup:
// do not include us when getting the devices
stageDevices, lastName := getDevices(path[:len(path)-1], filename, true)

// "org.osbuild.lvm2.metadata" expects a "device" to rename the VG,
// thus rename the last device to "device"
lastDevice := stageDevices[lastName]
delete(stageDevices, lastName)
stageDevices["device"] = lastDevice
stageDevices := getDevices(path[:len(path)-1], filename, true)

stage := NewLVM2MetadataStage(
&LVM2MetadataStageOptions{
Expand Down Expand Up @@ -180,7 +158,7 @@ func deviceName(p disk.Entity) string {
// The first returned value is a map of devices for the given path.
// The second returned value is the name of the last device in the path. This is the device that should be used as the
// source for the mount.
func getDevices(path []disk.Entity, filename string, lockLoopback bool) (map[string]Device, string) {
func getDevicesNoRename(path []disk.Entity, filename string, lockLoopback bool) (map[string]Device, string) {
var pt *disk.PartitionTable

do := make(map[string]Device)
Expand Down Expand Up @@ -219,9 +197,22 @@ func getDevices(path []disk.Entity, filename string, lockLoopback bool) (map[str
parent = name
}
}

return do, parent
}

func getDevices(path []disk.Entity, filename string, lockLoopback bool) map[string]Device {
do, parent := getDevicesNoRename(path, filename, lockLoopback)

// The last device in the chain must be named "device", because that's the device that mkfs stages run on.
// See their schema for reference.
lastDevice := do[parent]
delete(do, parent)
do["device"] = lastDevice

return do
}

// pathEscape implements similar path escaping as used by systemd-escape
// https://github.com/systemd/systemd/blob/c57ff6230e4e199d40f35a356e834ba99f3f8420/src/basic/unit-name.c#L389
func pathEscape(path string) string {
Expand Down Expand Up @@ -281,8 +272,8 @@ func GenMountsDevicesFromPT(filename string, pt *disk.PartitionTable) (string, [
mounts := make([]Mount, 0, len(pt.Partitions))
var fsRootMntName string
genMounts := func(mnt disk.Mountable, path []disk.Entity) error {
stageDevices, leafDeviceName := getDevices(path, filename, false)
mount, err := genOsbuildMount(leafDeviceName, mnt)
stageDevices, leafDevice := getDevicesNoRename(path, filename, false)
mount, err := genOsbuildMount(leafDevice, mnt)
if err != nil {
return err
}
Expand Down
28 changes: 3 additions & 25 deletions pkg/osbuild/mkfs_stage.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,7 @@ func GenFsStages(pt *disk.PartitionTable, filename string) []*Stage {
genStage := func(ent disk.Entity, path []disk.Entity) error {
switch e := ent.(type) {
case *disk.Filesystem:
// TODO: extract last device renaming into helper
stageDevices, lastName := getDevices(path, filename, true)

// The last device in the chain must be named "device", because that's
// the device that mkfs stages run on. See the stage schemas for
// reference.
lastDevice := stageDevices[lastName]
delete(stageDevices, lastName)
stageDevices["device"] = lastDevice
stageDevices := getDevices(path, filename, true)

switch e.GetFSType() {
case "xfs":
Expand All @@ -50,14 +42,7 @@ func GenFsStages(pt *disk.PartitionTable, filename string) []*Stage {
panic(fmt.Sprintf("unknown fs type: %s", e.GetFSType()))
}
case *disk.Btrfs:
stageDevices, lastName := getDevices(path, filename, true)

// The last device in the chain must be named "device", because that's
// the device that mkfs stages run on. See the stage schemas for
// reference.
lastDevice := stageDevices[lastName]
delete(stageDevices, lastName)
stageDevices["device"] = lastDevice
stageDevices := getDevices(path, filename, true)

options := &MkfsBtrfsStageOptions{
UUID: e.UUID,
Expand All @@ -78,14 +63,7 @@ func GenFsStages(pt *disk.PartitionTable, filename string) []*Stage {
stages = append(stages, NewBtrfsSubVol(&BtrfsSubVolOptions{subvolumes}, &stageDevices, &[]Mount{mount}))
case *disk.Swap:
// TODO: extract last device renaming into helper
stageDevices, lastName := getDevices(path, filename, true)

// The last device in the chain must be named "device", because that's
// the device that the mkswap stage runs on. See the stage schema
// for reference.
lastDevice := stageDevices[lastName]
delete(stageDevices, lastName)
stageDevices["device"] = lastDevice
stageDevices := getDevices(path, filename, true)

options := &MkswapStageOptions{
UUID: e.UUID,
Expand Down

0 comments on commit fa11558

Please sign in to comment.