From 9506ad1fcdfb2af11d4692848784be0302d415fe Mon Sep 17 00:00:00 2001 From: Aaron Anderson Date: Mon, 12 Oct 2026 09:36:00 -0500 Subject: [PATCH 1/2] fdisk: add disk-only inventory discovery --- fdisk/fdisk-lib.pl | 502 ++++++++++++++++------------ fdisk/t/README.md | 7 + fdisk/t/disk-discovery-perlcritic.t | 16 + fdisk/t/disk-discovery.t | 197 +++++++++++ fdisk/t/fixtures/disk-discovery.pl | 77 +++++ 5 files changed, 594 insertions(+), 205 deletions(-) create mode 100644 fdisk/t/README.md create mode 100644 fdisk/t/disk-discovery-perlcritic.t create mode 100644 fdisk/t/disk-discovery.t create mode 100644 fdisk/t/fixtures/disk-discovery.pl diff --git a/fdisk/fdisk-lib.pl b/fdisk/fdisk-lib.pl index f658cfa5f..535c31157 100755 --- a/fdisk/fdisk-lib.pl +++ b/fdisk/fdisk-lib.pl @@ -40,11 +40,15 @@ else { } $| = 1; -# list_disks_partitions([include-cds]) +# list_disks_partitions([include-cds], [disk-only], [error-ref]) +# Disk-only failures return no disks; error-ref receives the reason if supplied. # Returns a structure containing the details of all disks and partitions sub list_disks_partitions { -if (scalar(@list_disks_partitions_cache)) { +local ($include_cds, $disk_only) = @_; +my $error_ref = $_[2]; +$$error_ref = undef if ($disk_only && $error_ref); +if (!$disk_only && scalar(@list_disks_partitions_cache)) { return @list_disks_partitions_cache; } @@ -122,10 +126,13 @@ else { } local (@disks, @devs, $d); +my $discovery_partitions = ''; + if (open(PARTS, ") { + $discovery_partitions .= $_ if ($disk_only); if (/\d+\s+\d+\s+\d+\s+sd([a-z]+)\s/ || /\d+\s+\d+\s+\d+\s+(scsi\/host(\d+)\/bus(\d+)\/target(\d+)\/lun(\d+)\/disc)\s+/) { # New or old style SCSI device @@ -213,6 +220,15 @@ if (open(PARTS, " ($a =~ /\/hd[a-z]+$/ ? 1 : 0) } @devs; } +elsif ($disk_only) { + return &disk_discovery_error("Cannot open disk inventory for SMART discovery", $error_ref); + } +if ($disk_only) { + return &disk_discovery_error("Invalid disk inventory for SMART discovery", $error_ref) + if ($discovery_partitions !~ /major\s+minor\s+#blocks\s+name/); + return &disk_discovery_error("Disk inventory changed during SMART discovery", $error_ref) + if ($discovery_partitions ne &read_file_contents('/proc/partitions')); + } return ( ) if (!@devs); # No disks, ie on Xen # Skip cd-rom drive, identified from symlink. Don't do this if we can identify @@ -224,6 +240,16 @@ if (!-d "/proc/ide") { } } +my %discovery_identity; +if ($disk_only) { + foreach my $device (@devs) { + my $identity = &disk_discovery_identity($device); + return &disk_discovery_error("Missing device during SMART discovery: $device", $error_ref) + if (!defined($identity)); + $discovery_identity{$device} = $identity; + } + } + # Get Linux disk ID mapping local %id_map; local %all_id_map; @@ -240,6 +266,246 @@ foreach my $id (readdir(IDS)) { } closedir(IDS); +# Both paths use the same classification, ID and model logic. +my $set_metadata = sub { + my ($disk) = @_; + if ($disk->{'device'} =~ /\/sd([a-z]+)$/) { + # Old-style SCSI disk + $disk->{'desc'} = &text('select_device', 'SCSI', + uc($1)); + local ($dscsi) = grep { $_->{'dev'} eq "sd$1" } @dscsi; + $disk->{'scsi'} = $dscsi ? &indexof($dscsi, @dscsi) + : ord(uc($1))-65; + $disk->{'type'} = 'scsi'; + } + elsif ($disk->{'device'} =~ /\/hd([a-z]+)$/) { + # IDE disk + $disk->{'desc'} = &text('select_device', 'IDE', uc($1)); + $disk->{'type'} = 'ide'; + } + elsif ($disk->{'device'} =~ /\/xvd([a-z]+)$/) { + # Xen virtual disk + $disk->{'desc'} = &text('select_device', 'Xen', uc($1)); + $disk->{'type'} = 'ide'; + } + elsif ($disk->{'device'} =~ /\/mmcblk([0-9]+)$/) { + # SD-card / MMC + $disk->{'desc'} = &text('select_device', 'SD-Card', $1); + $disk->{'type'} = 'ide'; + } + elsif ($disk->{'device'} =~ /\/vd([a-z]+)$/) { + # KVM virtual disk + $disk->{'desc'} = &text('select_device', + 'VirtIO', uc($1)); + $disk->{'type'} = 'virtio'; + } + elsif ($disk->{'device'} =~ /\/(scsi\/host(\d+)\/bus(\d+)\/target(\d+)\/lun(\d+)\/disc)/) { + # New complete SCSI disk specification + $disk->{'host'} = $2; + $disk->{'bus'} = $3; + $disk->{'target'} = $4; + $disk->{'lun'} = $5; + $disk->{'desc'} = &text('select_scsi', + "$2", "$3", "$4", "$5"); + + # Work out the SCSI index for this disk + local $j; + if ($dscsi_mode) { + for($j=0; $j<@dscsi; $j++) { + if ($dscsi[$j]->{'host'} == $disk->{'host'} && $dscsi[$j]->{'bus'} == $disk->{'bus'} && $dscsi[$j]->{'target'} == $disk->{'target'} && $dscsi[$j]->{'lnun'} == $disk->{'lun'}) { + $disk->{'scsi'} = $j; + last; + } + } + } + else { + for($j=0; $j<@pscsi; $j++) { + if ($pscsi[$j] =~ /Host:\s+scsi(\d+).*Id:\s+(\d+)/i && $disk->{'host'} == $1 && $disk->{'target'} == $2) { + $disk->{'scsi'} = $j; + last; + } + } + } + $disk->{'type'} = 'scsi'; + $disk->{'prefix'} =~ s/disc$/part/g; + } + elsif ($disk->{'device'} =~ /\/(ide\/host(\d+)\/bus(\d+)\/target(\d+)\/lun(\d+)\/disc)/) { + # New-style IDE specification + $disk->{'host'} = $2; + $disk->{'bus'} = $3; + $disk->{'target'} = $4; + $disk->{'lun'} = $5; + $disk->{'desc'} = &text('select_newide', + "$2", "$3", "$4", "$5"); + $disk->{'type'} = 'ide'; + $disk->{'prefix'} =~ s/disc$/part/g; + } + elsif ($disk->{'device'} =~ /\/(rd\/c(\d+)d(\d+))/) { + # Mylex raid device + local ($mc, $md) = ($2, $3); + $disk->{'desc'} = &text('select_mylex', $mc, $md); + open(RD, ") { + if (/^Configuring\s+(.*)/i) { + $disk->{'model'} = $1; + } + elsif (/\s+(\S+):\s+([^, ]+)/ && + $1 eq $disk->{'device'}) { + $disk->{'raid'} = $2; + } + } + close(RD); + $disk->{'type'} = 'raid'; + $disk->{'prefix'} = $disk->{'device'}.'p'; + } + elsif ($disk->{'device'} =~ /\/(ida\/c(\d+)d(\d+))/) { + # Compaq RAID device + local ($ic, $id) = ($2, $3); + $disk->{'desc'} = &text('select_cpq', $ic, $id); + open(IDA, -d "/proc/driver/array" ? ") { + if (/^(\S+):\s+(.*)/ && $1 eq "ida$ic") { + $disk->{'model'} = $2; + } + } + close(IDA); + $disk->{'type'} = 'raid'; + $disk->{'prefix'} = $disk->{'device'}.'p'; + } + elsif ($disk->{'device'} =~ /\/(cciss\/c(\d+)d(\d+))/) { + # Compaq Smart Array RAID + local ($ic, $id) = ($2, $3); + $disk->{'desc'} = &text('select_smart', $ic, $id); + open(CCI, ") { + if (/^\s*(\S+):\s*(.*)/ && $1 eq "cciss$ic") { + $disk->{'model'} = $2; + } + } + close(CCI); + $disk->{'type'} = 'raid'; + $disk->{'prefix'} = $disk->{'device'}.'p'; + } + elsif ($disk->{'device'} =~ /\/(ataraid\/disc(\d+)\/disc)/) { + # Promise RAID controller + local $dd = $2; + $disk->{'desc'} = &text('select_promise', $dd); + $disk->{'type'} = 'raid'; + $disk->{'prefix'} =~ s/disc$/part/g; + } + elsif ($disk->{'device'} =~ /\/nvme(\d+)n(\d+)$/) { + # NVME SSD controller + $disk->{'desc'} = &text('select_nvme', "$1", "$2"); + $disk->{'type'} = 'scsi'; + $disk->{'prefix'} = $disk->{'device'}.'p'; + } + + # Work out short name, like sda + local $short; + if (defined($disk->{'host'})) { + $short = &hbt_to_device($disk->{'host'}, + $disk->{'bus'}, + $disk->{'target'}); + } + else { + $short = $disk->{'device'}; + $short =~ s/^.*\///g; + } + $disk->{'short'} = $short; + + $disk->{'id'} = $id_map{$disk->{'device'}} || + $id_map{"/dev/$short"}; + $disk->{'ids'} = $all_id_map{$disk->{'device'}} || + $all_id_map{"/dev/$short"}; +}; +my $enrich_metadata = sub { + local @disks = @_; + local $d; + # Check /proc/ide for IDE disk models + foreach $d (@disks) { + if ($d->{'type'} eq 'ide') { + local $short = $d->{'short'}; + $d->{'model'} = &read_file_contents("/proc/ide/$short/model"); + $d->{'model'} =~ s/\r|\n//g; + $d->{'media'} = &read_file_contents("/proc/ide/$short/media"); + $d->{'media'} =~ s/\r|\n//g; + if ($d->{'short'} =~ /^vd/ && !$d->{'model'}) { + # Fake up model for KVM VirtIO disks + $d->{'model'} = "KVM VirtIO"; + } + } + } + + # Fill in SCSI information + foreach $d (@disks) { + if ($d->{'type'} eq 'scsi') { + local $s = $d->{'scsi'}; + local $sysdir = "/sys/block/$d->{'short'}/device"; + if (-d $sysdir) { + # From kernel 2.6.30+ sys directory + $d->{'model'} = &read_file_contents("$sysdir/vendor"). + " ". + &read_file_contents("$sysdir/model"); + $d->{'model'} =~ s/\r|\n//g; + $d->{'media'} = &read_file_contents("$sysdir/media"); + $d->{'media'} =~ s/\r|\n//g; + } + elsif ($dscsi_mode) { + # From other scsi files + $d->{'model'} = "$dscsi[$s]->{'make'} $dscsi[$s]->{'model'}"; + $d->{'controller'} = $dscsi[$s]->{'host'}; + $d->{'scsiid'} = $dscsi[$s]->{'target'}; + } + else { + # From /proc/scsi/scsi lines + if ($pscsi[$s] =~ /Vendor:\s+(\S+).*Model:\s+(.*)\s+Rev:/i) { + $d->{'model'} = "$1 $2"; + } + if ($pscsi[$s] =~ /Host:\s+scsi(\d+).*Id:\s+(\d+)/i) { + $d->{'controller'} = int($1); + $d->{'scsiid'} = int($2); + } + } + if ($d->{'model'} =~ /ATA/) { + # Fake SCSI disk, actually IDE + $d->{'scsi'} = 0; + $d->{'desc'} =~ s/SCSI/SATA/g; + foreach my $p (@{$d->{'parts'} || []}) { + $p->{'desc'} =~ s/SCSI/SATA/g; + } + } + } + } + +}; + +if ($disk_only) { + my @inventory = @devs; + foreach my $device (@inventory) { + my $disk = { 'device' => $device, 'prefix' => $device }; + $set_metadata->($disk); + push(@disks, $disk); + } + $enrich_metadata->(@disks); + foreach my $disk (@disks) { + my $device = $disk->{'device'}; + my $identity = &disk_discovery_identity($device); + return &disk_discovery_error("Missing device during SMART discovery: $device", $error_ref) + if (!defined($identity)); + return &disk_discovery_error("Disk identity changed during SMART discovery: $device", $error_ref) + if ($discovery_identity{$device} ne $identity); + # An absent controller model must not silently bypass RAID expansion. + return &disk_discovery_error("Missing disk model during SMART discovery: $device", $error_ref) + if (($disk->{'type'} eq 'scsi' || $disk->{'type'} eq 'raid') && + $disk->{'model'} !~ /\S/); + } + return &disk_discovery_error("Disk inventory changed during SMART discovery", $error_ref) + if ($discovery_partitions ne &read_file_contents('/proc/partitions')); + # These are metadata records, not claims of empty partition tables. + # Do not read or populate the full partition cache. + return @disks; + } + # Call fdisk to get partition and geometry information local $qdevs = join(" ", map { quotemeta($_) } @devs); local ($disk, $m2); @@ -297,154 +563,7 @@ while() { local @st = stat($disk->{'device'}); next if (@cdstat && $st[1] == $cdstat[1]); - if ($disk->{'device'} =~ /\/sd([a-z]+)$/) { - # Old-style SCSI disk - $disk->{'desc'} = &text('select_device', 'SCSI', - uc($1)); - local ($dscsi) = grep { $_->{'dev'} eq "sd$1" } @dscsi; - $disk->{'scsi'} = $dscsi ? &indexof($dscsi, @dscsi) - : ord(uc($1))-65; - $disk->{'type'} = 'scsi'; - } - elsif ($disk->{'device'} =~ /\/hd([a-z]+)$/) { - # IDE disk - $disk->{'desc'} = &text('select_device', 'IDE', uc($1)); - $disk->{'type'} = 'ide'; - } - elsif ($disk->{'device'} =~ /\/xvd([a-z]+)$/) { - # Xen virtual disk - $disk->{'desc'} = &text('select_device', 'Xen', uc($1)); - $disk->{'type'} = 'ide'; - } - elsif ($disk->{'device'} =~ /\/mmcblk([0-9]+)$/) { - # SD-card / MMC - $disk->{'desc'} = &text('select_device', 'SD-Card', $1); - $disk->{'type'} = 'ide'; - } - elsif ($disk->{'device'} =~ /\/vd([a-z]+)$/) { - # KVM virtual disk - $disk->{'desc'} = &text('select_device', - 'VirtIO', uc($1)); - $disk->{'type'} = 'virtio'; - } - elsif ($disk->{'device'} =~ /\/(scsi\/host(\d+)\/bus(\d+)\/target(\d+)\/lun(\d+)\/disc)/) { - # New complete SCSI disk specification - $disk->{'host'} = $2; - $disk->{'bus'} = $3; - $disk->{'target'} = $4; - $disk->{'lun'} = $5; - $disk->{'desc'} = &text('select_scsi', - "$2", "$3", "$4", "$5"); - - # Work out the SCSI index for this disk - local $j; - if ($dscsi_mode) { - for($j=0; $j<@dscsi; $j++) { - if ($dscsi[$j]->{'host'} == $disk->{'host'} && $dscsi[$j]->{'bus'} == $disk->{'bus'} && $dscsi[$j]->{'target'} == $disk->{'target'} && $dscsi[$j]->{'lnun'} == $disk->{'lun'}) { - $disk->{'scsi'} = $j; - last; - } - } - } - else { - for($j=0; $j<@pscsi; $j++) { - if ($pscsi[$j] =~ /Host:\s+scsi(\d+).*Id:\s+(\d+)/i && $disk->{'host'} == $1 && $disk->{'target'} == $2) { - $disk->{'scsi'} = $j; - last; - } - } - } - $disk->{'type'} = 'scsi'; - $disk->{'prefix'} =~ s/disc$/part/g; - } - elsif ($disk->{'device'} =~ /\/(ide\/host(\d+)\/bus(\d+)\/target(\d+)\/lun(\d+)\/disc)/) { - # New-style IDE specification - $disk->{'host'} = $2; - $disk->{'bus'} = $3; - $disk->{'target'} = $4; - $disk->{'lun'} = $5; - $disk->{'desc'} = &text('select_newide', - "$2", "$3", "$4", "$5"); - $disk->{'type'} = 'ide'; - $disk->{'prefix'} =~ s/disc$/part/g; - } - elsif ($disk->{'device'} =~ /\/(rd\/c(\d+)d(\d+))/) { - # Mylex raid device - local ($mc, $md) = ($2, $3); - $disk->{'desc'} = &text('select_mylex', $mc, $md); - open(RD, ") { - if (/^Configuring\s+(.*)/i) { - $disk->{'model'} = $1; - } - elsif (/\s+(\S+):\s+([^, ]+)/ && - $1 eq $disk->{'device'}) { - $disk->{'raid'} = $2; - } - } - close(RD); - $disk->{'type'} = 'raid'; - $disk->{'prefix'} = $disk->{'device'}.'p'; - } - elsif ($disk->{'device'} =~ /\/(ida\/c(\d+)d(\d+))/) { - # Compaq RAID device - local ($ic, $id) = ($2, $3); - $disk->{'desc'} = &text('select_cpq', $ic, $id); - open(IDA, -d "/proc/driver/array" ? ") { - if (/^(\S+):\s+(.*)/ && $1 eq "ida$ic") { - $disk->{'model'} = $2; - } - } - close(IDA); - $disk->{'type'} = 'raid'; - $disk->{'prefix'} = $disk->{'device'}.'p'; - } - elsif ($disk->{'device'} =~ /\/(cciss\/c(\d+)d(\d+))/) { - # Compaq Smart Array RAID - local ($ic, $id) = ($2, $3); - $disk->{'desc'} = &text('select_smart', $ic, $id); - open(CCI, ") { - if (/^\s*(\S+):\s*(.*)/ && $1 eq "cciss$ic") { - $disk->{'model'} = $2; - } - } - close(CCI); - $disk->{'type'} = 'raid'; - $disk->{'prefix'} = $disk->{'device'}.'p'; - } - elsif ($disk->{'device'} =~ /\/(ataraid\/disc(\d+)\/disc)/) { - # Promise RAID controller - local $dd = $2; - $disk->{'desc'} = &text('select_promise', $dd); - $disk->{'type'} = 'raid'; - $disk->{'prefix'} =~ s/disc$/part/g; - } - elsif ($disk->{'device'} =~ /\/nvme(\d+)n(\d+)$/) { - # NVME SSD controller - $disk->{'desc'} = &text('select_nvme', "$1", "$2"); - $disk->{'type'} = 'scsi'; - $disk->{'prefix'} = $disk->{'device'}.'p'; - } - - # Work out short name, like sda - local $short; - if (defined($disk->{'host'})) { - $short = &hbt_to_device($disk->{'host'}, - $disk->{'bus'}, - $disk->{'target'}); - } - else { - $short = $disk->{'device'}; - $short =~ s/^.*\///g; - } - $disk->{'short'} = $short; - - $disk->{'id'} = $id_map{$disk->{'device'}} || - $id_map{"/dev/$short"}; - $disk->{'ids'} = $all_id_map{$disk->{'device'}} || - $all_id_map{"/dev/$short"}; + $set_metadata->($disk); push(@disks, $disk); } @@ -600,66 +719,39 @@ while() { } close(FDISK); -# Check /proc/ide for IDE disk models -foreach $d (@disks) { - if ($d->{'type'} eq 'ide') { - local $short = $d->{'short'}; - $d->{'model'} = &read_file_contents("/proc/ide/$short/model"); - $d->{'model'} =~ s/\r|\n//g; - $d->{'media'} = &read_file_contents("/proc/ide/$short/media"); - $d->{'media'} =~ s/\r|\n//g; - if ($d->{'short'} =~ /^vd/ && !$d->{'model'}) { - # Fake up model for KVM VirtIO disks - $d->{'model'} = "KVM VirtIO"; - } - } - } - -# Fill in SCSI information -foreach $d (@disks) { - if ($d->{'type'} eq 'scsi') { - local $s = $d->{'scsi'}; - local $sysdir = "/sys/block/$d->{'short'}/device"; - if (-d $sysdir) { - # From kernel 2.6.30+ sys directory - $d->{'model'} = &read_file_contents("$sysdir/vendor"). - " ". - &read_file_contents("$sysdir/model"); - $d->{'model'} =~ s/\r|\n//g; - $d->{'media'} = &read_file_contents("$sysdir/media"); - $d->{'media'} =~ s/\r|\n//g; - } - elsif ($dscsi_mode) { - # From other scsi files - $d->{'model'} = "$dscsi[$s]->{'make'} $dscsi[$s]->{'model'}"; - $d->{'controller'} = $dscsi[$s]->{'host'}; - $d->{'scsiid'} = $dscsi[$s]->{'target'}; - } - else { - # From /proc/scsi/scsi lines - if ($pscsi[$s] =~ /Vendor:\s+(\S+).*Model:\s+(.*)\s+Rev:/i) { - $d->{'model'} = "$1 $2"; - } - if ($pscsi[$s] =~ /Host:\s+scsi(\d+).*Id:\s+(\d+)/i) { - $d->{'controller'} = int($1); - $d->{'scsiid'} = int($2); - } - } - if ($d->{'model'} =~ /ATA/) { - # Fake SCSI disk, actually IDE - $d->{'scsi'} = 0; - $d->{'desc'} =~ s/SCSI/SATA/g; - foreach my $p (@{$d->{'parts'}}) { - $p->{'desc'} =~ s/SCSI/SATA/g; - } - } - } - } +$enrich_metadata->(@disks); @list_disks_partitions_cache = @disks; return @disks; } +# Report disk-only failure without changing the list-returning caller contract. +sub disk_discovery_error +{ +my ($message, $error_ref) = @_; +if ($error_ref) { + $$error_ref = $message; + } +else { + warn "$message\n"; + } +return (); +} + +# Compare device-node and sysfs object identity without opening the device. +# This detects disappearance/replacement during enumeration, not all possible +# changes after this function returns. The next collection always scans again. +sub disk_discovery_identity +{ +my ($device) = @_; +my @node = stat($device); +return if (!@node); +my $short = $device; +$short =~ s/^.*\///g; +my @sys = stat("/sys/class/block/$short"); +return join(':', @node[0,1,6], @sys ? @sys[0,1] : ('', '')); +} + # partition_description(device) # Converts a device path like /dev/hda1 into a human-readable name sub partition_description diff --git a/fdisk/t/README.md b/fdisk/t/README.md new file mode 100644 index 000000000..bb25303cd --- /dev/null +++ b/fdisk/t/README.md @@ -0,0 +1,7 @@ +# Disk-only discovery tests + +Run `prove -lr fdisk/t`. Perl::Critic enables lint checks for the added tests. +Fixtures use temporary kernel/device trees and fake partition commands; they +never query real disks. Coverage includes GPT/MBR, fdisk fallback, cache isolation, +fresh inventory and identifiers, disappearance, invalid/empty inventory, and +non-throwing error reporting. Physical RAID and real hotplug need hardware tests. diff --git a/fdisk/t/disk-discovery-perlcritic.t b/fdisk/t/disk-discovery-perlcritic.t new file mode 100644 index 000000000..05052b549 --- /dev/null +++ b/fdisk/t/disk-discovery-perlcritic.t @@ -0,0 +1,16 @@ +use strict; +use warnings; +use Test::More; +use FindBin qw($Bin); +BEGIN { + eval { require Perl::Critic; 1 } + or plan skip_all => 'Perl::Critic not installed'; +} +# Apply the existing policy to new tests, not untouched legacy module files. +my $critic = Perl::Critic->new(-profile => "$Bin/../../.perlcriticrc"); +for my $file (qw(disk-discovery.t fixtures/disk-discovery.pl disk-discovery-perlcritic.t)) { + my @violations = $critic->critique("$Bin/$file"); + is(scalar @violations, 0, "$file perlcritic"); + diag join('', @violations) if @violations; +} +done_testing(); diff --git a/fdisk/t/disk-discovery.t b/fdisk/t/disk-discovery.t new file mode 100644 index 000000000..3a53ae99d --- /dev/null +++ b/fdisk/t/disk-discovery.t @@ -0,0 +1,197 @@ +use strict; +use warnings; +use Test::More; +use JSON::PP; +use File::Temp qw(tempdir); +use File::Path qw(make_path remove_tree); +use File::Basename qw(dirname); +use Cwd qw(abs_path); +use IPC::Open3; +use Symbol qw(gensym); + +my $here = abs_path(dirname(__FILE__)); +my $library = "$here/../fdisk-lib.pl"; +my $fixture; + +sub write_file { + my ($name, $value) = @_; + make_path(dirname("$fixture/$name")); + open(my $fh, '>', "$fixture/$name") or die $!; + print {$fh} $value; + close($fh) or die $!; +} +sub disk { + my ($name, $vendor, $model) = @_; + write_file("dev/$name", ''); + write_file("sys/block/$name/device/vendor", "$vendor\n"); + write_file("sys/block/$name/device/model", "$model\n"); + make_path("$fixture/sys/class/block/$name"); +} +sub setup_fixture { + $fixture = tempdir(CLEANUP => 1); + write_file('proc/partitions', "major minor #blocks name\n 8 0 100 sda\n 8 1 90 sda1\n"); + write_file('proc/scsi/scsi', "Attached devices:\n"); + disk('sda', 'Example', 'USB NVMe'); + disk('sdb', 'ATA', 'second'); + make_path("$fixture/dev/disk/by-id"); + symlink('../../sda', "$fixture/dev/disk/by-id/old") or die $!; + write_file('tool-output', ''); + for my $tool (qw(parted fdisk)) { + write_file("bin/$tool", "#!$^X\n" . <<'TOOL'); +use strict; +my $root = $ENV{'FIXTURE_ROOT'}; +open(my $calls, '>>', "$root/calls") or die $!; +print {$calls} "query\n"; +close($calls); +open(my $data, '<', "$root/tool-output") or die $!; +print while <$data>; +TOOL + chmod(0755, "$fixture/bin/$tool") or die $!; + } +} +sub run_library { + my ($scenario) = @_; + local $ENV{'PATH'} = "$fixture/bin:$ENV{'PATH'}"; + local $ENV{'FIXTURE_ROOT'} = $fixture; + local $SIG{'ALRM'} = sub { die "fixture timed out\n"; }; + alarm(15); + my $err = gensym(); + my $pid = open3(my $in, my $out, $err, $^X, "$here/fixtures/disk-discovery.pl", + $library, $fixture, $scenario || 'disk'); + close($in); + my ($stdout, $stderr); + { local $/; $stdout = <$out>; $stderr = <$err>; } + waitpid($pid, 0); + my $status = $?; + alarm(0); + is($status, 0, 'fixture process succeeds') or diag($stderr); + is($stderr, '', 'no unexpected warnings'); + my $result = decode_json($stdout); + is($result->{'exception'}, undef, 'library does not throw an exception'); + return $result; +} +sub parted_output { + my ($table) = @_; + my $part = $table eq 'gpt' ? ' 1 1cyl 9cyl 8cyl ext4 name' : ' 1 1cyl 9cyl 8cyl primary ext4'; + my $extra = $table eq 'msdos' ? " 2 9cyl 10cyl 1cyl extended\n" : ''; + write_file('tool-output', "Disk $fixture/dev/sda: 10cyl\nBIOS cylinder,head,sector geometry: 10,4,8. Each cylinder is 16384b.\nPartition Table: $table\n$part\n$extra"); +} + +subtest 'disk-only inventory avoids partition commands and cache' => sub { + setup_fixture(); + my $r = run_library(); + is($r->{'error'}, undef, 'discovery succeeds'); + my $d = $r->{'results'}->[0]->[0]; + is($d->{'model'}, 'Example USB NVMe', 'model retained'); + is_deeply($d->{'ids'}, ['old'], 'stable ID retained'); + ok(!exists($d->{'parts'}) && !exists($d->{'table'}) && !exists($d->{'size'}), + 'metadata does not claim partition or geometry data'); + is_deeply($r->{'cache'}, [], 'full partition cache not populated'); + ok(!-e "$fixture/calls", 'no partition tool called'); +}; +subtest 'full discovery still parses GPT, MBR and extended partitions' => sub { + for my $table (qw(gpt msdos)) { + setup_fixture(); parted_output($table); + my $r = run_library('full'); + is($r->{'error'}, undef, 'full discovery succeeds'); + my $d = $r->{'results'}->[0]->[0]; + is($d->{'table'}, $table, 'partition table retained'); + is(scalar(@{$d->{'parts'}}), $table eq 'gpt' ? 1 : 2, 'partitions retained'); + is($d->{'model'}, 'Example USB NVMe', 'shared model metadata retained'); + ok(-e "$fixture/calls", 'full path queries partition tool'); + } +}; +subtest 'full fdisk fallback remains available' => sub { + setup_fixture(); + write_file('tool-output', "Disk $fixture/dev/sda: 1 GiB, 1073741824 bytes, 2097152 sectors\nGeometry: 4 heads, 8 sectors/track, 10 cylinders\nUnits = cylinders of 32 * 512\nDisklabel type: gpt\n$fixture/dev/sda1 1 9 9 4K Linux filesystem\n"); + my $r = run_library('full_fdisk'); + is($r->{'error'}, undef, 'fdisk fallback succeeds'); + is(scalar(@{$r->{'results'}->[0]->[0]->{'parts'}}), 1, 'fdisk partition retained'); + ok(-e "$fixture/calls", 'fdisk command used'); +}; +subtest 'cache isolation in both call orders' => sub { + for my $scenario (qw(disk_full full_disk)) { + setup_fixture(); parted_output('gpt'); + my $r = run_library($scenario); + my ($disk, $full) = @{$r->{'results'}}; + ($disk, $full) = ($full, $disk) if $scenario eq 'full_disk'; + ok(!exists($disk->[0]->{'parts'}), 'disk-only result has no partition claim'); + ok(@{$full->[0]->{'parts'}}, 'full result has partitions'); + is_deeply($r->{'cache'}, $full, 'full cache contains only full result'); + } +}; +subtest 'replacement refreshes model and identifiers' => sub { + setup_fixture(); my $r = run_library('refresh'); + my ($a, $b) = map { $_->[0] } @{$r->{'results'}}; + is($a->{'model'}, 'Example USB NVMe', 'first model'); + is($b->{'model'}, 'Example Replacement', 'new model'); + is_deeply($b->{'ids'}, ['new'], 'new stable ID'); +}; +subtest 'addition and removal refresh inventory' => sub { + setup_fixture(); my $r = run_library('remove_add'); + is($r->{'results'}->[0]->[0]->{'device'}, "$fixture/dev/sda", 'initial disk'); + is($r->{'results'}->[1]->[0]->{'device'}, "$fixture/dev/sdb", 'replacement inventory'); +}; +subtest 'disappearance during scan fails explicitly' => sub { + setup_fixture(); like(run_library('race')->{'error'}, qr/Missing device/, 'no stale result'); +}; +subtest 'missing model and missing inventory fail explicitly' => sub { + setup_fixture(); + unlink("$fixture/sys/block/sda/device/vendor", "$fixture/sys/block/sda/device/model"); + like(run_library()->{'error'}, qr/Missing disk model/, 'missing model fails'); + unlink("$fixture/proc/partitions"); + like(run_library()->{'error'}, qr/Cannot open disk inventory/, 'missing inventory fails'); +}; +subtest 'multiple drives and namespaces retain classifications' => sub { + setup_fixture(); + my @names = qw(sda sdaa nvme0n1 nvme0n2 mmcblk0 xvda vda hda); + write_file('proc/partitions', "major minor #blocks name\n" . join('', map { "8 0 100 $_\n" } @names)); + write_file('proc/ide/hda/media', "disk\n"); write_file('proc/ide/hda/model', "IDE fixture\n"); + disk($_, 'Example', 'disk') for @names; + my $r = run_library(); + is($r->{'error'}, undef, 'multi-drive discovery succeeds'); + is_deeply([sort map { $_->{'short'} } @{$r->{'results'}->[0]}], [sort @names], 'all devices retained'); + my ($virtio) = grep { $_->{'short'} eq 'vda' } @{$r->{'results'}->[0]}; + is($virtio->{'type'}, 'virtio', 'VirtIO classification preserved'); + ok(!-e "$fixture/calls", 'multiple drives need no partition query'); +}; +subtest 'unpartitioned disk without stable link remains discoverable' => sub { + setup_fixture(); unlink("$fixture/dev/disk/by-id/old"); + write_file('proc/partitions', "major minor #blocks name\n 8 0 100 sda\n"); + my $r = run_library(); + is(scalar(@{$r->{'results'}->[0]}), 1, 'disk retained'); + is($r->{'results'}->[0]->[0]->{'ids'}, undef, 'no fabricated identifiers'); +}; +subtest 'legacy controller metadata and SCSI fallback remain available' => sub { + setup_fixture(); + my @names = qw(cciss/c0d0 rd/c0d0 ida/c0d0); + write_file('proc/partitions', "major minor #blocks name\n" . join('', map { "8 0 100 $_\n" } @names)); + write_file("dev/$_", '') for @names; + write_file('proc/driver/cciss/cciss0', "cciss0: Smart Array\n"); + write_file('proc/rd/c0/current_status', "Configuring Mylex\n"); + write_file('proc/driver/cpqarray/ida0', "ida0: Compaq\n"); + my $r = run_library(); + is_deeply([map { $_->{'model'} } @{$r->{'results'}->[0]}], ['Smart Array','Mylex','Compaq'], 'controller models retained'); + ok(!grep({ $_->{'type'} ne 'raid' } @{$r->{'results'}->[0]}), 'legacy RAID classification retained'); + write_file('proc/partitions', "major minor #blocks name\n 8 0 100 sda\n"); + remove_tree("$fixture/sys/block/sda/device"); + write_file('proc/scsi/scsi', "Attached devices:\nHost: scsi0 Channel: 00 Id: 00 Lun: 00\n Vendor: LSI Model: RAID fixture Rev: 1\n Type: Direct-Access\n"); + like(run_library()->{'results'}->[0]->[0]->{'model'}, qr/LSI/, 'proc SCSI fallback retained'); +}; +subtest 'failure reporting preserves the list-returning contract' => sub { + setup_fixture(); + unlink("$fixture/dev/sda"); + my $r = run_library('warn'); + is_deeply($r->{'results'}, [[]], 'unreported-reference failure returns no partial disks'); + is(scalar @{$r->{'warnings'}}, 1, 'caller without error reference receives one warning'); + like($r->{'warnings'}->[0], qr/Missing device/, 'warning explains the failure'); + setup_fixture(); + is(run_library('reset_error')->{'error'}, undef, 'successful disk-only call clears stale error'); + write_file('proc/partitions', "invalid inventory\n"); + like(run_library()->{'error'}, qr/Invalid disk inventory/, 'invalid header is reported'); + write_file('proc/partitions', "major minor #blocks name\n"); + $r = run_library(); + is_deeply($r->{'results'}, [[]], 'empty valid inventory succeeds'); + is($r->{'error'}, undef, 'empty valid inventory is not an error'); +}; +done_testing(); diff --git a/fdisk/t/fixtures/disk-discovery.pl b/fdisk/t/fixtures/disk-discovery.pl new file mode 100644 index 000000000..92743da27 --- /dev/null +++ b/fdisk/t/fixtures/disk-discovery.pl @@ -0,0 +1,77 @@ +use strict; +use warnings; +use JSON::PP; +use Cwd qw(abs_path); +use File::Basename qw(dirname); +my ($source, $root, $scenario) = @ARGV; +open(my $fh, '<', $source) or die $!; +local $/; my $code = <$fh>; close($fh); $/ = "\n"; +# Keep the actual library functions; replace literal filesystem roots only. +$code = substr($code, index($code, 'sub list_disks_partitions')); +for my $prefix ('/proc/', '/sys/', '/dev/') { + $code =~ s/\Q$prefix\E/$root$prefix/g; +} +our (@reads, $has_parted, @list_disks_partitions_cache, $change); +$has_parted = $scenario !~ /fdisk/; +# Mock the cross-module filesystem helper used by the real library. +{ package mount; ## no critic (Modules::RequireFilenameMatchesPackage) + sub list_fstypes { return ('ext2', 'ext3', 'ext4', 'xfs'); } +} +sub text { return join(' ', @_); } +sub indexof { my ($x, @v) = @_; for (0..$#v) { return $_ if $v[$_] eq $x; } return -1; } +sub resolve_links { return abs_path($_[0]); } +sub simplify_path { my $p = $_[0]; $p =~ s{/dev/disk/by-id/../../}{/dev/}; return $p; } +sub read_file_contents { + my ($p) = @_; + die "Attempt to read device data" if $p =~ m{\Q$root\E/dev/}; + push(@reads, $p); + open(my $f, '<', $p) or return ''; + local $/; my $s = <$f>; close($f); + if ($change && $p =~ m{/device/model$}) { + unlink("$root/dev/sda"); + $change = 0; + } + return $s; +} +{ + # Load the real legacy functions, which use Webmin package globals. + no strict 'vars'; ## no critic (TestingAndDebugging::ProhibitNoStrict) + # Webmin does not enable these warnings for optional legacy metadata. + no warnings 'uninitialized'; + eval $code; ## no critic (BuiltinFunctions::ProhibitStringyEval) + die $@ if $@; +} +my @results; +my $error; +my $exception; +my @warnings; +eval { + if ($scenario eq 'race') { $change = 1; } + if ($scenario eq 'reset_error') { $error = 'previous failure'; } + if ($scenario eq 'disk_full' || $scenario eq 'full_disk') { + my $first = $scenario eq 'disk_full' ? 1 : 0; + push(@results, [list_disks_partitions(undef, $first, \$error)]); + push(@results, [list_disks_partitions(undef, !$first, \$error)]); + } + elsif ($scenario eq 'refresh') { + push(@results, [list_disks_partitions(undef, 1, \$error)]); + open(my $f, '>', "$root/sys/block/sda/device/model") or die $!; + print $f "Replacement\n"; close($f); + unlink("$root/dev/disk/by-id/old"); + symlink('../../sda', "$root/dev/disk/by-id/new") or die $!; + push(@results, [list_disks_partitions(undef, 1, \$error)]); + } + elsif ($scenario eq 'remove_add') { + push(@results, [list_disks_partitions(undef, 1, \$error)]); + open(my $f, '>', "$root/proc/partitions") or die $!; + print $f "major minor #blocks name\n 8 16 100 sdb\n"; close($f); + push(@results, [list_disks_partitions(undef, 1, \$error)]); + } + elsif ($scenario eq 'warn') { + local $SIG{__WARN__} = sub { push(@warnings, @_); }; + push(@results, [list_disks_partitions(undef, 1)]); + } + else { push(@results, [list_disks_partitions(undef, $scenario =~ /^full/ ? 0 : 1, \$error)]); } + 1; +} or $exception = $@; +print JSON::PP->new->canonical->encode({results=>\@results, error=>$error, exception=>$exception, warnings=>\@warnings, reads=>\@reads, cache=>\@list_disks_partitions_cache}); From 74298f01d8f63e6b395415ce80e5d485727a1687 Mon Sep 17 00:00:00 2001 From: Aaron Anderson Date: Wed, 14 Oct 2026 10:30:22 -0500 Subject: [PATCH 2/2] Preserve controller-qualified disk identities and canonical test fixtures --- fdisk/fdisk-lib.pl | 9 ++++++--- fdisk/t/README.md | 4 +++- fdisk/t/disk-discovery.t | 28 +++++++++++++++++++++++++++- fdisk/t/fixtures/disk-discovery.pl | 13 +++++++++++++ 4 files changed, 49 insertions(+), 5 deletions(-) diff --git a/fdisk/fdisk-lib.pl b/fdisk/fdisk-lib.pl index 535c31157..4cdbc24ce 100755 --- a/fdisk/fdisk-lib.pl +++ b/fdisk/fdisk-lib.pl @@ -41,8 +41,10 @@ else { $| = 1; # list_disks_partitions([include-cds], [disk-only], [error-ref]) -# Disk-only failures return no disks; error-ref receives the reason if supplied. -# Returns a structure containing the details of all disks and partitions +# Returns a list of disk hash refs; disk-only mode omits partition metadata. +# Failure returns an empty list, never an error string. The optional error-ref +# is a scalar reference that receives the reason without changing the return type. +# Example: my @disks = list_disks_partitions(undef, 1, \$error); sub list_disks_partitions { local ($include_cds, $disk_only) = @_; @@ -747,7 +749,8 @@ my ($device) = @_; my @node = stat($device); return if (!@node); my $short = $device; -$short =~ s/^.*\///g; +$short =~ s{^/dev/}{}; +$short =~ s{/}{!}g; my @sys = stat("/sys/class/block/$short"); return join(':', @node[0,1,6], @sys ? @sys[0,1] : ('', '')); } diff --git a/fdisk/t/README.md b/fdisk/t/README.md index bb25303cd..fcee9bf70 100644 --- a/fdisk/t/README.md +++ b/fdisk/t/README.md @@ -4,4 +4,6 @@ Run `prove -lr fdisk/t`. Perl::Critic enables lint checks for the added tests. Fixtures use temporary kernel/device trees and fake partition commands; they never query real disks. Coverage includes GPT/MBR, fdisk fallback, cache isolation, fresh inventory and identifiers, disappearance, invalid/empty inventory, and -non-throwing error reporting. Physical RAID and real hotplug need hardware tests. +non-throwing error reporting, controller-qualified sysfs replacement with a retained +device node, and canonical identities through symlinked temporary parents. Physical +RAID and real hotplug need hardware tests. diff --git a/fdisk/t/disk-discovery.t b/fdisk/t/disk-discovery.t index 3a53ae99d..c85b2ccf6 100644 --- a/fdisk/t/disk-discovery.t +++ b/fdisk/t/disk-discovery.t @@ -28,7 +28,8 @@ sub disk { make_path("$fixture/sys/class/block/$name"); } sub setup_fixture { - $fixture = tempdir(CLEANUP => 1); + my ($parent) = @_; + $fixture = abs_path(tempdir(CLEANUP => 1, defined($parent) ? (DIR => $parent) : ())); write_file('proc/partitions', "major minor #blocks name\n 8 0 100 sda\n 8 1 90 sda1\n"); write_file('proc/scsi/scsi', "Attached devices:\n"); disk('sda', 'Example', 'USB NVMe'); @@ -194,4 +195,29 @@ subtest 'failure reporting preserves the list-returning contract' => sub { is_deeply($r->{'results'}, [[]], 'empty valid inventory succeeds'); is($r->{'error'}, undef, 'empty valid inventory is not an error'); }; +subtest 'controller-qualified sysfs replacement is detected with a retained device node' => sub { + setup_fixture(); + write_file('proc/partitions', "major minor #blocks name\n 104 0 100 cciss/c0d0\n"); + write_file('dev/cciss/c0d0', ''); + write_file('proc/driver/cciss/cciss0', "cciss0: Smart Array\n"); + make_path("$fixture/sys/class/block/cciss!c0d0"); + my $stable = run_library('legacy_stable'); + is($stable->{'error'}, undef, 'controller sysfs path is accepted'); + my $before = [stat("$fixture/dev/cciss/c0d0")]; + my $changed = run_library('legacy_replace'); + like($changed->{'error'}, qr/Disk identity changed/, 'sysfs replacement is detected'); + is_deeply($changed->{'results'}, [[]], 'replacement returns no stale inventory'); + my $after = [stat("$fixture/dev/cciss/c0d0")]; + is_deeply([@{$after}[0,1,6]], [@{$before}[0,1,6]], 'device-node identity was retained'); +}; +subtest 'symlinked temporary parents produce canonical fixture identities' => sub { + my $parent = abs_path(tempdir(CLEANUP => 1)); + make_path("$parent/real"); + symlink("$parent/real", "$parent/alias") or die $!; + setup_fixture("$parent/alias"); + is($fixture, abs_path($fixture), 'fixture root is canonical'); + my $result = run_library(); + is($result->{'error'}, undef, 'discovery succeeds through symlinked parent'); + is_deeply($result->{'results'}->[0]->[0]->{'ids'}, ['old'], 'resolved stable IDs match canonical devices'); +}; done_testing(); diff --git a/fdisk/t/fixtures/disk-discovery.pl b/fdisk/t/fixtures/disk-discovery.pl index 92743da27..7802bd2fb 100644 --- a/fdisk/t/fixtures/disk-discovery.pl +++ b/fdisk/t/fixtures/disk-discovery.pl @@ -41,6 +41,19 @@ sub read_file_contents { eval $code; ## no critic (BuiltinFunctions::ProhibitStringyEval) die $@ if $@; } +if ($scenario eq 'legacy_replace') { + my $identity = \&disk_discovery_identity; + my $calls = 0; + no warnings 'redefine'; + *disk_discovery_identity = sub { + if (++$calls == 2) { + my $sys = "$root/sys/class/block/cciss!c0d0"; + rename($sys, "$sys.old") or die $!; + mkdir($sys) or die $!; + } + return $identity->(@_); + }; +} my @results; my $error; my $exception;