diff --git a/docs/release-notes.md b/docs/release-notes.md index d2a80411e..5329c07c3 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -20,6 +20,9 @@ nav_order: 9 ### Bug fixes +- Fix giving disk partition number 0 to get the next available slot. This caused the disks stage to fail since version 2.20.0. ([#2234](https://github.com/coreos/ignition/pull/2234)) +- Fix disk partitioning race condition where the kernel is already aware of the changes before running `partx`, causing a fatal error. ([#2234](https://github.com/coreos/ignition/pull/2234)) + ## Ignition 2.26.0 (2026-02-17) diff --git a/internal/exec/stages/disks/partitions.go b/internal/exec/stages/disks/partitions.go index 9a7ae9cad..8a447afcb 100644 --- a/internal/exec/stages/disks/partitions.go +++ b/internal/exec/stages/disks/partitions.go @@ -22,10 +22,12 @@ import ( "bufio" "errors" "fmt" + "iter" "os" "os/exec" "path/filepath" "regexp" + "slices" "sort" "strconv" "strings" @@ -130,10 +132,17 @@ func convertMiBToSectors(mib *int, sectorSize int) *int64 { } } -// getRealStartAndSize returns a map of partition numbers to a struct that contains what their real start -// and end sector should be. It runs sgdisk --pretend to determine what the partitions would look like if -// everything specified were to be (re)created. +// getRealStartAndSize returns a copy of the given partition configuration with the real partition +// numbers, start sectors, and end sectors filled in. It runs sgdisk --pretend to determine what the +// partitions would look like if everything specified were to be (re)created. func (s stage) getRealStartAndSize(dev types.Disk, devAlias string, diskInfo util.DiskInfo) ([]sgdisk.Partition, error) { + used := []bool{} + + // Determine which partition numbers are already used. + for _, part := range diskInfo.Partitions { + used[part.Number] = true + } + partitions := []sgdisk.Partition{} for _, cpart := range dev.Partitions { partitions = append(partitions, sgdisk.Partition{ @@ -156,6 +165,10 @@ func (s stage) getRealStartAndSize(dev types.Disk, devAlias string, diskInfo uti part.SizeInSectors = &info.SizeInSectors } } + if part.Number > 0 { + // Mark the partition number as used or not. + used[part.Number] = partitionShouldExist(part) + } if partitionShouldExist(part) { // Clear the label. sgdisk doesn't escape control characters. This makes parsing easier part.Label = nil @@ -182,9 +195,17 @@ func (s stage) getRealStartAndSize(dev types.Disk, devAlias string, diskInfo uti return nil, err } + free := 1 result := []sgdisk.Partition{} for _, part := range partitions { - if dims, ok := realDimensions[part.Number]; ok { + if part.Number == 0 { + // Find the next free partition number and use it. + for used[free] { + free++ + } + part.Number = free + free++ + } else if dims, ok := realDimensions[part.Number]; ok { if part.StartSector != nil { part.StartSector = &dims.start } @@ -486,9 +507,9 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { return err } - var partxAdd []uint64 - var partxDelete []uint64 - var partxUpdate []uint64 + var partxAdd []int + var partxDelete []int + var partxUpdate []int for _, part := range resolvedPartitions { shouldExist := partitionShouldExist(part) @@ -510,13 +531,13 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { case !exists && shouldExist: op.CreatePartition(part) modification = true - partxAdd = append(partxAdd, uint64(part.Number)) + partxAdd = append(partxAdd, part.Number) case exists && !shouldExist && !wipeEntry: return fmt.Errorf("partition %d exists but is specified as nonexistant and wipePartitionEntry is false", part.Number) case exists && !shouldExist && wipeEntry: op.DeletePartition(part.Number) modification = true - partxDelete = append(partxDelete, uint64(part.Number)) + partxDelete = append(partxDelete, part.Number) case exists && shouldExist && matches: s.Info("partition %d found with correct specifications", part.Number) case exists && shouldExist && !wipeEntry && !matches: @@ -530,7 +551,7 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { part.StartSector = &info.StartSector op.CreatePartition(part) modification = true - partxUpdate = append(partxUpdate, uint64(part.Number)) + partxUpdate = append(partxUpdate, part.Number) } else { return fmt.Errorf("partition %d didn't match: %v", part.Number, matchErr) } @@ -539,7 +560,7 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { op.DeletePartition(part.Number) op.CreatePartition(part) modification = true - partxUpdate = append(partxUpdate, uint64(part.Number)) + partxUpdate = append(partxUpdate, part.Number) default: // unfortunatey, golang doesn't check that all cases are handled exhaustively return fmt.Errorf("unreachable code reached when processing partition %d. golang--", part.Number) @@ -558,24 +579,18 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { // kernel partition table with BLKPG but only uses BLKRRPART which fails // as soon as one partition of the disk is mounted if len(activeParts) > 0 { - runPartxCommand := func(op string, partitions []uint64) error { - for _, partNr := range partitions { - cmd := exec.Command(distro.PartxCmd(), "--"+op, "--nr", strconv.FormatUint(partNr, 10), blockDevResolved) + runPartxCommand := func(op string, partitions iter.Seq[int]) error { + for partNr := range partitions { + cmd := exec.Command(distro.PartxCmd(), "--"+op, "--nr", fmt.Sprint(partNr), blockDevResolved) if _, err := s.LogCmd(cmd, "triggering partition %d %s on %q", partNr, op, devAlias); err != nil { return fmt.Errorf("partition %s failed: %v", op, err) } } return nil } - if err := runPartxCommand("delete", partxDelete); err != nil { - return err - } - if err := runPartxCommand("update", partxUpdate); err != nil { - return err - } - if err := runPartxCommand("add", partxAdd); err != nil { - return err - } + runPartxCommand("delete", slices.Values(partxDelete)) + runPartxCommand("update", slices.Values(partxUpdate)) + runPartxCommand("add", slices.Values(partxAdd)) } // It's best to wait here for the /dev/ABC entries to be @@ -586,5 +601,54 @@ func (s stage) partitionDisk(dev types.Disk, devAlias string) error { return fmt.Errorf("failed to wait for udev on %q after partitioning: %v", devAlias, err) } + for _, part := range resolvedPartitions { + partDev := fmt.Sprintf("%s%s%d", blockDevResolved, prefix, part.Number) + + if slices.Contains(partxDelete, part.Number) { + _, err := os.Stat(partDev) + if err == nil { + return fmt.Errorf("%q unexpectedly exists after partitioning", partDev) + } else if !os.IsNotExist(err) { + return fmt.Errorf("failed to stat %q after partitioning: %v", partDev, err) + } + } else { + sysBlockDir := fmt.Sprintf("/sys/class/block/%s/", filepath.Base(partDev)) + + // sysfs always reports in 512-byte sectors; convert our expected + // values from logical sectors to 512-byte sectors for comparison + logicalTo512 := int64(diskInfo.LogicalSectorSize) / 512 + + startStr, err := os.ReadFile(sysBlockDir + "start") + if err != nil { + return fmt.Errorf("failed to read start of %q from sysfs: %v", partDev, err) + } + kernelStart, err := strconv.ParseInt(strings.TrimSpace(string(startStr)), 10, 64) + if err != nil { + return fmt.Errorf("failed to parse start of %q from sysfs: %v", partDev, err) + } + if part.StartSector != nil { + expectedStart := *part.StartSector * logicalTo512 + if kernelStart != expectedStart { + return fmt.Errorf("kernel partition start for %q does not match expected (%d != %d 512-byte sectors)", partDev, kernelStart, expectedStart) + } + } + + sizeStr, err := os.ReadFile(sysBlockDir + "size") + if err != nil { + return fmt.Errorf("failed to read size of %q from sysfs: %v", partDev, err) + } + kernelSize, err := strconv.ParseInt(strings.TrimSpace(string(sizeStr)), 10, 64) + if err != nil { + return fmt.Errorf("failed to parse size of %q from sysfs: %v", partDev, err) + } + if part.SizeInSectors != nil { + expectedSize := *part.SizeInSectors * logicalTo512 + if kernelSize != expectedSize { + return fmt.Errorf("kernel partition size for %q does not match expected (%d != %d 512-byte sectors)", partDev, kernelSize, expectedSize) + } + } + } + } + return nil }