From b0eec26133ee578dcf7c3e613ce7f65eca96faa4 Mon Sep 17 00:00:00 2001 From: James Le Cuirot Date: Tue, 2 Jun 2026 15:12:37 +0100 Subject: [PATCH 1/2] stages/disks: Fix giving partition number 0 to get the next available This was broken since partx was used in commit c2cc56cd. Passing 0 to partx causes it to try and add all the partitions, which will almost always fail because the kernel will usually already know about at least some of them. This changes getRealStartAndSize() to also determine and return the resulting partition numbers so that subsequent operations use these instead of 0. sgdisk does support --new=0, but it has no way to report which partition number it actually used. Signed-off-by: James Le Cuirot --- docs/release-notes.md | 2 ++ internal/exec/stages/disks/partitions.go | 27 ++++++++++++++++++++---- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index d2a80411e..8c4f8d8f1 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -20,6 +20,8 @@ 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)) + ## 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..6435c2eef 100644 --- a/internal/exec/stages/disks/partitions.go +++ b/internal/exec/stages/disks/partitions.go @@ -130,10 +130,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 +163,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 +193,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 } From 1514e0b6fb8dfcfd30c8f472a6c798dcd5b3a185 Mon Sep 17 00:00:00 2001 From: James Le Cuirot Date: Fri, 8 May 2026 17:58:38 +0100 Subject: [PATCH 2/2] stages/disks: Allow partx to fail then check the state later `partx --add` will fail if the kernel is already aware of the new partition. It was always theoretically possible that udev might trigger early, and that appears to be happening now. Allow partx to fail and then check that added/updated partitions have the right start sector and size and that deleted partitions are absent once udev has settled. Signed-off-by: James Le Cuirot --- docs/release-notes.md | 1 + internal/exec/stages/disks/partitions.go | 83 ++++++++++++++++++------ 2 files changed, 65 insertions(+), 19 deletions(-) diff --git a/docs/release-notes.md b/docs/release-notes.md index 8c4f8d8f1..5329c07c3 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -21,6 +21,7 @@ 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 6435c2eef..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" @@ -505,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) @@ -529,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: @@ -549,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) } @@ -558,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) @@ -577,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 @@ -605,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 }