Repository navigation
Report free space in on-disk order and stop GEOM label shadowing - #29
Merged
Merged
Conversation
disk-part.sh and part-label.sh built their output from
get_disk_partitions and get_partitions_lables, which return only the
occupied entries, then appended a single freemb value for the largest
gap. A front-end had no way to tell where a gap sat, so it could not
merge free space with the neighbours it touches after a delete.
Both scripts now walk gpart show directly. gpart sorts entries by start
block, so emitting each line as it is read puts every gap in its true
position between the entries it separates, reported as
freespaceN-blockstart, -blocksize and -sizemb. blocksize is exact and
sizemb rounds down, reading 0 for a gap under 2048 blocks.
disk-info.sh derived the controller type by scraping dmesg, which
returns nothing once the boot messages have rolled out of the ring
buffer. The device name prefix is the CAM peripheral that attached, so
map that to the protocol family instead.
None of that helps if gpart cannot read the disk at all.
kern.geom.label.disk_ident and kern.geom.label.gptid default to enabled,
so GEOM tastes a raw disk twice and adaX coexists with
diskid/DISK-<serial>. Once the installed system imports its pool, the
pool binds to the diskid provider and GEOM withdraws the raw partition
geom for the whole disk. gpart(8) then fails on that disk and its
/dev/adaXpY nodes stop resolving. run_final_cleanup now disables both
labels on the installed system.
setup_fstab writes raw ${PARTDEV} paths for every non-ZFS entry, so a
ZFS install still has swap and the EFI mount pointing at those nodes.
Write both lines unconditionally rather than gating them on ZFS, since
the entries that break sit beside the pool on the same disk. sysrc
cannot set these; it rejects dotted names, hence the appends.
Reviewer's GuideThe PR changes disk and partition reporting to stream gpart’s on-disk layout, including each free-space gap, replaces fragile dmesg-based protocol detection with device-prefix mapping, and prevents GEOM label providers from hiding raw devices needed by installed fstab entries. Sequence diagram for on-disk partition and free-space reportingsequenceDiagram
participant Frontend
participant DiskPart as disk-part.sh
participant Gpart as gpart
Frontend->>DiskPart: request disk layout
DiskPart->>Gpart: gpart show DISK
Gpart-->>DiskPart: entries sorted by start block
loop each layout entry
alt partition entry
DiskPart-->>Frontend: partition blockstart, blocksize, sizemb
else free-space entry
DiskPart-->>Frontend: freespaceN blockstart, blocksize, sizemb
end
end
Sequence diagram for persistent disk protocol detectionsequenceDiagram
participant DiskInfo as disk-info.sh
participant Device as device name
DiskInfo->>Device: inspect DISK prefix
alt ada or ad
Device-->>DiskInfo: ATA
else da
Device-->>DiskInfo: SCSI
else nvd or nda
Device-->>DiskInfo: NVMe
else vtbd
Device-->>DiskInfo: VirtIO
else mmcsd, md, or unknown
Device-->>DiskInfo: mapped protocol or Unknown
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. The installer permanently disables disk and GPT-ID GEOM labels, so an incorrect device-addressing assumption could leave installed systems unable to boot or access swap, EFI, or imported storage. Reverting the configuration can restore behavior, but if the system is already unbootable that recovery requires out-of-band access.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
disk-part.sh and part-label.sh built their output from get_disk_partitions and get_partitions_lables, which return only the occupied entries, then appended a single freemb value for the largest gap. A front-end had no way to tell where a gap sat, so it could not merge free space with the neighbours it touches after a delete.
Both scripts now walk gpart show directly. gpart sorts entries by start block, so emitting each line as it is read puts every gap in its true position between the entries it separates, reported as freespaceN-blockstart, -blocksize and -sizemb. blocksize is exact and sizemb rounds down, reading 0 for a gap under 2048 blocks.
disk-info.sh derived the controller type by scraping dmesg, which returns nothing once the boot messages have rolled out of the ring buffer. The device name prefix is the CAM peripheral that attached, so map that to the protocol family instead.
None of that helps if gpart cannot read the disk at all. kern.geom.label.disk_ident and kern.geom.label.gptid default to enabled, so GEOM tastes a raw disk twice and adaX coexists with diskid/DISK-. Once the installed system imports its pool, the pool binds to the diskid provider and GEOM withdraws the raw partition geom for the whole disk. gpart(8) then fails on that disk and its /dev/adaXpY nodes stop resolving. run_final_cleanup now disables both labels on the installed system.
setup_fstab writes raw ${PARTDEV} paths for every non-ZFS entry, so a ZFS install still has swap and the EFI mount pointing at those nodes. Write both lines unconditionally rather than gating them on ZFS, since the entries that break sit beside the pool on the same disk. sysrc cannot set these; it rejects dotted names, hence the appends.
Summary by Sourcery
Preserve disk layout ordering and raw-device availability so front-ends can manage free space and installed systems can reliably access their partitions.
New Features:
Bug Fixes:
Enhancements: