From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id D3C431FF0AF for ; Fri, 06 Nov 2026 02:27:30 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 3EF0A216A4; Fri, 06 Nov 2026 02:27:27 +0100 (CET) From: Thomas Lamprecht To: pve-devel@lists.proxmox.com Subject: [PATCH v3 storage 41/76] api: disks: guard SAN wipes with stable identities and storage checks Date: Fri, 6 Nov 2026 00:31:16 +0100 Message-ID: <20261105233336.2178064-42-t.lamprecht@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261105233336.2178064-1-t.lamprecht@proxmox.com> References: <20261105233336.2178064-1-t.lamprecht@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793926180254 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.188 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_1 0.1 random spam to be learned in bayes RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record X-MailFrom: t.lamprecht@proxmox.com X-Mailman-Rule-Hits: max-size X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; news-moderation; no-subject; digests; suspicious-header Message-ID-Hash: P77BIWGQZD7UPEJ23N5HJAI337U4NNBH X-Message-ID-Hash: P77BIWGQZD7UPEJ23N5HJAI337U4NNBH X-Mailman-Approved-At: Fri, 06 Nov 2026 02:27:17 +0100 X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: A SAN LUN without local users may still belong to storage used on another node. Before wiping, check storage references and PV identities under the cluster storage configuration lock. Read PV membership without LVM hints; missing volume-group metadata does not prove a PV is orphaned. Retain SAN identities across worker delays, require usable paths before probes, and wipe same-LUN partition maps before removing them. Expose usage hints and verified device references in the scan, while keeping the worker checks authoritative. Local wipes remain independent of quorum. A path without an identity, such as a local disk that multipathd ignores, counts as a possible path of the LUN: the write is refused unless the cached VPD page of the path shows another logical unit. Apply the same identity, readiness, and ownership checks before each SAN volume-group initialization write, through both the disk API and storage-add. Accept only whole LUNs and reject raw guest disks and LUNs on targets used by storage for disk images. pvcreate and vgcreate cannot use an open descriptor. Give them a checked, identity-bearing by-id name, but a race remains before LVM resolves and opens it. Cluster locks do not serialize hosts outside the cluster. Signed-off-by: Thomas Lamprecht --- src/PVE/API2/Disks.pm | 35 +- src/PVE/API2/Disks/LVM.pm | 7 + src/PVE/API2/Storage/Scan.pm | 24 + src/PVE/Diskmanage.pm | 390 ++++++++++++- src/PVE/Multipath/Identity.pm | 119 +++- src/PVE/Storage.pm | 219 ++++++++ src/PVE/Storage/LVMPlugin.pm | 27 +- src/PVE/Storage/SANScan.pm | 7 + src/test/Makefile | 15 +- src/test/disk_api_test.pm | 1 + src/test/run_block_device_users_tests.pl | 11 + src/test/run_lun_identity_tests.pl | 214 +++++++ src/test/run_san_lun_scan_tests.pl | 26 + src/test/run_san_lvm_create_tests.pl | 453 +++++++++++++-- src/test/run_san_wipe_tests.pl | 673 +++++++++++++++++++++++ src/test/run_test_lvm_base_volume.pl | 8 +- src/test/run_wipe_partition_tests.pl | 297 +++++++++- src/test/run_wipe_readiness_tests.pl | 138 +++++ src/test/run_wipe_reference_tests.pl | 168 +++++- src/test/run_wipedisk_tests.pl | 58 +- 20 files changed, 2743 insertions(+), 147 deletions(-) create mode 100755 src/test/run_san_wipe_tests.pl create mode 100755 src/test/run_wipe_readiness_tests.pl diff --git a/src/PVE/API2/Disks.pm b/src/PVE/API2/Disks.pm index ebc73103..37a2df31 100644 --- a/src/PVE/API2/Disks.pm +++ b/src/PVE/API2/Disks.pm @@ -321,7 +321,7 @@ __PACKAGE__->register_method({ disk => { type => 'string', description => "Block device name", - pattern => '^/dev/[a-zA-Z0-9\/]+$', + pattern => '^/dev/[a-zA-Z0-9._:/-]+$', }, }, }, @@ -333,18 +333,39 @@ __PACKAGE__->register_method({ my $expected = PVE::Diskmanage::block_device_reference($reference); my $disk = $expected->{device}; - my $mounted = PVE::Diskmanage::is_mounted($disk); - die "disk/partition '${mounted}' is mounted\n" if $mounted; - - my $held = PVE::Diskmanage::has_holder($disk); - die "disk/partition '${held}' has a holder\n" if $held; + my $users = PVE::Diskmanage::block_device_users( + [basename($disk)], + { 'partition-maps' => 1, 'metadata-only' => 1 }, + ); + die "cannot wipe '$disk' - " . join(', ', $users->@*) . "\n" if $users->@*; my $rpcenv = PVE::RPCEnvironment::get(); my $authuser = $rpcenv->get_user(); + # A SAN LUN may be another storage's, which is checked against the cluster-wide storage + # configuration under its lock. A local disk is not, and its wipe must work without quorum. + my $is_san = sub { + my ($dev) = @_; + return PVE::Diskmanage::is_san_device($dev) + || PVE::Diskmanage::is_multipath_device($dev); + }; + my $san = $is_san->($disk); + my $worker = sub { PVE::Diskmanage::locked_disk_action(sub { - PVE::Diskmanage::wipe_blockdev_checked($reference, $expected); + my $locked = $san ? \&PVE::Storage::lock_storage_config : sub { $_[0]->() }; + $locked->(sub { + my $cfg = $san ? PVE::Storage::config() : undef; + my $check_storage = $san + ? sub { + my ($disk, $partitions) = @_; + PVE::Storage::assert_san_write_safe($cfg, $disk, $partitions); + } + : undef; + PVE::Diskmanage::wipe_blockdev_checked( + $reference, $expected, $check_storage, + ); + }); }); }; diff --git a/src/PVE/API2/Disks/LVM.pm b/src/PVE/API2/Disks/LVM.pm index e2cc8118..6129df56 100644 --- a/src/PVE/API2/Disks/LVM.pm +++ b/src/PVE/API2/Disks/LVM.pm @@ -160,6 +160,7 @@ __PACKAGE__->register_method({ || PVE::Diskmanage::is_san_device($device); }; my $san = $is_san->($dev); + PVE::Storage::LVMPlugin::assert_whole_lun($dev) if $san; my $expected = $san ? PVE::Diskmanage::block_device_reference($reference) : undef; $dev = $expected->{device} if $san; @@ -189,6 +190,7 @@ __PACKAGE__->register_method({ my $create = sub { if ($san) { $dev = PVE::Diskmanage::verify_blockdev_path($reference); + PVE::Storage::LVMPlugin::assert_whole_lun($dev); $dev = PVE::Diskmanage::block_device_reference($reference, $expected) ->{device}; } @@ -208,6 +210,11 @@ __PACKAGE__->register_method({ san => $san, expected => $expected, }; + if ($san) { + my $cfg = PVE::Storage::config(); + $guard->{check_storage} = + sub { PVE::Storage::assert_san_write_safe($cfg, $_[0]) }; + } PVE::Storage::LVMPlugin::lvm_create_volume_group( $guard->{reference}, $name, diff --git a/src/PVE/API2/Storage/Scan.pm b/src/PVE/API2/Storage/Scan.pm index f6263150..e7f6eba8 100644 --- a/src/PVE/API2/Storage/Scan.pm +++ b/src/PVE/API2/Storage/Scan.pm @@ -513,6 +513,30 @@ __PACKAGE__->register_method({ type => 'string', optional => 1, }, + 'in-use' => { + description => + "Local usage indicated by mounts and devices using this device," + . " including its partitions and kpartx partition maps for the same LUN." + . " 'no' does not rule out open file descriptors, swap, or other uses" + . " checked before wiping. 'unknown' means the metadata check failed.", + type => 'string', + enum => ['yes', 'no', 'unknown'], + }, + 'in-use-by' => { + description => + "Local devices and mounts using this device, or why its usage" + . " could not be checked. Device mapper devices are identified by their" + . " map name when available.", + type => 'string', + }, + 'by-id' => { + description => + "A path under /dev/disk/by-id that encodes the device's identity" + . " and has been verified to resolve to this device. Absent when no such" + . " link could be verified.", + type => 'string', + optional => 1, + }, 'duplicate-pv' => { description => "Another listed LUN has the same LVM physical volume UUID, as" diff --git a/src/PVE/Diskmanage.pm b/src/PVE/Diskmanage.pm index d0e1dd0d..796d4eb4 100644 --- a/src/PVE/Diskmanage.pm +++ b/src/PVE/Diskmanage.pm @@ -70,6 +70,23 @@ sub block_device_number { return PVE::Tools::dev_t_major($st[6]) . ':' . PVE::Tools::dev_t_minor($st[6]); } +# Canonical udev names that encode the kernel's LUN identity, unlike model/serial aliases. +# Partition names carry the identity of the containing LUN as well as their partition number. +sub block_identity_from_id { + my ($name) = @_; + if ($name =~ m/^dm-uuid-(?:part([1-9]\d*)-)?mpath-([a-zA-Z0-9_.:+-]+)$/) { + return { key => "scsi:$2", partition => $1 // '' }; + } + if ( + $name =~ m/^nvme-((?:eui\.(?:[0-9a-f]{16}|[0-9a-f]{32}) + |uuid\.[0-9a-f]{8}(?:-[0-9a-f]{4}){3}-[0-9a-f]{12} + |nvme\.[0-9a-f]{4}-[0-9a-f]+-[0-9a-f]+-[0-9a-f]{8}))(?:-part([1-9]\d*))?$/x + ) { + return { key => "nvme:$1", partition => $2 // '' }; + } + return undef; +} + # The sysfs inode distinguishes device instances even when the kernel reuses a name and dev_t. sub block_device_stamp { my ($device, $sys) = @_; @@ -82,15 +99,64 @@ sub block_device_stamp { return "$ino:$number"; } -# Retain the device instance before queueing a worker; kernel names can be reused while it waits. +# Retain both the original reference and the identity it denotes before queueing a worker. Only +# identities established from kernel metadata may survive renumbering while the worker waits. sub block_device_reference { my ($reference, $expected, $sys) = @_; $sys //= '/sys'; my $device = verify_blockdev_path($reference); my $stamp = block_device_stamp($device, $sys); + my $whole = is_partition($device) ? get_blockdev($device) : $device; + my $name = basename($whole); + my $dir = "$sys/class/block/$name"; + my $partition = + $whole ne $device + ? file_read_firstline("$sys/class/block/" . basename($device) . '/partition') + : ''; + my ($identity, $dm_uuid); + + if ($name =~ m/^dm-\d+$/) { + my $uuid = file_read_firstline("$dir/dm/uuid") + // die "cannot use '$reference' - missing device mapper identity\n"; + # passed on to dmsetup, the daemons run with taint checks + ($dm_uuid) = $uuid =~ m/^([\w.:+-]+)$/ + or die "unable to identify '$device' - unexpected device mapper identity\n"; + $identity = block_identity_from_id("dm-uuid-$uuid"); + $identity //= { key => "dm:$uuid", partition => '' }; + } elsif ($name =~ m/^nvme\d+n\d+$/) { + my $wwid = file_read_firstline("$dir/wwid") + // die "cannot use '$reference' - missing namespace identity\n"; + $identity = { key => "nvme:$wwid", partition => $partition }; + } elsif ($name =~ m/^sd[a-z]+$/) { + require PVE::Multipath::Identity; + my $ids = PVE::Multipath::Identity::collect(all => 1); + my $wwid = PVE::Multipath::Identity::path_wwid($ids, $name); + $identity = { key => "scsi:$wwid", partition => $partition } if defined($wwid); + } + $identity //= { key => "instance:$stamp", partition => $partition }; + if ($reference =~ m{^/dev/disk/by-id/([^/]+)$}) { + my $id = $1; + my $encoded = block_identity_from_id($id); + if (!$encoded && $id =~ m/^scsi-(.+?)(?:-part([1-9]\d*))?$/) { + $encoded = { key => "scsi:$1", partition => $2 // '' }; + } elsif (!$encoded && $id =~ m/^wwn-0x([0-9a-f]+)(?:-part([1-9]\d*))?$/) { + my ($wwn, $part) = ($1, $2 // ''); + die "cannot use '$reference' - device identity does not match its by-id name\n" + if $identity->{key} !~ m/^scsi:[23]\Q$wwn\E$/; + $encoded = { key => $identity->{key}, partition => $part }; + } + die "cannot use '$reference' - device identity does not match its by-id name\n" + if $encoded + && ($encoded->{key} ne $identity->{key} + || $encoded->{partition} ne $identity->{partition}); + } die "cannot use '$reference' - device identity changed\n" - if $expected && $expected->{stamp} ne $stamp; - return { device => $device, stamp => $stamp }; + if $expected + && ($expected->{key} ne $identity->{key} + || $expected->{partition} ne $identity->{partition}); + die "cannot use '$reference' - device changed while identifying it\n" + if block_device_stamp($device, $sys) ne $stamp; + return { %$identity, device => $device, stamp => $stamp, dm_uuid => $dm_uuid }; } sub assert_block_device_instance { @@ -102,6 +168,107 @@ sub assert_block_device_instance { return; } +# A scan must not invent a stable name from a friendly mapper alias or an opaque serial string. +# Return a canonical identity-bearing link only while it resolves to the device being reported. +sub canonical_blockdev_link { + my ($device, $ids, $expected_wwid, $sys) = @_; + $sys //= '/sys'; + return eval { + $device = verify_blockdev_path($device); + my $stamp = block_device_stamp($device, $sys); + my $whole = is_partition($device) ? get_blockdev($device) : $device; + my $name = basename($whole); + my $suffix = ''; + if ($whole ne $device) { + my $part = + file_read_firstline("$sys/class/block/" . basename($device) . '/partition'); + die "invalid partition number\n" if !defined($part) || $part !~ m/^[1-9]\d*$/; + $suffix = "-part$part"; + } + my @links; + if ($name =~ m/^dm-\d+$/) { + my $uuid = file_read_firstline("$sys/class/block/$name/dm/uuid") // ''; + if ($uuid =~ m/^(?:part[1-9]\d*-)?mpath-([a-zA-Z0-9_.:+-]+)$/) { + die "map identity changed\n" if defined($expected_wwid) && $1 ne $expected_wwid; + my $id = "dm-uuid-$uuid"; + $id = "dm-uuid-part$1-$uuid" if $suffix =~ m/^-part([1-9]\d*)$/; + push @links, $id if block_identity_from_id($id); + } + } elsif ($name =~ m/^nvme\d+n\d+$/) { + my $wwid = file_read_firstline("$sys/class/block/$name/wwid") // ''; + push @links, "nvme-$wwid$suffix" if block_identity_from_id("nvme-$wwid$suffix"); + } elsif ($name =~ m/^sd[a-z]+$/) { + require PVE::Multipath::Identity; + $ids //= PVE::Multipath::Identity::collect(all => 1); + die "cannot inspect SAN identities\n" if $ids->{error}; + my $wwid = PVE::Multipath::Identity::path_wwid($ids, $name); + if (defined($wwid) && $wwid =~ m/^[a-zA-Z0-9_.:+-]+$/) { + die "path identity changed\n" + if defined($expected_wwid) && $wwid ne $expected_wwid; + push @links, "scsi-$wwid$suffix"; + push @links, "wwn-0x$1$suffix" if $wwid =~ m/^[23]([0-9a-f]+)$/; + } + } + my $link; + for my $id (@links) { + my ($path) = "/dev/disk/by-id/$id" =~ m{\A(/dev/disk/by-id/[a-zA-Z0-9_.:+-]+)\z}; + next if !defined($path); + next if (eval { verify_blockdev_path($path) } // '') ne $device; + $link = $path; + last; + } + die "device changed\n" if block_device_stamp($device, $sys) ne $stamp; + $link; + }; +} + +# This is a metadata-only hint, not a certificate that the full destructive-operation checks pass. +# With 'map-wwid', the device is the multipath map of that WWID and the users of its current paths +# count as well, for example a logical volume that LVM activated on a path before the map existed. +# A user found stays a user if a later check fails. +sub block_device_usage_hint { + my ($device, $options) = @_; + my %options = (%{ $options // {} }, 'partition-maps' => 1, 'metadata-only' => 1); + my $wwid = delete $options{'map-wwid'}; + my (@users, $error); + my $check = sub { + my ($code) = @_; + eval { $code->() }; + chomp($error //= $@) if $@; + }; + my $name; + $check->(sub { + $name = basename(verify_blockdev_path($device)); + push @users, block_device_users([$name], \%options)->@*; + }); + if (defined($wwid) && defined($name)) { + my $dir = ($options{sysfs} // '/sys') . "/class/block/$name"; + my $topology = sub { + my $uuid = file_read_firstline("$dir/dm/uuid") // ''; + die "'$name' is not the multipath map of '$wwid'\n" if $uuid ne "mpath-$wwid"; + opendir(my $dh, "$dir/slaves") or die "unable to list '$dir/slaves' - $!\n"; + my @paths = sort grep { $_ ne '.' && $_ ne '..' } readdir($dh); + closedir($dh); + return \@paths; + }; + my %path_options = ( + %options, 'ignore-holders' => { %{ $options{'ignore-holders'} // {} }, $name => 1 }, + ); + $check->(sub { + my $paths = $topology->(); + # one path at a time, so that a failed check keeps the users of the others + for my $path ($paths->@*) { + $check->(sub { push @users, block_device_users([$path], \%path_options)->@* }); + } + die "the paths of '$name' changed during the check\n" + if join(' ', $topology->()->@*) ne join(' ', $paths->@*); + }); + } + return { 'in-use' => 'yes', 'in-use-by' => join(', ', @users) } if @users; + return { 'in-use' => 'unknown', 'in-use-by' => "check failed: $error" } if defined($error); + return { 'in-use' => 'no', 'in-use-by' => '' }; +} + # Retain the checked device instance even if its kernel name or udev alias is reused. sub open_block_device { my ($device, $stamp) = @_; @@ -122,8 +289,9 @@ my sub block_entries { return [map { m/^([\w.:+-]+)$/ ? $1 : die "unexpected entry '$_' in '$dir'\n" } @names]; } +# Only exact same-LUN kpartx UUIDs make a holder a partition, not its name or its table type. sub block_device_partitions { - my ($name, $sys) = @_; + my ($name, $sys, $partition_maps) = @_; $sys //= '/sys'; my $dir = "$sys/class/block/$name"; my @parts; @@ -134,10 +302,22 @@ sub block_device_partitions { if !defined($number) || $number !~ m/^[1-9]\d*$/; push @parts, $part; } + if ($partition_maps && $name =~ m/^dm-\d+$/) { + my $uuid = file_read_firstline("$dir/dm/uuid") + // die "unable to identify '$name' - missing device mapper identity\n"; + if (my ($wwid) = $uuid =~ m/^(?:part[1-9]\d*-)?mpath-(\S+)$/) { + for my $holder (block_entries("$dir/holders")->@*) { + next if $holder !~ m/^dm-\d+$/; + my $child = file_read_firstline("$sys/class/block/$holder/dm/uuid") + // die "unable to identify holder '$holder'\n"; + push @parts, $holder if $child =~ m/^part[1-9]\d*-mpath-\Q$wwid\E$/; + } + } + } return \@parts; } -# Postorder keeps child partitions accessible until their signatures have been erased. +# Postorder keeps nested partition maps accessible until their own signatures have been erased. sub wipe_partitions { my ($device, $sys) = @_; $sys //= '/sys'; @@ -146,7 +326,7 @@ sub wipe_partitions { $visit = sub { my ($name) = @_; die "cannot inspect '$device' - cyclic partition topology\n" if $seen{$name}++; - for my $part (block_device_partitions($name, $sys)->@*) { + for my $part (block_device_partitions($name, $sys, 1)->@*) { $visit->($part); push @parts, $part; } @@ -156,6 +336,116 @@ sub wipe_partitions { return \@parts; } +# Readiness is checked before signature probes: even read-only I/O can stall on a queueing map. +# A map's direct member users matter too, including same-identity paths not yet added to the map. +sub assert_block_device_ready { + my ($device, $options) = @_; + $options //= {}; + my $sys = $options->{sysfs} // '/sys'; + my $whole = is_partition($device) ? get_blockdev($device) : $device; + my $name = basename($whole); + my $dir = "$sys/class/block/$name"; + my $size = file_read_firstline("$dir/size"); + die "cannot use '$device' - device is unavailable\n" + if !defined($size) || $size !~ m/^\d+$/ || !$size; + my $uuid = $name =~ m/^dm-\d+$/ ? file_read_firstline("$dir/dm/uuid") : undef; + die "cannot use '$device' - missing device mapper identity\n" + if $name =~ m/^dm-\d+$/ && !defined($uuid); + + if (defined($uuid) && $uuid =~ m/^part[1-9]\d*-mpath-/) { + my $parents = block_entries("$dir/slaves"); + die "cannot use '$device' - cannot identify containing map\n" if @$parents != 1; + return assert_block_device_ready("/dev/$parents->[0]", $options); + } + if ($name =~ m/^nvme\d+n\d+$/) { + my $status = get_nvme_path_status()->{$name}; + my $state = file_read_firstline("$dir/device/state") // ''; + die "cannot use '$device' - namespace has no usable path\n" + if $status ? !$status->{active} : $state ne 'live'; + my $id = file_read_firstline("$dir/wwid") + // die "cannot use '$device' - missing namespace identity\n"; + for my $other ( + grep { /^nvme\d+n\d+$/ && $_ ne $name } + block_entries("$sys/class/block")->@* + ) { + my $other_id = file_read_firstline("$sys/class/block/$other/wwid") + // die "cannot use '$device' - unresolved namespace '$other'\n"; + die "cannot use '$device' - namespace has multiple devices without a selected map\n" + if $id eq $other_id; + } + return; + } + + my ($wwid) = defined($uuid) ? $uuid =~ m/^mpath-(\S+)$/ : (); + my %members; + if (defined($wwid)) { + require PVE::Multipath; + %members = map { $_ => 1 } block_entries("$dir/slaves")->@*; + my ($map) = grep { ($_->{sysfs} // '') eq $name && ($_->{wwid} // '') eq $wwid } + PVE::Multipath::get_maps()->@*; + my $usable = 0; + if ($map) { + for my $path (map { $_->{paths}->@* } $map->{'path-groups'}->@*) { + $usable++ + if $members{ $path->{dev} } + && ($path->{'dm-state'} // '') eq 'active' + && ($path->{'check-state'} // '') =~ m/^(?:ready|ghost)$/; + } + } + die "cannot use '$device' - multipath map has no usable path\n" if !$usable; + } elsif (!is_san_device($whole)) { + return; + } + + require PVE::Multipath::Identity; + my $ids = PVE::Multipath::Identity::collect(all => 1); + die "cannot use '$device' - cannot inspect sibling paths: $ids->{error}\n" if $ids->{error}; + $wwid //= PVE::Multipath::Identity::path_wwid($ids, $name) + // die "cannot use '$device' - unresolved LUN identity\n"; + for my $path (sort keys $ids->{paths}->%*) { + my $id = PVE::Multipath::Identity::path_wwid($ids, $path); + if (!defined($id)) { + next if PVE::Multipath::Identity::path_is_other_lun($ids, $path, $wwid); + die "cannot use '$device' - cannot exclude '$path' as another path of this LUN because" + . " its identity could not be verified; check the path identity and multipathd" + . " configuration\n"; + } + next if $id ne $wwid; + die "cannot use '$device' - LUN has multiple paths without a selected multipath map\n" + if !defined($uuid) && $path ne $name; + $members{$path} = 1 if defined($uuid); + } + if (defined($uuid)) { + my %namespace_ids; + for my $member (grep { /^nvme/ } keys %members) { + my $id = file_read_firstline("$sys/class/block/$member/wwid") + // die "cannot use '$device' - unresolved namespace '$member'\n"; + $namespace_ids{$id} = 1; + } + if (%namespace_ids) { + for my $other (grep { /^nvme\d+n\d+$/ } block_entries("$sys/class/block")->@*) { + my $id = file_read_firstline("$sys/class/block/$other/wwid") + // die "cannot use '$device' - unresolved namespace '$other'\n"; + $members{$other} = 1 if $namespace_ids{$id}; + } + } + my $users = block_device_users( + [sort keys %members], + { + %$options, + 'ignore-holders' => { $name => 1 }, + 'ignore-executables' => + { '/sbin/multipathd' => 1, '/usr/sbin/multipathd' => 1 }, + }, + ); + die "cannot use '$device' - " . join(', ', $users->@*) . "\n" if $users->@*; + } else { + my $state = file_read_firstline("$dir/device/state") // ''; + die "cannot use '$device' - path is not running\n" if $state ne 'running'; + } + return; +} + # Never read device data. Probe failures are not evidence of non-use. Excluding a holder must not # exclude direct users; multipathd can be exempted because its checkers keep member paths open. sub block_device_users { @@ -183,6 +473,11 @@ sub block_device_users { die "unable to read '$file' - missing value\n" if !defined($value); return $value; }; + my $display_name = sub { + my ($name) = @_; + return $name if $name !~ m/^dm-\d+$/; + return file_read_firstline("$sys/class/block/$name/dm/name") || $name; + }; my ($numbers, $seen, $users) = ({}, {}, []); my $visit; $visit = sub { @@ -192,16 +487,15 @@ sub block_device_users { my $dir = "$sys/class/block/$name"; my $dev = $read->("$dir/dev"); die "invalid device number for '$name'\n" if $dev !~ m/^\d+:\d+$/; - $numbers->{$dev} = $name; - for my $part ($entries->($dir)->@*) { - next if !-e "$dir/$part/partition"; - my $number = $read->("$dir/$part/partition"); - die "invalid partition number for '$part'\n" if $number !~ m/^[1-9]\d*$/; - $visit->($part); - } + my $display = $display_name->($name); + $numbers->{$dev} = $display; + my $parts = block_device_partitions($name, $sys, $options->{'partition-maps'}); + my %parts = map { $_ => 1 } $parts->@*; + $visit->($_) for $parts->@*; + for my $holder ($entries->("$dir/holders")->@*) { - next if $ignore->{$holder}; - push $users->@*, "$name held by $holder"; + next if $ignore->{$holder} || $parts{$holder}; + push $users->@*, "$display held by " . $display_name->($holder); $visit->($holder); } }; @@ -215,9 +509,12 @@ sub block_device_users { my (undef, undef, $dev, undef, $target) = split(/\s+/, $line); die "unable to check mounts - malformed mountinfo\n" if !defined($target) || $dev !~ m/^\d+:\d+$/ || $line !~ m/ - /; + $target =~ s/\\([0-7]{3})/chr(oct($1))/ge; push $users->@*, "$numbers->{$dev} mounted at $target" if defined($numbers->{$dev}); } + return $users if $options->{'metadata-only'}; + my $by_path = sub { my ($path, $what) = @_; return if $path eq ''; @@ -331,6 +628,27 @@ sub parse_wipefs_signatures { return $probe->{signatures}; } +sub parse_pv_report { + my ($json) = @_; + + my $report = decode_json($json); + die "Malformed pvs report\n" + if ref($report) ne 'HASH' + || ref($report->{report}) ne 'ARRAY' + || $report->{report}->@* != 1 + || ref($report->{report}->[0]) ne 'HASH' + || ref($report->{report}->[0]->{pv}) ne 'ARRAY'; + for my $pv ($report->{report}->[0]->{pv}->@*) { + die "Malformed PV in pvs report\n" + if ref($pv) ne 'HASH' + || grep { !defined($pv->{$_}) || ref($pv->{$_}) } qw(pv_name vg_name pv_in_use); + die "Invalid PV in pvs report\n" + if ($pv->{vg_name} ne '[unknown]' && $pv->{vg_name} !~ m/^[a-zA-Z0-9+_.-]*$/) + || $pv->{pv_in_use} !~ m/^(?:0|1|unknown|)$/; + } + return $report->{report}->[0]->{pv}; +} + # Read cached metadata only; never probe the device. sub udev_properties { my ($dev) = @_; @@ -774,6 +1092,15 @@ sub dir_is_empty { return 1; } +# Some controllers pad the NQN or derive it from padded serial/model fields; trim trailing space. +sub read_subsysnqn { + my ($path) = @_; + + my $nqn = file_read_firstline($path) // return undef; + $nqn =~ s/\s+$//; + return $nqn; +} + # Returns the NVMe transport (pcie, fc, tcp, rdma, loop) of a namespace, either directly from # its controller, or from any controller of its subsystem for native NVMe multipath setups. sub get_nvme_transport { @@ -1464,12 +1791,15 @@ sub change_parttype { run_command(['sgdisk', "-t${partnum}:${parttype}", $blockdev], errmsg => $err); } -# Caller holds the disk lock through verification, wiping and the device trigger. +# Caller holds the disk lock and, for SAN devices, the storage config lock. Without a storage +# callback the device was classified local; a topology change must not bypass that lock. sub wipe_blockdev_checked { - my ($reference, $expected) = @_; + my ($reference, $expected, $check_storage) = @_; my $current = block_device_reference($reference, $expected); my $disk = $current->{device}; + die "cannot wipe '$reference' - device topology changed\n" + if !$check_storage && (is_san_device($disk) || is_multipath_device($disk)); my $partitions = wipe_partitions($disk); my $children = { map { $_ => block_device_reference("/dev/$_") } $partitions->@* }; my $verify = sub { @@ -1481,18 +1811,17 @@ sub wipe_blockdev_checked { die "cannot wipe '$reference' - partition topology changed\n" if join(',', @remaining) ne join(',', $parts->@*); for my $part (@remaining) { - my $now = block_device_reference( - "/dev/$part", $children->{$part}, - ); + my $now = block_device_reference("/dev/$part", $children->{$part}); die "cannot wipe '$reference' - partition '$part' changed\n" if $now->{stamp} ne $children->{$part}->{stamp}; } }; + assert_block_device_ready($disk); $verify->(); my $handle = open_block_device($disk, $current->{stamp}); - my $users = block_device_users( - [basename($disk)], {}, - ); + $check_storage->($disk, $partitions) if $check_storage; + assert_block_device_ready($disk); + my $users = block_device_users([basename($disk)], { 'partition-maps' => 1 }); die "cannot wipe '$disk' - " . join(', ', $users->@*) . "\n" if $users->@*; $verify->(); wipe_blockdev( @@ -1519,7 +1848,7 @@ sub wipe_blockdev { my $current = wipe_partitions($devpath); die "cannot wipe '$devpath' - partition topology changed\n" if join(',', $current->@*) ne join(',', $partitions->@*); - my $users = block_device_users([basename($devpath)], {}); + my $users = block_device_users([basename($devpath)], { 'partition-maps' => 1 }); die "cannot wipe '$devpath' - " . join(', ', $users->@*) . "\n" if $users->@*; for my $part ($partitions->@*) { $verify->(); @@ -1528,8 +1857,17 @@ sub wipe_blockdev { my $path = $handle ? "/proc/$$/fd/" . fileno($handle) : "/dev/$part"; run_command(['wipefs', '--all', $path], errmsg => "error wiping '/dev/$part'"); close($handle) or die "error closing '/dev/$part' - $!\n" if $handle; + next if $part !~ m/^dm-\d+$/; + $verify->(); + my @selector = $child ? ('--uuid', $child->{dm_uuid}) : ("/dev/$part"); + # udev probes the wiped partition map right away and keeps it open meanwhile + run_command( + ['dmsetup', 'remove', '--retry', @selector], + errmsg => "error removing '/dev/$part'", + ); + $removed->{$part} = 1; } - # New holders make the parent unsafe even after its child signatures have been erased. + # Recreated partition maps, new holders and new openers all make the parent unsafe. $users = block_device_users([basename($devpath)], {}); die "cannot wipe '$devpath' - " . join(', ', $users->@*) . "\n" if $users->@*; } @@ -1591,7 +1929,7 @@ sub udevadm_trigger { return if scalar(@devs) == 0; - eval { run_command(['udevadm', 'trigger', @devs]); }; + eval { run_command(['udevadm', 'trigger', '--settle', @devs]); }; warn $@ if $@; } diff --git a/src/PVE/Multipath/Identity.pm b/src/PVE/Multipath/Identity.pm index 5fa6542d..ed02373e 100644 --- a/src/PVE/Multipath/Identity.pm +++ b/src/PVE/Multipath/Identity.pm @@ -81,7 +81,7 @@ sub resolve($disks, $daemon, $naa_ok) { my $unresolved = !$daemon; for my $name ($lun->{paths}->@*) { my $disk = $disks->{$name}; - $held->{ $disk->{map} } = 1 if defined($disk->{map}); + $held->{$_} = 1 for ($disk->{maps} // [$disk->{map} // ()])->@*; my $line = $daemon ? $daemon->{$name} : undef; if (line_identifies($disk, $line, $naa_ok)) { $listed->{ $line->{wwid} } = 1; @@ -178,13 +178,21 @@ my sub scsi_disks($sessions, $targets, $all) { my $devt = sysfs_line("$dir/block/$name/dev") // return; my $disk = { lun => int($lun), devt => $devt, hctl => $hctl, dir => $dir }; $disk->{target} = $target if defined($target); - # Read the holder through the SCSI device, so neither name can refer to another device - # meanwhile. - my (undef, $holder) = dir_glob_regex("$SYSFS/$dir/block/$name/holders", '(dm-\d+)'); - if (defined($holder)) { - my $uuid = sysfs_line("$dir/block/$name/holders/$holder/dm/uuid") // ''; - ($disk->{map}) = $uuid =~ m/^mpath-(\S+)$/; - } + # Read the holders through the SCSI device, so neither name can refer to another device + # meanwhile. Check every holder, as a logical volume can hold a path next to its map. + my %maps; + dir_glob_foreach( + "$SYSFS/$dir/block/$name/holders", + 'dm-\d+', + sub($holder, @) { + my $uuid = sysfs_line("$dir/block/$name/holders/$holder/dm/uuid") // ''; + $maps{$1} = 1 if $uuid =~ m/^mpath-(\S+)$/; + }, + ); + my @maps = sort keys %maps; + $disk->{map} = $maps[0] if @maps; + # several maps on one path leave its LUN unresolved + $disk->{maps} = \@maps if @maps > 1; $res->{$name} = $disk; }, ); @@ -237,10 +245,12 @@ sub collect(%param) { for my $lun (values %$luns) { for my $name ($lun->{paths}->@*) { my $disk = $disks->{$name}; + # claimed: a map or the daemon names an identity, even one that did not resolve $paths->{$name} = { $disk->%{qw(devt hctl)}, wwid => $lun->{wwid}, $lun->{unresolved} ? (unresolved => 1) : (), + defined($disk->{map}) || ($daemon && $daemon->{$name}) ? (claimed => 1) : (), }; } } @@ -296,4 +306,97 @@ sub path_wwid($ids, $name) { return $path->{wwid}; } +my sub valid_naa($value) { + my $naa = length($value) ? ord($value) >> 4 : 0; + return length($value) == 16 ? $naa == 6 : length($value) == 8 && grep { $naa == $_ } 2, 3, 5; +} + +# The VPD page 0x83 of the path $name of $ids, if the name denotes the collected device before and +# after reading it. +my sub current_page($ids, $name) { + my $path = $ids->{paths}->{$name} // return undef; + my $current = sub { + my $devt = sysfs_line("class/block/$name/dev"); + my $device = abs_path("$SYSFS/class/block/$name/device"); + return + defined($devt) + && defined($device) + && $devt eq $path->{devt} + && basename($device) eq $path->{hctl}; + }; + return undef if !$current->(); + my $page = eval { file_get_contents("$SYSFS/class/block/$name/device/vpd_pg83") }; + return undef if !$current->(); + return $page; +} + +# Return the binary NAA designators of the logical unit in VPD page 0x83, in hex, and the number of +# its other designators. Return undef for a malformed page, and if any other logical unit designator +# is malformed or of a kind that multipathd might turn into an NAA WWID ('3...'), for example a SCSI +# name string 'naa.'. +my sub unit_designators($page) { + my $designators = vpd_designators($page) // return undef; + my ($naa, $other) = ([], 0); + for my $designator ($designators->@*) { + my ($set, $association, $type, $value) = $designator->@*; + next if $association != 0; # target port and target device designators + if ($type == 3) { + return undef if $set != 1 || !valid_naa($value); + push $naa->@*, unpack('H*', $value); + next; + } + my $length = length($value); + # printable ASCII with at least one character, padded with spaces or NUL bytes + my $ascii = $value =~ m/\A[\x20-\x7e]*[\x21-\x7e][\x20-\x7e]*\0*\z/; + if ($type == 0) { # vendor specific + return undef if $set == 1 ? !$length : $set != 2 || !$ascii; + } elsif ($type == 1) { # T10 vendor identification + return undef if $set != 2 || $length < 8 || !$ascii; + } elsif ($type == 2) { # EUI-64 based + return undef if $set != 1 || ($length != 8 && $length != 12 && $length != 16); + } elsif ($type == 8) { # SCSI name string, only the ASCII forms of iSCSI and EUI names + my $name = $value =~ s/\0+\z//r; + return undef + if $set != 3 + || ($name !~ m/\Aiqn\.[0-9]{4}-[0-9]{2}\.[a-zA-Z0-9.:_-]+\z/ + && $name !~ m/\Aeui\.[0-9a-fA-F]{16}(?:[0-9a-fA-F]{8}){0,2}\z/); + } else { + return undef; + } + $other++; + } + return { naa => $naa, other => $other }; +} + +# Whether the path $name of $ids, which has no resolved identity, is not a path of the LUN with +# the NAA WWID $wwid. Only a path that neither a map nor multipathd names, for example a local disk +# that multipathd does not list, can qualify: an identity from the configured source that did not +# resolve may still be this LUN. Its VPD page 0x83 must name the logical unit only with designators +# that multipathd cannot turn into $wwid, and a current path of the LUN must have $wwid as a binary +# NAA designator, as other sources, like a SCSI name string or a configured udev property, can +# yield the same spelling. The kernel caches the page when it scans the device, as multipathd +# caches its identity. +sub path_is_other_lun($ids, $name, $wwid) { + my ($hex) = $wwid =~ m/^3((?:[0-9a-fA-F]{16}){1,2})$/ or return 0; + $hex = lc($hex); + my $path = $ids->{paths}->{$name} // return 0; + return 0 if $path->{claimed}; + my $unit = unit_designators(current_page($ids, $name)) // return 0; + return 0 if !$unit->{other} && !$unit->{naa}->@*; + return 0 if grep { $_ eq $hex } $unit->{naa}->@*; + for my $member (sort keys $ids->{paths}->%*) { + next if (path_wwid($ids, $member) // '') ne $wwid; + my $designators = vpd_designators(current_page($ids, $member)) // next; + return 1 if grep { + my ($set, $association, $type, $value) = $_->@*; + $set == 1 + && $association == 0 + && $type == 3 + && valid_naa($value) + && unpack('H*', $value) eq $hex + } $designators->@*; + } + return 0; +} + 1; diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm index 8cbe9308..cb7a62f9 100755 --- a/src/PVE/Storage.pm +++ b/src/PVE/Storage.pm @@ -329,6 +329,225 @@ sub derived_multipath_wwids { return $derived; } +# Compare whole LUN identities, not aliases or the spelling of a configured base volume. A +# partition belongs to its containing LUN for storage-reference checks. +sub san_device_identity { + my ($device, $ids, $sysfs, $allow_member) = @_; + $sysfs //= '/sys'; + + $device = PVE::Diskmanage::verify_blockdev_path($device); + $device = PVE::Diskmanage::get_blockdev($device) if PVE::Diskmanage::is_partition($device); + my $name = basename($device); + my $dir = "$sysfs/class/block/$name"; + my $number = file_read_firstline("$dir/dev"); + die "unable to identify '$device' - missing device number\n" + if !defined($number) || $number !~ m/^\d+:\d+$/; + opendir(my $dh, "$dir/holders") or die "unable to inspect '$device' - $!\n"; + my @holders = grep { $_ ne '.' && $_ ne '..' } readdir($dh); + closedir($dh); + my $wwid; + + for my $holder (@holders) { + next if $holder !~ m/^dm-\d+$/; + my $uuid = file_read_firstline("$sysfs/class/block/$holder/dm/uuid"); + die "unable to identify holder '$holder'\n" if !defined($uuid); + if ($uuid =~ m/^mpath-(\S+)$/) { + die "cannot use '$device' - member path of multipath map '$holder'\n" + if !$allow_member; + $wwid = $1; + } + } + + my $res = { key => "local:$number", targets => {} }; + if ($name =~ m/^dm-\d+$/) { + my $uuid = file_read_firstline("$dir/dm/uuid") + // die "unable to identify '$device' - missing device mapper identity\n"; + ($wwid) = $uuid =~ m/^(?:part\d+-)?mpath-(\S+)$/; + } elsif ($name =~ m/^nvme\d+n\d+$/) { + my $nqn = PVE::Diskmanage::read_subsysnqn("$dir/device/subsysnqn") + // die "unable to identify '$device' - missing subsystem\n"; + my $id = file_read_firstline("$dir/wwid") + // die "unable to identify '$device' - missing namespace identity\n"; + die "unable to identify '$device' - empty namespace identity\n" if !$id; + $res->{key} = "nvme:$id"; + $res->{subsystem} = $nqn; + } elsif (PVE::Diskmanage::is_san_device($device)) { + $ids = $ids->() if ref($ids) eq 'CODE'; + $wwid = PVE::Multipath::Identity::path_wwid($ids, $name) + // die "unable to identify '$device' - unresolved LUN identity\n"; + } + if (defined($wwid)) { + $ids = $ids->() if ref($ids) eq 'CODE'; + $res->{key} = "scsi:$wwid"; + my $observed = 0; + for my $lun (values $ids->{luns}->%*) { + $observed = 1 if ($lun->{wwid} // '') eq $wwid; + next if !defined($lun->{target}); + if (!defined($lun->{wwid})) { + my @paths = sort(($lun->{paths} // [])->@*); + next + if @paths + && !grep { !PVE::Multipath::Identity::path_is_other_lun($ids, $_, $wwid) } + @paths; + die "unable to inspect target '$lun->{target}' - the identity of '" + . join(', ', @paths) + . "' could not be verified; check the path identity and multipathd" + . " configuration\n"; + } + $res->{targets}->{ $lun->{target} } = 1 if $lun->{wwid} eq $wwid; + } + die "unable to inspect '$device' - no identified member paths\n" if !$observed; + } + return $res; +} + +my sub normalized_pv_uuid { + my ($uuid) = @_; + die "unable to inspect physical volume - invalid PV UUID\n" + if !defined($uuid) + || ref($uuid) + || $uuid !~ m/^(?:[a-zA-Z0-9]{32}|[a-zA-Z0-9]{6}(?:-[a-zA-Z0-9]{4}){5}-[a-zA-Z0-9]{6})$/; + $uuid =~ s/-//g; + return $uuid; +} + +# Called under the storage configuration lock before writing a SAN device. Scan attribution is +# intentionally not used: it selects only one owner and can be stale. +sub assert_san_write_safe { + my ($cfg, $device, $partitions) = @_; + $partitions //= PVE::Diskmanage::wipe_partitions($device); + PVE::Diskmanage::assert_block_device_ready($device); + + my $collected; + my $ids = sub { + return $collected if $collected; + my $result = PVE::Multipath::Identity::collect(all => 1); + die "unable to inspect SAN identities - $result->{error}\n" if $result->{error}; + if (-d "$PVE::Storage::ISCSIPlugin::SYSFS/class/iscsi_session") { + my $sessions = PVE::Storage::ISCSIPlugin::iscsi_session_device_map(1); + # san_device_identity() decides about devices without an identity + for my $name (keys %$sessions) { + $result->{luns}->{"$name\0strict"} = { + wwid => PVE::Multipath::Identity::path_wwid($result, $name), + target => $sessions->{$name}->{target}, + paths => [$name], + }; + } + } + return $collected = $result; + }; + my $identity = san_device_identity($device, $ids); + for my $sid (sort keys $cfg->{ids}->%*) { + my $scfg = $cfg->{ids}->{$sid}; + if ( + $scfg->{content}->{images} || $scfg->{type} eq 'iscsidirect' || $scfg->{type} eq 'zfs' + ) { + die "cannot use '$device' - storage '$sid' serves disk images from this target\n" + if $scfg->{target} && $identity->{targets}->{ $scfg->{target} }; + die "cannot use '$device' - storage '$sid' serves disk images from this subsystem\n" + if $scfg->{subsysnqn} && ($identity->{subsystem} // '') eq $scfg->{subsysnqn}; + } + next if $scfg->{type} ne 'lvm' || !$scfg->{base}; + my $base = eval { + my ($base_sid) = parse_volume_id($scfg->{base}); + my (undef, $name) = parse_volname($cfg, $scfg->{base}); + my $type = $cfg->{ids}->{$base_sid}->{type}; + my $canonical = + $type eq 'iscsi' ? PVE::Diskmanage::block_identity_from_id($name) : undef; + $canonical + // san_device_identity(scalar(path($cfg, $scfg->{base})), $ids, undef, 1); + }; + if (!$base) { + my $err = $@ // ''; + chomp($err); + die "cannot use '$device' - cannot check storage '$sid', its base volume" + . " '$scfg->{base}' is not visible on this node\n" + if $err =~ m/not a valid block device|failed to get absolute path/ || !$err; + die "cannot use '$device' - cannot check base volume '$scfg->{base}' of storage '$sid'" + . " - $err\n"; + } + die "cannot use '$device' - storage '$sid' uses this LUN as its base volume\n" + if $base->{key} eq $identity->{key}; + } + + # LVM can select a clone instead of the candidate when both have the same PV UUID. Probe + # every device being destroyed directly rather than relying on that selected device list. + my %candidate_pvs; + for my $candidate ($device, map { "/dev/$_" } $partitions->@*) { + my $output = ''; + run_command( + ['wipefs', '--no-act', '--json', '--output', 'TYPE,UUID', $candidate], + timeout => 10, + outfunc => sub { $output .= "$_[0]\n" }, + ); + my $probe = eval { PVE::Diskmanage::parse_wipefs_signatures($output) }; + if (my $err = $@) { + my $kind = $err eq "Malformed wipefs signature\n" ? 'signature' : 'output'; + die "unable to probe '$candidate' - malformed wipefs $kind\n"; + } + for my $signature ($probe->@*) { + next if $signature->{type} ne 'LVM2_member'; + $candidate_pvs{ normalized_pv_uuid($signature->{uuid}) } = 1; + } + } + + my $output = ''; + run_command( + [ + '/sbin/pvs', + '--nohints', + '--readonly', + '--binary', + '--reportformat', + 'json', + '--options', + 'pv_name,vg_name,pv_uuid,pv_in_use', + ], + timeout => 10, + outfunc => sub { $output .= "$_[0]\n" }, + ); + my $report = eval { PVE::Diskmanage::parse_pv_report($output) }; + if (my $err = $@) { + my $kind = $err =~ m/^(?:Malformed|Invalid) PV in pvs report\n$/ ? 'PV' : 'pvs output'; + die "unable to inspect volume groups - malformed $kind\n"; + } + my %reported; + for my $pv ($report->@*) { + die "unable to inspect volume groups - malformed PV\n" if $pv->{pv_name} !~ m{^/dev/}; + $reported{ normalized_pv_uuid($pv->{pv_uuid}) } = 1; + } + die "cannot use '$device' - LVM does not report its physical volume," + . " so its volume group is unknown\n" + if grep { !$reported{$_} } keys %candidate_pvs; + + for my $pv ($report->@*) { + my $uuid = normalized_pv_uuid($pv->{pv_uuid}); + # The sentinel means missing metadata even if the use flag contradicts it. A matching UUID + # already proves membership; resolving an alias first could hide that proof if it vanished. + my $unknown_vg = $pv->{vg_name} eq '[unknown]' + || ($pv->{vg_name} eq '' && $pv->{pv_in_use} ne '0'); + if ($unknown_vg) { + die "cannot use '$device' - physical volume belongs to a volume group" + . " whose metadata is not visible on this node\n" + if $candidate_pvs{$uuid} + || san_device_identity($pv->{pv_name}, $ids, undef, 1)->{key} eq $identity->{key}; + next; + } + next if $pv->{vg_name} eq ''; + my @storages = grep { + my $scfg = $cfg->{ids}->{$_}; + ($scfg->{type} eq 'lvm' || $scfg->{type} eq 'lvmthin') + && ($scfg->{vgname} // '') eq $pv->{vg_name}; + } sort keys $cfg->{ids}->%*; + next if !@storages; + die "cannot use '$device' - volume group '$pv->{vg_name}' is used by storage '" + . $storages[0] . "'\n" + if $candidate_pvs{$uuid} + || san_device_identity($pv->{pv_name}, $ids, undef, 1)->{key} eq $identity->{key}; + } + return; +} + # storage_can_replicate: # return true if storage supports replication # (volumes allocated with vdisk_alloc() has replication feature) diff --git a/src/PVE/Storage/LVMPlugin.pm b/src/PVE/Storage/LVMPlugin.pm index 5ba56519..0660529a 100644 --- a/src/PVE/Storage/LVMPlugin.pm +++ b/src/PVE/Storage/LVMPlugin.pm @@ -146,6 +146,15 @@ sub clear_first_sector { } } +sub assert_whole_lun { + my ($device) = @_; + + my $uuid = file_read_firstline('/sys/class/block/' . basename($device) . '/dm/uuid') // ''; + die "cannot use '$device' - create the volume group on the whole LUN\n" + if PVE::Diskmanage::is_partition($device) || $uuid =~ m/^part[1-9]\d*-mpath-/; + return; +} + sub lvm_create_volume_group { my ($device, $vgname, $shared, $guard) = @_; $guard = {%$guard} if $guard; @@ -153,12 +162,15 @@ sub lvm_create_volume_group { my $requested_device = $guard ? $guard->{reference} : $device; $device = PVE::Diskmanage::verify_blockdev_path($requested_device); my $san = $guard && $guard->{san}; + my $stable_device; if ($guard) { if ($san) { + assert_whole_lun($device); my $current = $guard->{current} = PVE::Diskmanage::block_device_reference( $requested_device, $guard->{expected}, ); $device = $current->{device}; + PVE::Diskmanage::assert_block_device_ready($device); $guard->{handle} = PVE::Diskmanage::open_block_device($device, $current->{stamp}); $guard->{verify} = sub { PVE::Diskmanage::assert_block_device_instance($requested_device, $current); @@ -174,10 +186,12 @@ sub lvm_create_volume_group { $guard->{verify}->(); } } - my $check_instance = sub { $guard->{verify}->() if $guard }; + my $check_instance = sub { $guard->{verify}->($stable_device) if $guard }; my $check_safe = sub { $check_instance->(); if ($san) { + $guard->{check_storage}->($device); + PVE::Diskmanage::assert_block_device_ready($device); my $users = PVE::Diskmanage::block_device_users( [basename($device)], { 'partition-maps' => 1 }, ); @@ -258,17 +272,21 @@ sub lvm_create_volume_group { delete $signatures{LVM2_member} if $res && !$res->{vgname}; $refuse_data->(keys %signatures) if %signatures; + if ($san) { + $stable_device = PVE::Diskmanage::canonical_blockdev_link($device) + // die "cannot use '$device' - cannot verify a stable name under /dev/disk/by-id\n"; + } $check_safe->(); clear_first_sector($san ? '/proc/' . $$ . '/fd/' . fileno($guard->{handle}) : $device); # we use --metadatasize 250k, which reseults in "pe_start = 512" # so pe_start is aligned on a 128k boundary (advantage for SSDs) - my $cmd = ['/sbin/pvcreate', '--metadatasize', '250k', $device]; + my $cmd = ['/sbin/pvcreate', '--metadatasize', '250k', $stable_device // $device]; $check_safe->(); run_command($cmd, errmsg => "pvcreate '$device' error"); - $cmd = ['/sbin/vgcreate', $vgname, $device]; + $cmd = ['/sbin/vgcreate', $vgname, $stable_device // $device]; # push @$cmd, '-c', 'y' if $shared; # we do not use this yet $check_safe->(); @@ -1107,6 +1125,8 @@ sub on_add_hook { # for the map to assemble before the VG is created on it PVE::Storage::activate_volumes($cfg, [$base]); + assert_whole_lun(PVE::Diskmanage::verify_blockdev_path($path)); + # a failing hook does not remove a volume group it created, so check the device first assert_device_discard_supported($path) if discard_on_remove_requested($scfg->{'on-volume-remove'}); @@ -1120,6 +1140,7 @@ sub on_add_hook { { reference => $path, san => 1, + check_storage => sub { PVE::Storage::assert_san_write_safe($cfg, $_[0]) }, }, ); } diff --git a/src/PVE/Storage/SANScan.pm b/src/PVE/Storage/SANScan.pm index 4015361b..7efdbfd5 100644 --- a/src/PVE/Storage/SANScan.pm +++ b/src/PVE/Storage/SANScan.pm @@ -256,6 +256,13 @@ my sub build_lun_rows($cfg, $inventory) { $entry->{multipath} = 'unmanaged'; } } + my $hint = PVE::Diskmanage::block_device_usage_hint( + $disk->{devpath}, + defined($disk->{slaves}) ? { 'map-wwid' => $disk->{wwn} } : undef, + ); + $entry->@{qw(in-use in-use-by)} = $hint->@{qw(in-use in-use-by)}; + my $link = PVE::Diskmanage::canonical_blockdev_link($disk->{devpath}, $ids, $wwid); + $entry->{'by-id'} = $link if defined($link); push $records->@*, { row => $entry, disk => $disk, identity => lun_compare_key($entry, $disk) }; }; diff --git a/src/test/Makefile b/src/test/Makefile index 3ae2639f..49f6413c 100644 --- a/src/test/Makefile +++ b/src/test/Makefile @@ -1,6 +1,6 @@ all: test -test: test_zfspoolplugin test_lvmplugin test_lvm_create test_lvm_lock_retry test_lvm_base_volume test_lvm_hints test_lvm_base_activation test_lvm_status test_status_command test_disklist test_disk_api test_bwlimit test_plugin test_ovf test_volume_access test_multipath test_iscsi test_lun_identity test_multipath_api test_san_lun_scan test_san_api test_san_libraries test_san_taint +test: test_zfspoolplugin test_lvmplugin test_lvm_create test_lvm_lock_retry test_lvm_base_volume test_lvm_hints test_lvm_base_activation test_lvm_status test_status_command test_disklist test_wipe test_disk_api test_bwlimit test_plugin test_ovf test_volume_access test_multipath test_iscsi test_lun_identity test_multipath_api test_san_lun_scan test_san_api test_san_libraries test_san_taint test_zfspoolplugin: run_test_zfspoolplugin.pl ./run_test_zfspoolplugin.pl @@ -37,11 +37,15 @@ test_disklist: run_disk_tests.pl run_block_device_users_tests.pl ./run_disk_tests.pl ./run_block_device_users_tests.pl -test_disk_api: run_disk_api_tests.pl run_wipedisk_tests.pl run_wipe_reference_tests.pl run_wipe_partition_tests.pl - ./run_disk_api_tests.pl +test_wipe: run_wipedisk_tests.pl run_wipe_partition_tests.pl run_wipe_readiness_tests.pl run_wipe_reference_tests.pl ./run_wipedisk_tests.pl - ./run_wipe_reference_tests.pl ./run_wipe_partition_tests.pl + ./run_wipe_readiness_tests.pl + ./run_wipe_reference_tests.pl + +test_disk_api: run_disk_api_tests.pl run_san_wipe_tests.pl + ./run_disk_api_tests.pl + ./run_san_wipe_tests.pl test_bwlimit: run_bwlimit_tests.pl ./run_bwlimit_tests.pl @@ -82,5 +86,6 @@ test_san_api: run_san_api_tests.pl test_san_libraries: run_san_library_tests.pl ./run_san_library_tests.pl -test_san_taint: run_san_taint_tests.pl +test_san_taint: run_san_lvm_create_tests.pl run_san_taint_tests.pl + perl -T ./run_san_lvm_create_tests.pl perl -T ./run_san_taint_tests.pl diff --git a/src/test/disk_api_test.pm b/src/test/disk_api_test.pm index 2371a953..2d30d2e5 100644 --- a/src/test/disk_api_test.pm +++ b/src/test/disk_api_test.pm @@ -132,6 +132,7 @@ subtest 'a volume group on a map is created under the storage configuration lock $disk->mock(locked_disk_action => sub { $lock->('disk', $_[0]) }); $disk->mock(verify_blockdev_path => sub { $_[0] }); $disk->mock(block_device_reference => sub { { device => $_[0] } }); + $storage->mock(config => sub { { ids => {} } }); $disk->mock(assert_disk_unused => sub { }); $disk->mock(is_partition => sub { 0 }); $disk->mock(udevadm_trigger => sub { }); diff --git a/src/test/run_block_device_users_tests.pl b/src/test/run_block_device_users_tests.pl index 1c040f60..b041201f 100755 --- a/src/test/run_block_device_users_tests.pl +++ b/src/test/run_block_device_users_tests.pl @@ -16,6 +16,7 @@ my $options = { sysfs => "$root/sys", procfs => "$root/proc" }; my $disk = Test::MockModule->new('PVE::Diskmanage'); my %numbers = ('/dev/alias' => '8:1'); $disk->mock( + verify_blockdev_path => sub { $_[0] }, block_device_number => sub { my ($path) = @_; die "stat failed\n" if !exists($numbers{$path}); @@ -53,6 +54,11 @@ device('sda1', '8:1', 'sda'); is_deeply(users(), [], 'unused disk and partition'); write_file('/proc/self/mountinfo', "1 0 8:1 / /mnt rw - ext4 /dev/alias rw\n"); is_deeply(users(), ['sda1 mounted at /mnt'], 'mounts matched by number'); +is_deeply( + PVE::Diskmanage::block_device_usage_hint('/dev/sda', $options), + { 'in-use' => 'yes', 'in-use-by' => 'sda1 mounted at /mnt' }, + 'metadata hint includes kernel partition mounts', +); write_file('/proc/self/mountinfo', "1 0 0:1 / / rw - rootfs rootfs rw\n"); write_file('/proc/swaps', "Filename\tType\tSize\tUsed\tPriority\n/dev/alias partition 1 0 -2\n"); is_deeply(users(), ['sda1 used as swap'], 'swap alias'); @@ -135,6 +141,11 @@ is_deeply( unlink("$root/sys/class/block/sda1/dev") or die $!; eval { users() }; like($@, qr/missing value/, 'missing partition identity fails closed'); +is( + PVE::Diskmanage::block_device_usage_hint('/dev/sda', $options)->{'in-use'}, + 'unknown', + 'missing kernel partition identity gives unknown hint', +); write_file('/sys/class/block/sda1/dev', "8:1\n"); remove_tree("$root/sys/class/block/sda1/holders"); eval { users() }; diff --git a/src/test/run_lun_identity_tests.pl b/src/test/run_lun_identity_tests.pl index a49f27c4..5a5fdced 100755 --- a/src/test/run_lun_identity_tests.pl +++ b/src/test/run_lun_identity_tests.pl @@ -538,6 +538,200 @@ for my $case ([$Q, '3WQ', 'sdg'], [$R, '3WR', 'sdh']) { query_path_identities => sub { $queries++; die $reply if !ref($reply); return $reply }); } +# --- paths without an identity that are provably another LUN --- + +{ + my $W = '3600a09803832355367242f747a716b7a'; + my sub page { + my $body = + join('', map { pack('C C x C', $_->[0], $_->[1], length($_->[2])) . $_->[2] } @_); + return pack('C C n', 0, 0x83, length($body)) . $body; + } + my sub naa { [0x01, 0x03, pack('H*', $_[0])] } + # a local QEMU disk, as lab nodes have it: one vendor specific ASCII designator + my $qemu = pack('H*', '0083000f0200000b64726976652d7363736930'); + disk_device('0:0:0:0', 'sda', '8:0'); + put('bus/scsi/devices/0:0:0:0/type', "0\n"); + # the map's path names its NAA, the other path of the same LUN is not listed by the daemon + put('bus/scsi/devices/7:0:0:0/vpd_pg83', $vpd); + $reply = {}; + my sub other { + my ($name, $wwid) = @_; + $ids = PVE::Multipath::Identity::collect(all => 1); + return PVE::Multipath::Identity::path_is_other_lun($ids, $name, $wwid // $W); + } + my sub sda_page { put('bus/scsi/devices/0:0:0:0/vpd_pg83', $_[0]) } + + sda_page($qemu); + ok(other('sda'), 'other LUN: a local disk with a vendor specific designator only'); + is($ids->{paths}->{sda}->{wwid}, undef, 'other LUN: and without an identity of its own'); + ok(!other('sdc'), 'other LUN: not the unlisted path of the same LUN'); + ok(!other('sda', '3OTHER'), 'other LUN: undecided for a WWID that is not an NAA'); + ok(!other('sda', '2600a09803832355367242f747a716b7a'), 'other LUN: undecided for EUI-64'); + ok(!other('sda', '3600a0980383235536724'), 'other LUN: undecided for a short NAA'); + + sda_page(page(naa('600a09803832355367242f747a716c30'))); + ok(other('sda'), 'other LUN: a disk with the NAA of another LUN'); + sda_page(page(naa('600a09803832355367242f747a716c30'), naa(substr($W, 1)))); + ok(!other('sda'), 'other LUN: not with the NAA of the LUN after another one'); + sda_page(page([0x01, 0x13, pack('H*', '600a098000000002ac16036a00000cbc')])); + ok(!other('sda'), 'other LUN: undecided with target port designators only'); + sda_page(page()); + ok(!other('sda'), 'other LUN: undecided with no designator'); + sda_page(page([0x02, 0x03, '600a09803832355367242f747a716c30'])); + ok(!other('sda'), 'other LUN: undecided with an NAA designator in ASCII'); + sda_page(page(naa('600a0980383235536724'))); + ok(!other('sda'), 'other LUN: undecided with an NAA designator of a wrong length'); + sda_page(page(naa('500a09803832355367242f747a716c30'))); + ok(!other('sda'), 'other LUN: undecided with an NAA type that does not fit its length'); + sda_page(page([0x03, 0x08, 'naa.600A09803832355367242F747A716B7A'])); + ok(!other('sda'), 'other LUN: undecided with a SCSI name string the daemon reads as an NAA'); + sda_page(page([0x03, 0x08, 'iqn.2003-01.org.example:disk'])); + ok(other('sda'), 'other LUN: a SCSI name string that is no NAA'); + sda_page(substr($qemu, 0, 10)); + ok(!other('sda'), 'other LUN: undecided with a truncated page'); + unlink("$sys/bus/scsi/devices/0:0:0:0/vpd_pg83"); + ok(!other('sda'), 'other LUN: undecided without a page'); + + for my $case ( + [[0x00, 0x0f, 'x'], 'a designator of a reserved type'], + [[0x02, 0x00, ''], 'an empty vendor specific designator'], + [[0x03, 0x08, 'bad'], 'a SCSI name string without a known prefix'], + [[0x01, 0x07, pack('H*', '00' x 16)], 'an MD5 logical unit identifier'], + [[0x01, 0x02, pack('H*', '0123')], 'an EUI-64 designator of a wrong length'], + [[0x03, 0x08, "iqn.2003-01.org.example:\xff"], 'a SCSI name string that is not ASCII'], + [[0x03, 0x08, 'eui.x'], 'a SCSI name string with a malformed EUI'], + [[0x02, 0x00, "drive\xff"], 'a vendor specific designator that is not ASCII'], + [[0x02, 0x01, "QEMU \xff"], 'a T10 designator that is not ASCII'], + [[0x02, 0x00, "drive-scsi0\n"], 'a vendor specific designator ending in a newline'], + [[0x02, 0x01, "QEMU X\n"], 'a T10 designator ending in a newline'], + [[0x03, 0x08, "iqn.2003-01.org.example:disk\n"], 'an iSCSI name ending in a newline'], + [[0x03, 0x08, "eui.0123456789abcdef\n"], 'an EUI name ending in a newline'], + ) { + sda_page(page($case->[0])); + ok(!other('sda'), "other LUN: undecided with $case->[1]"); + } + sda_page(page([0x01, 0x02, pack('H*', '0123456789abcdef')])); + ok(other('sda'), 'other LUN: a disk with an EUI-64 designator only'); + sda_page(page([0x03, 0x08, "eui.0123456789ABCDEF\0\0\0\0"])); + ok(other('sda'), 'other LUN: a disk with an EUI name string only'); + # the daemon names it, with an identity that its page does not confirm + $reply = { sda => { devt => '8:0', hctl => '0:0:0:0', init => 'ok', wwid => $W } }; + ok( + !other('sda'), + 'other LUN: undecided if the daemon names the disk with an identity that did not resolve', + ); + $reply = {}; + + sda_page($qemu); + $ids = PVE::Multipath::Identity::collect(all => 1); + put('bus/scsi/devices/0:0:0:0/block/sda/dev', "8:99\n"); + ok( + !PVE::Multipath::Identity::path_is_other_lun($ids, 'sda', $W), + 'other LUN: undecided once the name denotes another device', + ); + put('bus/scsi/devices/0:0:0:0/block/sda/dev', "8:0\n"); + { + $ids = PVE::Multipath::Identity::collect(all => 1); + my $module = Test::MockModule->new('PVE::Multipath::Identity'); + my $read = \&PVE::Multipath::Identity::file_get_contents; + my $change = 'sda'; + $module->mock( + file_get_contents => sub { + my $page = $read->($_[0]); + put("bus/scsi/devices/$1/block/$change/dev", "8:99\n") + if $_[0] =~ m{/$change/device/vpd_pg83$} + && readlink("$sys/class/block/$change") =~ m{/devices/([^/]+)/block/}; + return $page; + }, + ); + ok( + !PVE::Multipath::Identity::path_is_other_lun($ids, 'sda', $W), + 'other LUN: undecided if the device changes while its page is read', + ); + put('bus/scsi/devices/0:0:0:0/block/sda/dev', "8:0\n"); + $change = 'sdb'; + $ids = PVE::Multipath::Identity::collect(all => 1); + ok( + !PVE::Multipath::Identity::path_is_other_lun($ids, 'sda', $W), + 'other LUN: undecided if the path of the LUN changes while its page is read', + ); + put('bus/scsi/devices/7:0:0:0/block/sdb/dev', "8:16\n"); + $module->unmock('file_get_contents'); + $ids = PVE::Multipath::Identity::collect(all => 1); + ok( + PVE::Multipath::Identity::path_is_other_lun($ids, 'sda', $W), + 'other LUN: and decided again with both devices unchanged', + ); + put('bus/scsi/devices/7:0:0:0/block/sdb/dev', "8:99\n"); + ok( + !PVE::Multipath::Identity::path_is_other_lun($ids, 'sda', $W), + 'other LUN: undecided once the path of the LUN changed since an earlier answer', + ); + put('bus/scsi/devices/7:0:0:0/block/sdb/dev', "8:16\n"); + } + { + # a logical volume can hold a path next to its map + put('devices/virtual/block/dm-20/dm/uuid', "LVM-oldvg\n"); + symlink( + "$sys/devices/virtual/block/dm-20", + "$sys/bus/scsi/devices/0:0:0:0/block/sda/holders/dm-20", + ) or die "$!\n"; + hold('0:0:0:0', 'sda', 'dm-21', $W); + sda_page(page([0x01, 0x02, pack('H*', '0123456789abcdef')])); + $ids = PVE::Multipath::Identity::collect(all => 1); + is( + PVE::Multipath::Identity::path_wwid($ids, 'sda'), + $W, + 'holders: the map of a path that a logical volume holds as well', + ); + # and in the other order, as the order of the holder links is the directory's + unlink("$sys/bus/scsi/devices/0:0:0:0/block/sda/holders/$_") for qw(dm-20 dm-21); + hold('0:0:0:0', 'sda', 'dm-21', $W); + symlink( + "$sys/devices/virtual/block/dm-20", + "$sys/bus/scsi/devices/0:0:0:0/block/sda/holders/dm-20", + ) or die "$!\n"; + $ids = PVE::Multipath::Identity::collect(all => 1); + is( + PVE::Multipath::Identity::path_wwid($ids, 'sda'), + $W, + 'holders: the map of a path that a logical volume holds as well, linked the other way', + ); + hold('0:0:0:0', 'sda', 'dm-22', '3600a09803832355367242f747a716c30'); + $ids = PVE::Multipath::Identity::collect(all => 1); + is(PVE::Multipath::Identity::path_wwid($ids, 'sda'), undef, 'holders: none with two maps'); + ok( + $ids->{paths}->{sda}->{claimed} && $ids->{paths}->{sda}->{unresolved}, + 'holders: but claimed and unresolved', + ); + ok( + !PVE::Multipath::Identity::path_is_other_lun($ids, 'sda', $W), + 'other LUN: undecided for a path that maps hold', + ); + unlink("$sys/bus/scsi/devices/0:0:0:0/block/sda/holders/$_") for qw(dm-20 dm-21 dm-22); + } + { + # NAA format 5 has 8 bytes, so a 16-byte one cannot stand for the WWID + my $W5 = '35' . substr($W, 2); + hold('7:0:0:1', 'sdc', 'dm-9', $W5); + put('bus/scsi/devices/7:0:0:1/vpd_pg83', page(naa(substr($W5, 1)))); + ok(!other('sda', $W5), 'other LUN: undecided if the NAA of the LUN is not a valid one'); + unlink("$sys/bus/scsi/devices/7:0:0:1/block/sdc/holders/dm-9") or die "$!\n"; + put('bus/scsi/devices/7:0:0:1/vpd_pg83', $vpd); + } + + # only if a path of the LUN names its NAA in binary, as a name string can give the same WWID + unlink("$sys/bus/scsi/devices/7:0:0:0/vpd_pg83"); + ok(!other('sda'), 'other LUN: undecided if no path of the LUN names its NAA'); + put('bus/scsi/devices/7:0:0:0/vpd_pg83', page([0x03, 0x08, 'naa.' . uc(substr($W, 1))])); + ok(!other('sda'), 'other LUN: undecided if the LUN names its NAA as a SCSI name string only'); + + unlink("$sys/bus/scsi/devices/7:0:0:0/vpd_pg83"); + remove_tree("$sys/bus/scsi/devices/0:0:0:0"); + unlink("$sys/class/block/sda"); +} + # --- the iSCSI plugin --- $ids = { @@ -660,6 +854,26 @@ is_deeply( is_deeply($list->(), ['0.0.3.scsi-3W3'], 'plain volumes: a LUN without aliases'); is($collected, 0, 'plain volumes: needs no identities'); + my $device = { + map { + my $name = $_; + map { $_ => $name } $fixture->{$name}->[2]->@* + } keys %$fixture + }; + my $diskmanage = Test::MockModule->new('PVE::Diskmanage'); + $diskmanage->mock( + verify_blockdev_path => sub { '/dev/' . $device->{ $_[0] =~ s|^/dev/disk/by-id/||r } }, + block_device_stamp => sub { '1:8:0' }, + is_partition => sub { 0 }, + ); + for my $volname ($listed->@[0 .. 3]) { + my ($id) = $volname =~ m/^\d+\.\d+\.\d+\.(.+)$/; + eval { PVE::Diskmanage::block_device_reference("/dev/disk/by-id/$id") }; + is($@, '', "plain volumes: $id passes the volume group creation identity check"); + } + eval { PVE::Diskmanage::block_device_reference('/dev/disk/by-id/scsi-8naa.W0') }; + like($@, qr/does not match its by-id name/, 'plain volumes: while an alias would not'); + $iscsi->mock( iscsi_session_device_map => $session_device_map, load_stable_scsi_paths => sub { {} }, diff --git a/src/test/run_san_lun_scan_tests.pl b/src/test/run_san_lun_scan_tests.pl index 1771a5c8..af8b4870 100755 --- a/src/test/run_san_lun_scan_tests.pl +++ b/src/test/run_san_lun_scan_tests.pl @@ -46,6 +46,9 @@ my sub sd { # what the scan collects in which order my @order; my $diskmanage = Test::MockModule->new('PVE::Diskmanage'); +my $hint = { 'in-use' => 'no', 'in-use-by' => '' }; +my @hint_calls; +my $by_id; $diskmanage->mock( get_disks => sub { push @order, 'disks'; @@ -69,6 +72,8 @@ $diskmanage->mock( }; }, get_nvme_path_status => sub { {} }, + block_device_usage_hint => sub { push @hint_calls, [@_]; $hint }, + canonical_blockdev_link => sub { $by_id }, udev_properties => sub { my ($dev) = @_; my $pv = $dev =~ m/^dm-(.+)$/ ? $maps->{$1}->{pv} : ($devices->{$dev} // {})->{pv}; @@ -196,7 +201,28 @@ my sub flagged { { '3W0' => { slaves => ['sdb', 'sdc'], pv => 'PV0' } }, ); is(scan()->{'3W0'}->{'paths-total'}, 2, 'map path count of the disk scan'); +@hint_calls = (); my $res = scan(); +is($res->{sdd}->{'in-use'}, 'no', 'scan exposes metadata usage hint'); +is_deeply( + { map { $_->[0] => $_->[1] } @hint_calls }, + { + '/dev/mapper/3W0' => { 'map-wwid' => '3W0' }, + '/dev/sdd' => undef, + }, + 'usage hint: a map also for its paths, a path without a map on its own', +); +ok(!exists($res->{sdd}->{'by-id'}), 'scan omits unverified by-id path'); +$hint = { 'in-use' => 'yes', 'in-use-by' => 'array held by vg-data' }; +$by_id = '/dev/disk/by-id/dm-uuid-mpath-3W0'; +my $busy = scan()->{'3W0'}; +is($busy->{'in-use'}, 'yes', 'scan propagates positive usage hint'); +is($busy->{'in-use-by'}, 'array held by vg-data', 'scan propagates user names'); +is($busy->{'by-id'}, $by_id, 'scan emits the verified canonical link'); +$hint = { 'in-use' => 'unknown', 'in-use-by' => 'check failed' }; +is(scan()->{sdd}->{'in-use'}, 'unknown', 'scan preserves failed-probe state'); +$hint = { 'in-use' => 'no', 'in-use-by' => '' }; +$by_id = undef; is($res->{sdd}->{wwid}, '3W0', 'wwid: a path without a map has the identity of its LUN'); ok($derived_with && $derived_with == $collected, 'the storages derive from the same collection'); is_deeply([@order[0, 1]], ['identities', 'disks'], 'the identities are collected before the disks'); diff --git a/src/test/run_san_lvm_create_tests.pl b/src/test/run_san_lvm_create_tests.pl index 8d6bbfdc..c41142da 100755 --- a/src/test/run_san_lvm_create_tests.pl +++ b/src/test/run_san_lvm_create_tests.pl @@ -7,7 +7,7 @@ use File::Basename qw(basename); use File::Path qw(make_path); use File::Temp qw(tempdir); use JSON qw(encode_json); -use Scalar::Util qw(weaken); +use Scalar::Util qw(refaddr tainted weaken); use Storable qw(dclone); use Test::MockModule; use Test::More; @@ -16,20 +16,40 @@ use lib ('.', '..'); use PVE::API2::Disks::LVM; use PVE::API2::Storage::Config; +$ENV{PATH} = '/usr/sbin:/usr/bin:/sbin:/bin'; +delete @ENV{qw(IFS CDPATH ENV BASH_ENV)}; + +my sub command_boundary { + my ($cmd) = @_; + + # Exercise exec's taint check without letting any device command reach a real device. + PVE::Tools::run_command(['/usr/bin/true', @$cmd]) if ${^TAINT}; +} + my $disk = Test::MockModule->new('PVE::Diskmanage'); my $storage = Test::MockModule->new('PVE::Storage'); my $lvm = Test::MockModule->new('PVE::Storage::LVMPlugin'); +my $mp = Test::MockModule->new('PVE::Multipath'); +my $identity = Test::MockModule->new('PVE::Multipath::Identity'); +my $iscsi = Test::MockModule->new('PVE::Storage::ISCSIPlugin'); my $rpc = Test::MockModule->new('PVE::RPCEnvironment'); -my $inotify = Test::MockModule->new('PVE::INotify'); my $tools = Test::MockModule->new('PVE::Tools'); -my $glob = \&PVE::Tools::dir_glob_foreach; +my $inotify = Test::MockModule->new('PVE::INotify'); my $reference = \&PVE::Diskmanage::block_device_reference; my $stamp = \&PVE::Diskmanage::block_device_stamp; +my $ready = \&PVE::Diskmanage::assert_block_device_ready; my $users = \&PVE::Diskmanage::block_device_users; -my ($root, $resolved, $worker, $cfg, $handle, $local, $pv, $during_probe, $inventory_used); -my (@locks, @writes); +my $partitions = \&PVE::Diskmanage::wipe_partitions; +my $san_identity = \&PVE::Storage::san_device_identity; +my $storage_guard = \&PVE::Storage::assert_san_write_safe; +my $canonical_link = \&PVE::Diskmanage::canonical_blockdev_link; +my $glob = \&PVE::Tools::dir_glob_foreach; +my ($root, $resolved, $worker, $cfg, $handle, $local, $partition, $pv, $during_probe, $after_write); +my (@locks, @writes, @probes, $config_writes, $ids, $missing_links, $inventory_used); +my ($config_reads, @guard_configs); my $uuid = 'aaaaaa-bbbb-cccc-dddd-eeee-ffff-gggggg'; -my $canonical = '/dev/mapper/lun'; +my $canonical = '/dev/disk/by-id/dm-uuid-mpath-test'; +my $nvme_id = 'eui.0123456789abcdef'; my sub put { my ($path, $value) = @_; my ($dir) = "$root/$path" =~ m{^(.*)/[^/]+$}; @@ -38,6 +58,13 @@ my sub put { print $fh "$value\n"; close($fh) or die $!; } +my sub device { + my ($name, $number) = @_; + put("sys/class/block/$name/dev", $number); + put("sys/class/block/$name/size", '8192'); + put("sys/class/block/$name/device/state", 'running'); + make_path("$root/sys/class/block/$name/holders"); +} my sub locked { my ($name, $code) = @_; push @locks, $name; @@ -52,6 +79,7 @@ my sub written { ok($handle && defined(fileno($handle)), 'production retains SAN handle through write') if !$local; push @writes, [$tool, $path]; + $after_write->($tool) if $after_write; } { @@ -63,10 +91,22 @@ my sub written { $rpc->mock(get => sub { bless {}, 'SanLVMTestRPC' }); $inotify->mock(nodename => sub { 'n1' }); $disk->mock( - verify_blockdev_path => sub { $_[0] =~ m{^/dev/mapper/} ? $resolved : $_[0] }, + verify_blockdev_path => sub { + die "missing link\n" if $missing_links->{ $_[0] }; + return '/dev/sda' if $_[0] eq '/dev/disk/by-id/scsi-test'; + return $_[0] =~ m{^/dev/(?:disk|mapper)/} ? $resolved : $_[0]; + }, + canonical_blockdev_link => sub { $canonical_link->($_[0], $_[1], $_[2], "$root/sys") }, is_san_device => sub { !$local }, is_multipath_device => sub { !$local && $_[0] =~ /dm-/ }, - is_partition => sub { 0 }, + is_partition => sub { $partition && $_[0] eq $resolved }, + get_blockdev => sub { '/dev/sda' }, + block_device_reference => sub { $reference->($_[0], $_[1], "$root/sys") }, + block_device_stamp => sub { $stamp->($_[0], "$root/sys") }, + assert_block_device_ready => sub { + push @probes, 'readiness'; + $ready->($_[0], { sysfs => "$root/sys", procfs => "$root/proc" }); + }, block_device_users => sub { $users->($_[0], { %{ $_[1] // {} }, sysfs => "$root/sys", procfs => "$root/proc" }); }, @@ -75,9 +115,10 @@ $disk->mock( return PVE::Tools::file_read_firstline( "$root/sys/class/block/" . basename($path) . '/dev'); }, - block_device_reference => sub { $reference->($_[0], $_[1], "$root/sys") }, - block_device_stamp => sub { $stamp->($_[0], "$root/sys") }, + wipe_partitions => sub { $partitions->($_[0], "$root/sys") }, open_block_device => sub { + push @probes, 'open'; + is($_[1], $stamp->($_[0], "$root/sys"), 'open receives current instance stamp'); open(my $opened, '+<', "$root/handle") or die $!; $handle = $opened; weaken($handle); @@ -85,9 +126,50 @@ $disk->mock( }, is_mounted => sub { undef }, has_holder => sub { undef }, - assert_disk_unused => sub { die "device '$_[0]' is already in use\n" if $inventory_used }, + assert_disk_unused => sub { + push @probes, 'inventory'; + die "device '$_[0]' is already in use\n" if $inventory_used; + }, locked_disk_action => sub { locked('disk', $_[0]) }, + get_partnum => sub { 1 }, + get_disks => sub { + push @probes, 'disk'; + return { sda => { gpt => 1 } }; + }, + run_command => sub { + my ($cmd) = @_; + command_boundary($cmd); + die "unexpected command @$cmd\n" if $cmd->[0] ne 'sgdisk'; + written('parttype', $cmd->[-1]); + }, udevadm_trigger => sub { }, + get_nvme_path_status => sub { { nvme900n1 => { active => 1 } } }, +); +$tools->mock( + dir_glob_foreach => sub { + my $path = $_[0]; + $path =~ s{^/sys}{$root/sys}; + $glob->($path, $_[1], $_[2]); + }, +); +$identity->mock( + collect => sub { push @probes, 'identity'; $ids }, + path_wwid => sub { $_[0]->{paths}->{ $_[1] }->{wwid} }, +); +$iscsi->mock(iscsi_session_device_map => sub { {} }); +$mp->mock( + get_maps => sub { + return [ + map { { + sysfs => "dm-$_", + wwid => 'test', + 'path-groups' => [{ + paths => + [{ dev => 'sda', 'dm-state' => 'active', 'check-state' => 'ready' }], + }], + } } (900, 901) + ]; + }, ); $storage->mock( lock_storage_config => sub { @@ -98,18 +180,45 @@ $storage->mock( ); locked('storage', $_[0]); }, - config => - sub { is($locks[-1], 'storage', 'configuration read under storage lock'); dclone($cfg) }, - write_config => sub { }, + config => sub { + is($locks[-1], 'storage', 'configuration read under storage lock'); + $config_reads++; + dclone($cfg); + }, + assert_san_write_safe => sub { + push @guard_configs, refaddr($_[0]); + $storage_guard->(@_); + }, + write_config => sub { $config_writes++ }, path => sub { $canonical }, activate_storage => sub { }, activate_volumes => sub { }, -); -$tools->mock( - dir_glob_foreach => sub { - my $path = $_[0]; - $path =~ s{^/sys}{$root/sys}; - $glob->($path, $_[1], $_[2]); + san_device_identity => sub { $san_identity->($_[0], $_[1], "$root/sys", $_[3]) }, + run_command => sub { + my ($cmd, %opt) = @_; + command_boundary($cmd); + push @probes, $cmd->[0]; + if ($cmd->[0] eq 'wipefs') { + $opt{outfunc}->(encode_json({ + signatures => $pv eq 'blank' + ? [] + : [{ type => 'LVM2_member', uuid => $uuid }], + })); + } elsif ($cmd->[0] eq '/sbin/pvs') { + my $rows = + $pv eq 'blank' + ? [] + : [{ + pv_name => $resolved, + pv_uuid => $uuid, + vg_name => $pv eq 'same' ? 'vg' : '', + pv_in_use => $pv eq 'same' ? '1' : '0', + }]; + $opt{outfunc}->(encode_json({ report => [{ pv => $rows }] })); + $during_probe->('storage') if $during_probe; + } else { + die "unexpected storage command @$cmd\n"; + } }, ); $lvm->mock( @@ -120,18 +229,23 @@ $lvm->mock( }, abs_path => sub { $_[0] }, clear_first_sector => sub { written('clear', $_[0]) }, - lvm_vgs => sub { $pv eq 'same' ? { vg => {} } : {} }, + lvm_vgs => sub { push @probes, 'vgs'; $pv eq 'same' ? { vg => {} } : {} }, assert_discard_supported => sub { }, + assert_device_discard_supported => sub { push @probes, 'discard' }, run_command => sub { my ($cmd, %opt) = @_; + command_boundary($cmd); if ($cmd->[0] eq 'wipefs') { + push @probes, 'wipefs'; $opt{outfunc} ->(encode_json({ signatures => $pv eq 'blank' ? [] : [{ type => 'LVM2_member' }] }) ); } elsif ($cmd->[0] eq '/usr/bin/file') { + push @probes, 'file'; $opt{outfunc}->("$cmd->[-1]: " . ($pv eq 'blank' ? 'data' : 'LVM2 PV')); - $during_probe->() if $during_probe; + $during_probe->('lvm') if $during_probe; } elsif ($cmd->[0] eq '/sbin/pvs') { + push @probes, 'pvs'; $opt{outfunc}->("$resolved:8192:" . ($pv eq 'same' ? 'vg' : '') . ":$uuid:" @@ -144,29 +258,45 @@ $lvm->mock( ); my sub reset_case { $root = tempdir(CLEANUP => 1); - for my $name ('dm-900', 'vdz') { - put("sys/class/block/$name/dev", $name eq 'vdz' ? '252:0' : '253:900'); - put("sys/class/block/$name/size", '8192'); - make_path("$root/sys/class/block/$name/holders"); + for my $n (900, 901, 902) { + device("dm-$n", "253:$n"); + put("sys/class/block/dm-$n/dm/uuid", $n == 902 ? 'mpath-other' : 'mpath-test'); + make_path("$root/sys/class/block/dm-$n/slaves"); + symlink('../../sda', "$root/sys/class/block/dm-$n/slaves/sda") or die $!; } - put('sys/class/block/dm-900/dm/uuid', 'mpath-test'); - put('sys/class/block/dm-900/slaves/sda/device/state', 'running'); + device('sda', '8:0'); + device('sdb', '8:16'); + device('vdz', '252:0'); + device('nvme900n1', '259:900'); + put('sys/class/block/nvme900n1/wwid', $nvme_id); + put('sys/class/block/nvme900n1/device/subsysnqn', 'nqn.test'); put('proc/self/mountinfo', '1 0 0:1 / / rw - rootfs rootfs rw'); put('proc/swaps', "Filename\tType\tSize\tUsed\tPriority"); put('handle', 'retained descriptor fixture'); - ($resolved, $local, $pv) = ('/dev/dm-900', 0, 'blank'); - ($during_probe, $worker, $handle) = (undef, undef, undef); + ($resolved, $local, $partition, $pv) = ('/dev/dm-900', 0, 0, 'blank'); + ($during_probe, $after_write, $worker, $handle) = (undef, undef, undef, undef); $cfg = { ids => { base => { type => 'iscsi', target => 'iqn.test', content => { none => 1 } } } }; + $ids = { + paths => { sda => { wwid => 'test' }, sdb => { wwid => 'test' } }, + luns => { lun => { wwid => 'test', target => 'iqn.test' } }, + }; @writes = (); + @probes = (); @locks = (); + $config_writes = 0; + $config_reads = 0; + @guard_configs = (); + $missing_links = {}; $inventory_used = 0; + $canonical = '/dev/disk/by-id/dm-uuid-mpath-test'; } my sub queue { - PVE::API2::Disks::LVM->create({ node => 'n1', name => 'vg', device => $_[0] // $canonical }); + my ($path) = @_; + PVE::API2::Disks::LVM->create({ node => 'n1', name => 'vg', device => $path // $canonical }); } my sub run_route { - my ($route) = @_; + my ($route, $options) = @_; eval { if ($route eq 'disk') { queue(); @@ -176,27 +306,116 @@ my sub run_route { type => 'lvm', storage => 'new', vgname => 'vg', - base => 'base:0.0.1.lun', + %{ $options // {} }, + base => 'base:0.0.1.dm-uuid-mpath-test', }); } }; return $@; } -for my $route ('disk', 'base') { - reset_case(); +my sub raw_user { + my ($dev) = @_; make_path("$root/proc/999999/fd"); - symlink('/dev/dm-900', "$root/proc/999999/fd/3") or die $!; + symlink("/dev/$dev", "$root/proc/999999/fd/3") or die $!; put('proc/999999/comm', 'qemu'); - like( - run_route($route), - qr/cannot use.*qemu .* has dm-900 open/, - "$route raw map opener refused", - ); - is_deeply(\@writes, [], 'raw user receives no destructive command'); +} + +if (${^TAINT}) { + reset_case(); + for my $attr ('dm-900/dm/uuid', 'nvme900n1/wwid') { + my $value = PVE::Tools::file_read_firstline("$root/sys/class/block/$attr"); + ok(tainted($value), "$attr fixture retains taint from its file read"); + eval { command_boundary(['/sbin/pvcreate', $value]) }; + like($@, qr/Insecure dependency/, 'benign exec boundary rejects an uncaptured value'); + } +} + +for my $route ('disk', 'base') { + for my $transport ($route eq 'disk' ? ('iscsi', 'nvmeof') : ('iscsi')) { + for my $restriction ({}, { disable => 1 }, { nodes => { elsewhere => 1 } }) { + reset_case(); + if ($transport eq 'nvmeof') { + $resolved = '/dev/nvme900n1'; + $canonical = "/dev/disk/by-id/nvme-$nvme_id"; + $cfg->{ids}->{base} = + { type => 'nvmeof', subsysnqn => 'nqn.test', content => { none => 1 } }; + } + $cfg->{ids}->{images} = + { %{ $cfg->{ids}->{base} }, content => { images => 1 }, %$restriction }; + my $kind = $transport eq 'nvmeof' ? 'subsystem' : 'target'; + my $prefix = $route eq 'disk' ? "cannot use '$resolved' - " : ''; + is( + run_route($route), + "${prefix}storage 'images' serves disk images from this $kind\n", + "$route $transport image owner refused", + ); + is_deeply(\@writes, [], 'signatureless image volume receives no destructive command'); + is($config_writes, 0, 'refusal does not publish storage'); + $canonical = '/dev/disk/by-id/dm-uuid-mpath-test'; + } + } + for my $dev ('dm-900', 'sda', 'sdb') { + reset_case(); + raw_user($dev); + like( + run_route($route), + qr/cannot use.*qemu .* has $dev open/, + "$route raw map, member or same-identity sibling opener refused", + ); + is_deeply(\@writes, [], 'raw user receives no destructive command'); + } reset_case(); unlink("$root/proc/swaps") or die $!; like(run_route($route), qr{can't open '.*/proc/swaps'}, "$route inconclusive users refuse"); is_deeply(\@writes, [], 'inconclusive users receive no destructive command'); + for my $dev ('sda', 'sdb') { + for my $probe_number (1 .. 3) { + reset_case(); + my $probes = 0; + $during_probe = sub { + raw_user($dev) if $_[0] eq 'storage' && ++$probes == $probe_number; + }; + like( + run_route($route), + qr/cannot use.*qemu .* has $dev open/, + "$route opener during storage probe $probe_number refuses the next write", + ); + is(scalar(@writes), $probe_number - 1, 'only writes preceding the opener occurred'); + is_deeply(\@writes, [], 'opener during first ownership probe causes zero writes') + if $probe_number == 1; + } + } + for my $probe ('lvm', 'storage') { + reset_case(); + $during_probe = sub { $resolved = '/dev/dm-902' if $_[0] eq $probe }; + like( + run_route($route), + qr/identity does not match/, + "$route replacement after $probe probe refused", + ); + is_deeply(\@writes, [], 'replacement after probing receives no destructive command'); + } + if ($route eq 'disk') { + reset_case(); + $resolved = '/dev/nvme900n1'; + $canonical = "/dev/disk/by-id/nvme-$nvme_id"; + $cfg->{ids}->{base} = + { type => 'nvmeof', subsysnqn => 'nqn.test', content => { none => 1 } }; + raw_user('nvme900n1'); + like( + run_route($route), + qr/cannot use.*qemu .* has nvme900n1 open/, + "$route raw namespace refuses", + ); + is_deeply(\@writes, [], 'raw namespace receives no destructive command'); + unlink("$root/proc/999999/fd/3") or die $!; + is(run_route($route), '', "$route closed namespace permits initialization"); + is(scalar(@writes), 3, 'closed namespace gets all initialization writes'); + is($writes[1]->[1], $canonical, 'PV operand encodes namespace identity'); + is($writes[2]->[1], $canonical, 'VG operand encodes namespace identity'); + $canonical = '/dev/disk/by-id/dm-uuid-mpath-test'; + + } for my $kind ('blank', 'orphan', 'same') { reset_case(); @@ -224,8 +443,67 @@ for my $route ('disk', 'base') { 'only the base-route same-VG retry succeeds without writes', ); } + if ($kind ne 'same') { + is( + $config_reads, + $route eq 'disk' ? 1 : 2, + 'guard uses one snapshot; base API retains its separate pre-hook config read', + ); + is(scalar(@guard_configs), 3, 'ownership is checked before every write'); + is_deeply( + \@guard_configs, + [($guard_configs[0]) x 3], + 'every guard check receives the same locked configuration snapshot', + ); + } + ok(!defined($handle), 'production releases the retained handle after completion'); + like($writes[0]->[1], qr{^/proc/\d+/fd/\d+$}, 'sector clear uses retained descriptor') + if @writes; } } + +reset_case(); +queue(); +$resolved = '/dev/dm-901'; +eval { $worker->() }; +is($@, '', 'identity-preserving queued renumbering succeeds'); +is($writes[1]->[1], $canonical, 'LVM uses the verified identity-encoding name after renumbering'); +is($writes[2]->[1], $canonical, 'VG creation also uses the verified stable name'); +for my $path ($canonical, '/dev/mapper/friendly', '/dev/dm-900') { + reset_case(); + queue($path); + if ($path eq '/dev/dm-900') { + put('sys/class/block/dm-900/dm/uuid', 'mpath-other'); + } else { + $resolved = '/dev/dm-902'; + } + eval { $worker->() }; + like( + $@, + qr/cannot use.*identity (?:changed|does not match)/, + 'moved by-id, rebound alias or recycled dm number refuses', + ); + is_deeply(\@writes, [], 'queued identity replacement receives no destructive command'); +} +for my $tool ('clear', '/sbin/pvcreate') { + reset_case(); + $after_write = sub { $resolved = '/dev/dm-902' if $_[0] eq $tool }; + like( + run_route('disk'), + qr/identity does not match/, + "replacement after $tool prevents next write", + ); + is( + scalar(@writes), + $tool eq 'clear' ? 1 : 2, + 'only preceding writes to the checked instance occurred', + ); +} +reset_case(); +$during_probe = sub { put('sys/class/block/dm-900/dev', '253:999') }; +like(run_route('disk'), qr/device topology changed/, 'instance change after probing refuses'); +is_deeply(\@writes, [], 'changed instance receives no destructive command'); + reset_case(); $local = 1; $resolved = '/dev/vdz'; @@ -241,20 +519,93 @@ eval { $worker->() }; like($@, qr/cannot use.*device topology changed/, 'queued local-to-SAN transition refuses'); is_deeply(\@writes, [], 'transition does not bypass storage lock'); +reset_case(); +$local = 1; +$resolved = '/dev/sda1'; +$partition = 1; +device('sda1', '8:1'); +put('sys/class/block/sda1/partition', '1'); +queue($resolved); +eval { $worker->() }; +is($@, '', 'local partition initialization still works without quorum'); +is_deeply( + \@writes, + [ + ['parttype', '/dev/sda'], + ['clear', $resolved], + ['/sbin/pvcreate', $resolved], + ['/sbin/vgcreate', $resolved], + ], + 'local partitions retain unguarded type changes and normal LVM operands', +); + for my $route ('disk', 'base') { reset_case(); - $during_probe = sub { put('sys/class/block/dm-900/dev', '253:999') }; - like(run_route($route), qr/device identity changed/, "$route changed instance refuses"); - is_deeply(\@writes, [], 'replacement after probing receives no destructive command'); + $canonical = '/dev/mapper/friendly'; + is(run_route($route), '', "$route converts friendly alias to canonical operand"); + is($writes[1]->[1], '/dev/disk/by-id/dm-uuid-mpath-test', + 'PV operand encodes the map identity'); + is($writes[2]->[1], '/dev/disk/by-id/dm-uuid-mpath-test', + 'VG operand encodes the map identity'); reset_case(); - is(run_route($route), '', "$route initializes unchanged instance"); - like($writes[0]->[1], qr{^/proc/\d+/fd/\d+$}, 'sector clear uses retained descriptor'); - ok(!defined($handle), 'production releases the retained handle after completion'); + $canonical = '/dev/mapper/friendly'; + $missing_links->{'/dev/disk/by-id/dm-uuid-mpath-test'} = 1; + is( + run_route($route), + "cannot use '/dev/dm-900' - cannot verify a stable name under /dev/disk/by-id\n", + "$route refuses missing canonical link", + ); + is_deeply(\@writes, [], 'missing stable operand refuses before sector clearing'); } + +for my $route ('disk', 'base') { + for my $kind ($route eq 'disk' ? ('scsi', 'nvme', 'map') : ('scsi', 'map')) { + reset_case(); + if ($kind eq 'map') { + device('dm-903', '253:903'); + put('sys/class/block/dm-903/dm/uuid', 'part2-mpath-test'); + $resolved = '/dev/dm-903'; + $canonical = '/dev/mapper/friendly'; + } else { + my $name = $kind eq 'scsi' ? 'sda1' : 'nvme900n1p1'; + device($name, $kind eq 'scsi' ? '8:1' : '259:901'); + put("sys/class/block/$name/partition", '1'); + $resolved = "/dev/$name"; + $partition = 1; + $canonical = + $kind eq 'scsi' + ? '/dev/disk/by-id/scsi-test-part1' + : "/dev/disk/by-id/nvme-$nvme_id-part1"; + $cfg->{ids}->{base} = + { type => 'nvmeof', subsysnqn => 'nqn.test', content => { none => 1 } } + if $kind eq 'nvme'; + } + is( + run_route($route, { 'on-volume-remove' => 'discard=1' }), + "cannot use '$resolved' - create the volume group on the whole LUN\n", + "$route refuses a $kind SAN partition", + ); + is_deeply( + \@probes, + [], + 'partition refused before identity, readiness, inventory or content probes', + ); + is_deeply(\@writes, [], 'partition refusal issues no destructive command'); + is($config_writes, 0, 'partition refusal writes no storage configuration'); + } +} + reset_case(); queue(); -put('sys/class/block/dm-900/dev', '253:999'); +@probes = (); +put('sys/class/block/dm-900/dm/uuid', 'part2-mpath-test'); eval { $worker->() }; -like($@, qr/device identity changed/, 'queued replacement refuses'); -is_deeply(\@writes, [], 'queued replacement receives no destructive command'); +is( + $@, + "cannot use '/dev/dm-900' - create the volume group on the whole LUN\n", + 'queued whole-LUN reference replaced by a partition refuses', +); +is_deeply(\@probes, [], 'queued partition refused before probes'); +is_deeply(\@writes, [], 'queued partition receives no destructive command'); + done_testing(); diff --git a/src/test/run_san_wipe_tests.pl b/src/test/run_san_wipe_tests.pl new file mode 100755 index 00000000..0779bc85 --- /dev/null +++ b/src/test/run_san_wipe_tests.pl @@ -0,0 +1,673 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use File::Path qw(make_path); +use File::Temp qw(tempdir); +use FindBin qw($Bin); +use JSON; +use Test::MockModule; +use Test::More; + +use lib ('.', '..'); +use PVE::Storage; + +my $storage = Test::MockModule->new('PVE::Storage'); +my $disk = Test::MockModule->new('PVE::Diskmanage'); +my ($ready, $readiness_error); +$disk->mock( + assert_block_device_ready => sub { + die "no usable path\n" if $readiness_error; + $ready = 1; + }, +); +my $identity = Test::MockModule->new('PVE::Multipath::Identity'); +my $iscsi = Test::MockModule->new('PVE::Storage::ISCSIPlugin'); +$iscsi->mock(iscsi_session_device_map => sub { {} }); +$identity->mock(collect => sub { { luns => {} } }); +my ($report, $failure, $cfg, $key, $subsystem, $path, $signatures, $partitions, $pv_identities); +my $pv_uuid = 'aaaaaa-bbbb-cccc-dddd-eeee-ffff-gggggg'; +my (@probed, @resolved, @writes); +$storage->mock( + san_device_identity => sub { + push @resolved, $_[0]; + # a base path taken in list context would shift the arguments + die "san_device_identity called with " . scalar(@_) . " arguments\n" if @_ > 4; + # as verify_blockdev_path() does for a link that is not there + die "failed to get absolute path to $_[0]\n" if $_[0] eq '/dev/missing'; + return { + key => $pv_identities->{ $_[0] } // ($_[0] eq '/dev/other' ? 'scsi:other' : $key), + targets => { 'iqn.test' => 1 }, + subsystem => $subsystem, + }; + }, + # as the real one: path, owner and type in list context + path => sub { wantarray ? ($path, undef, 'images') : $path }, + run_command => sub { + my ($cmd, %opt) = @_; + ok($ready, 'readiness precedes every device-reading probe'); + is($opt{timeout}, 10, 'PV probe bounded'); + if ($cmd->[0] eq 'wipefs') { + push @probed, $cmd->[-1]; + die "wipefs failed\n" if $failure eq 'wipefs'; + $opt{outfunc}->($signatures->{ $cmd->[-1] } // '{"signatures":[]}'); + } elsif ($cmd->[0] eq '/sbin/pvs') { + is_deeply( + $cmd, + [ + '/sbin/pvs', + '--nohints', + '--readonly', + '--binary', + '--reportformat', + 'json', + '--options', + 'pv_name,vg_name,pv_uuid,pv_in_use', + ], + 'full hint-free read-only PV report includes independent use flag', + ); + die "pvs failed\n" if $failure; + $opt{outfunc}->($report); + } else { + push @writes, $cmd; + die "unexpected command @$cmd\n"; + } + }, +); +my sub reset_case { + $report = encode_json({ report => [{ pv => [] }] }); + $cfg = { ids => {} }; + $failure = 0; + $key = 'scsi:test'; + $subsystem = undef; + $path = '/dev/alias'; + $signatures = {}; + $partitions = []; + $pv_identities = {}; + @probed = (); + @resolved = (); + @writes = (); + ($ready, $readiness_error) = (0, 0); +} +my sub check { + eval { PVE::Storage::assert_san_write_safe($cfg, '/dev/dm-0', $partitions) }; + return $@; +} +reset_case(); +is(check(), '', 'unreferenced LUN allowed'); +for my $type ('iscsi', 'iscsidirect', 'zfs', 'vendor') { + reset_case(); + $cfg->{ids} = { + base => { type => 'iscsi', target => 'iqn.test', content => { none => 1 } }, + images => { + type => $type, + target => 'iqn.test', + content => { images => 1 }, + disable => 1, + nodes => { elsewhere => 1 }, + }, + }; + like(check(), qr/storage 'images' serves disk images/, "$type conflicting reference found"); +} +for my $base ('0.0.1.scsi-alias', '0.0.1.wwn-0x123-part1') { + reset_case(); + $cfg->{ids}->{base} = { type => 'iscsi' }; + $cfg->{ids}->{lvm} = { type => 'lvm', base => "base:$base", disable => 1 }; + like(check(), qr/uses this LUN as its base volume/, "$base compared by resolved identity"); + $path = '/dev/other'; + is(check(), '', 'different LUN identity allowed'); + $path = '/dev/missing'; + like( + check(), + qr/cannot check storage 'lvm', its base volume 'base:\Q$base\E' is not visible/, + 'unresolvable alias refuses with storage context', + ); +} +for my $case ( + ['iscsi', '0.0.1.dm-uuid-mpath-test', 'scsi:test'], + ['iscsi', '0.0.1.dm-uuid-part2-mpath-test', 'scsi:test'], +) { + reset_case(); + my ($type, $base, $identity) = @$case; + $cfg->{ids}->{base} = { type => $type }; + $cfg->{ids}->{lvm} = { type => 'lvm', base => "base:$base", disable => 1 }; + $path = '/dev/missing'; + $key = $identity; + like(check(), qr/uses this LUN as its base volume/, "$base matched without resolution"); + $key = 'scsi:different'; + is(check(), '', "$base: genuinely different invisible LUN allowed"); +} +reset_case(); +$cfg->{ids}->{base} = { type => 'iscsi' }; +$cfg->{ids}->{lvm} = { type => 'lvm', base => 'base:dm-uuid-mpath-test' }; +like( + check(), + qr/cannot check base volume 'base:dm-uuid-mpath-test' of storage 'lvm'/, + 'plugin parser must accept the volume name', +); +for my $alias ( + 'prefix-dm-uuid-mpath-test', 'nvme-eui.0123456789abcdef-extra', 'nvme-model_serial', +) { + ok(!PVE::Diskmanage::block_identity_from_id($alias), "$alias is not a canonical identity"); +} +reset_case(); +$cfg->{ids}->{lvm} = { type => 'lvm', vgname => 'vg', nodes => { elsewhere => 1 } }; +$report = encode_json({ + report => [{ + pv => + [{ pv_name => '/dev/alias', vg_name => 'vg', pv_uuid => $pv_uuid, pv_in_use => '1' }], + }], +}); +like(check(), qr/volume group 'vg' is used by storage 'lvm'/, 'VG referenced elsewhere refuses'); +$cfg = { ids => {} }; +is(check(), '', 'unreferenced old VG allowed'); +$failure = 1; +like(check(), qr/pvs failed/, 'failed PV probe refuses'); +$failure = 0; +$report = '{}'; +like(check(), qr/malformed pvs/, 'malformed PV report refuses'); + +reset_case(); +$cfg->{ids}->{lvm} = { type => 'lvmthin', vgname => 'vg', disable => 1 }; +$report = encode_json({ + report => [{ + pv => + [{ pv_name => '/dev/other', vg_name => 'vg', pv_uuid => $pv_uuid, pv_in_use => '1' }], + }], +}); +is(check(), '', 'different LUN without a matching PV UUID allowed'); + +for my $candidate ('/dev/dm-0', '/dev/dm-1', '/dev/sda1') { + $partitions = ['dm-1', 'sda1']; + $signatures = { + $candidate => + encode_json({ signatures => [{ type => 'LVM2_member', uuid => $pv_uuid =~ s/-//gr }] }), + }; + like( + check(), + qr/volume group 'vg' is used by storage 'lvm'/, + "$candidate duplicate UUID refuses even when LVM lists another LUN", + ); +} +is_deeply( + [@probed[-3 .. -1]], + ['/dev/dm-0', '/dev/dm-1', '/dev/sda1'], + 'parent and every destroyed partition directly probed', +); +for my $bad ( + '{}', + '{"signatures":[{}]}', + '{"signatures":[{"type":"LVM2_member"}]}', + '{"signatures":[{"type":"LVM2_member","uuid":"bad"}]}', +) { + $signatures = { '/dev/dm-0' => $bad }; + like(check(), qr/malformed wipefs|invalid PV UUID/, 'inconclusive direct probe refuses'); +} +$signatures = {}; +$failure = 'wipefs'; +like(check(), qr/wipefs failed/, 'direct probe failure refuses'); +$failure = 0; +$report = encode_json({ + report => [{ pv => [{ pv_name => '/dev/other', vg_name => 'vg', pv_in_use => '1' }] }] }); +like(check(), qr/invalid PV UUID/, 'configured VG inventory requires PV UUIDs'); + +reset_case(); +$signatures = { + '/dev/dm-0' => encode_json({ signatures => [{ type => 'LVM2_member', uuid => $pv_uuid }] }), +}; +like(check(), qr/LVM does not report its physical volume/, 'missing candidate UUID refuses'); +my $candidate = { pv_name => '/dev/alias', vg_name => '', pv_uuid => $pv_uuid, pv_in_use => '0' }; +$report = encode_json({ report => [{ pv => [$candidate] }] }); +is(check(), '', 'positively unused orphan allowed'); + +for my $used ('1', '', 'unknown') { + $candidate->{pv_in_use} = $used; + $report = encode_json({ report => [{ pv => [$candidate] }] }); + like( + check(), + qr/belongs to a volume group whose metadata is not visible/, + "empty VG with use flag '$used' refuses missing or damaged metadata", + ); +} +$candidate->{vg_name} = 'foreign'; +$candidate->{pv_in_use} = '1'; +$cfg->{ids}->{foreign} = { type => 'lvm', vgname => 'foreign', nodes => { elsewhere => 1 } }; +$report = encode_json({ report => [{ pv => [$candidate] }] }); +like(check(), qr/volume group 'foreign' is used by storage/, 'unhinted PV reference refuses'); + +subtest 'malformed multi-record reports are refused' => sub { + reset_case(); + $signatures->{'/dev/dm-0'} = encode_json({ + signatures => [{ type => 'LVM2_member', uuid => 'bad' }, {}], + }); + ok(check(), 'invalid UUID and malformed signature cannot pass'); + is_deeply(\@writes, [], 'no writes after malformed signatures'); + + reset_case(); + $report = encode_json({ + report => [{ + pv => [ + { pv_name => '/dev/dm-0', vg_name => '', pv_uuid => 'bad', pv_in_use => '0' }, + {}, + ], + }], + }); + ok(check(), 'invalid UUID and malformed PV cannot pass'); + is_deeply(\@writes, [], 'no writes after malformed PV report'); +}; + +subtest 'captured no-metadata PV and unrelated orphan' => sub { + my $capture = PVE::Tools::file_get_contents("$Bin/lvm-fixtures/lab1-nomda-orphan.txt"); + my ($json) = $capture =~ m/\A== devices B,C\n(.*?)^== full nohints /ms; + die "missing captured JSON report\n" if !defined($json); + my $captured = decode_json($json); + is(scalar($captured->{log}->@*), 2, 'both captured log warnings remain in the report'); + is($_->{log_type}, 'warn', 'captured log entry is a warning') for $captured->{log}->@*; + for my $pv ($captured->{report}->[0]->{pv}->@*) { + reset_case(); + $report = $json; + $pv_identities = + { map { $_->{pv_name} => "scsi:$_->{pv_uuid}" } $captured->{report}->[0]->{pv}->@* }; + $key = $pv_identities->{ $pv->{pv_name} }; + $signatures->{'/dev/dm-0'} = encode_json({ + signatures => [{ type => 'LVM2_member', uuid => $pv->{pv_uuid} }], + }); + if ($pv->{vg_name} eq '[unknown]') { + like( + check(), + qr/physical volume belongs to a volume group whose metadata is not visible/, + 'captured no-metadata PV refused specifically, not as a malformed report', + ); + is_deeply( + \@resolved, + ['/dev/dm-0'], + 'captured candidate UUID precedes alias resolution', + ); + } else { + is(check(), '', 'captured orphan passes despite the unrelated no-metadata PV'); + is( + scalar(@resolved), + 2, + 'unrelated no-metadata PV identity is checked before skipping', + ); + } + is_deeply(\@writes, [], 'captured report guard performs no writes'); + } +}; + +subtest 'unknown VG membership is isolated without orphan assumptions' => sub { + my $other_uuid = 'hhhhhh-iiii-jjjj-kkkk-llll-mmmm-nnnnnn'; + for my $vg ('', '[unknown]') { + for my $flag ('1', '', 'unknown', $vg ? ('0') : ()) { + for my $alias ('/dev/alias', '/dev/other', '/dev/missing') { + reset_case(); + $signatures->{'/dev/dm-0'} = encode_json({ + signatures => [{ type => 'LVM2_member', uuid => $pv_uuid }], + }); + $report = encode_json({ + report => [{ + pv => [{ + pv_name => $alias, + vg_name => $vg, + pv_uuid => $pv_uuid, + pv_in_use => $flag, + }], + }], + }); + like( + check(), + qr/physical volume belongs to a volume group whose metadata is not visible/, + "VG '$vg', flag '$flag', alias '$alias': UUID proves candidate membership", + ); + is_deeply(\@resolved, ['/dev/dm-0'], 'matching UUID precedes alias resolution'); + is_deeply(\@writes, [], 'refused candidate causes no writes'); + } + } + } + for my $alias ('/dev/alias', '/dev/sda1', '/dev/other', '/dev/missing') { + reset_case(); + $signatures->{'/dev/dm-0'} = encode_json({ + signatures => [{ type => 'LVM2_member', uuid => $pv_uuid }], + }); + $report = encode_json({ + report => [{ + pv => [ + { + pv_name => '/dev/dm-0', + vg_name => '', + pv_uuid => $pv_uuid, + pv_in_use => '0', + }, + { + pv_name => $alias, + vg_name => '[unknown]', + pv_uuid => $other_uuid, + pv_in_use => '1', + }, + ], + }], + }); + my $error = check(); + if ($alias eq '/dev/other') { + is($error, '', 'unrelated unknown-membership PV does not block the orphan'); + } elsif ($alias eq '/dev/missing') { + like( + $error, + qr/failed to get absolute path/, + 'unresolved different UUID fails closed', + ); + } else { + like( + $error, + qr/metadata is not visible/, + 'different UUID can share whole-LUN identity', + ); + } + is_deeply( + \@resolved, + ['/dev/dm-0', $alias], + 'different UUID still requires identity proof', + ); + is_deeply(\@writes, [], 'guard performs no writes'); + } + + for my $flag (undef, [], {}, 'maybe') { + reset_case(); + my $pv = { + pv_name => '/dev/other', + vg_name => '[unknown]', + pv_uuid => $pv_uuid, + }; + $pv->{pv_in_use} = $flag if defined($flag); + $report = encode_json({ report => [{ pv => [$pv] }] }); + like( + check(), + qr/malformed PV/, + 'sentinel does not excuse a malformed or missing use flag', + ); + is_deeply(\@writes, [], 'malformed use flag causes no writes'); + } + for my $vg ('[missing]', '[UNKNOWN]', 'bad/name') { + reset_case(); + $report = encode_json({ + report => [{ + pv => [{ + pv_name => '/dev/other', + vg_name => $vg, + pv_uuid => $pv_uuid, + pv_in_use => '0', + }], + }], + }); + like(check(), qr/malformed PV/, "unrecognized VG '$vg' still fails closed"); + } + reset_case(); + $report = encode_json({ + report => [{ + pv => [{ + pv_name => '[unknown]', + vg_name => '[unknown]', + pv_uuid => $pv_uuid, + pv_in_use => '1', + }], + }], + }); + like(check(), qr/malformed PV/, 'missing-device spelling is not a supported VG sentinel'); + + reset_case(); + $cfg->{ids}->{lvm} = { type => 'lvm', vgname => 'unknown' }; + $signatures->{'/dev/dm-0'} = encode_json({ + signatures => [{ type => 'LVM2_member', uuid => $pv_uuid }], + }); + $report = encode_json({ + report => [{ + pv => [{ + pv_name => '/dev/missing', + vg_name => 'unknown', + pv_uuid => $pv_uuid, + pv_in_use => '1', + }], + }], + }); + like( + check(), + qr/volume group 'unknown' is used by storage 'lvm'/, + 'unknown is a valid VG name', + ); + is_deeply(\@resolved, ['/dev/dm-0'], 'named VG UUID proof also precedes resolution'); +}; + +reset_case(); +$readiness_error = 1; +like(check(), qr/no usable path/, 'readiness refusal propagates'); +is_deeply(\@probed, [], 'no direct signature probe after path loss'); +ok(!$ready, 'pvs is not reached after path loss'); + +$storage->unmock('san_device_identity'); +my $root = tempdir(CLEANUP => 1); +my sub put { + my ($name, $file, $value) = @_; + my $dir = "$root/class/block/$name"; + make_path("$dir/holders", "$dir/dm", "$dir/device"); + open(my $fh, '>', "$dir/$file") or die $!; + print $fh "$value\n"; + close($fh) or die $!; +} +put('dm-0', 'dev', '253:0'); +put('dm-0', 'dm/uuid', 'mpath-test'); +put('sda', 'dev', '8:0'); +$disk->mock( + verify_blockdev_path => sub { $_[0] }, + is_partition => sub { 0 }, + is_san_device => sub { 1 }, +); +$identity->mock(path_wwid => sub { 'test' }); +my $ids = { luns => { one => { target => 'iqn.test', wwid => 'test' } } }; +is( + PVE::Storage::san_device_identity('/dev/dm-0', $ids, $root)->{key}, + 'scsi:test', + 'map UUID identity', +); +is( + PVE::Storage::san_device_identity('/dev/sda', $ids, $root)->{key}, + 'scsi:test', + 'single path identity', +); +symlink('../../dm-0', "$root/class/block/sda/holders/dm-0") or die $!; +eval { PVE::Storage::san_device_identity('/dev/sda', $ids, $root) }; +like($@, qr/member path/, 'member cannot be wiped'); +is( + PVE::Storage::san_device_identity('/dev/sda', $ids, $root, 1)->{key}, + 'scsi:test', + 'member-named reference resolves', +); +$disk->mock(is_partition => sub { $_[0] eq '/dev/sda1' }, get_blockdev => sub { '/dev/sda' }); +eval { PVE::Storage::san_device_identity('/dev/sda1', $ids, $root) }; +like($@, qr/member path/, 'partition of member cannot be wiped'); +$disk->mock(is_partition => sub { 0 }); +put('nvme0n1', 'dev', '259:0'); +put('nvme0n1', 'wwid', 'eui.test'); +# padded, as some controllers report it +put('nvme0n1', 'device/subsysnqn', 'nqn.test '); +my $ns = + PVE::Storage::san_device_identity('/dev/nvme0n1', sub { die 'unexpected SCSI probe' }, $root); +is($ns->{key}, 'nvme:eui.test', 'native namespace identity'); +is($ns->{subsystem}, 'nqn.test', 'native subsystem attribution'); + +# Offload initiators can run sessions to several targets on one SCSI host. Through the real +# collectors, only the LUN of the target that serves disk images is refused, not its peer. +{ + my $sys = tempdir(CLEANUP => 1); + local $PVE::Multipath::Identity::SYSFS = $sys; + local $PVE::Storage::ISCSIPlugin::SYSFS = $sys; + my sub write_file { + my ($path, $value) = @_; + make_path("$sys/$path" =~ s|/[^/]+$||r); + open(my $fh, '>', "$sys/$path") or die $!; + print $fh "$value\n"; + close($fh) or die $!; + } + my $lines = {}; + for my $lun ( + [1, 'iqn.images', '9:0:0:1', 'sdx', '8:240', '2EUIX'], + [2, 'iqn.other', '9:0:1:2', 'sdy', '65:0', '2EUIY'], + ) { + my ($sid, $target, $hctl, $name, $devt, $wwid) = @$lun; + my ($scsi_target) = $hctl =~ m/^(\d+:\d+:\d+):/; + my $session = "devices/platform/host9/session$sid"; + my $dir = "$session/target$scsi_target/$hctl"; + write_file("class/iscsi_session/session$sid/targetname", $target); + symlink("$sys/$session", "$sys/class/iscsi_session/session$sid/device") or die $!; + write_file("$dir/type", '0'); + write_file("$dir/block/$name/dev", $devt); + make_path("$sys/$dir/block/$name/holders", "$sys/bus/scsi/devices", "$sys/class/block"); + symlink("$sys/$dir", "$sys/bus/scsi/devices/$hctl") or die $!; + symlink("$sys/$dir", "$sys/$dir/block/$name/device") or die $!; + symlink("$sys/$dir/block/$name", "$sys/class/block/$name") or die $!; + $lines->{$name} = + { devt => $devt, hctl => $hctl, init => 'ok', target => $target, wwid => $wwid }; + } + $readiness_error = 0; + $identity->unmock($_) for grep { $identity->is_mocked($_) } qw(collect path_wwid); + $iscsi->unmock('iscsi_session_device_map'); + my $multipath = Test::MockModule->new('PVE::Multipath'); + $multipath->mock(query_path_identities => sub { $lines }); + $disk->mock( + verify_blockdev_path => sub { $_[0] }, + is_partition => sub { 0 }, + is_san_device => sub { 1 }, + ); + my $san_identity = \&PVE::Storage::san_device_identity; + $storage->mock( + san_device_identity => sub { $san_identity->($_[0], $_[1], $sys, $_[3]) }, + run_command => sub { + my ($cmd, %opt) = @_; + $opt{outfunc} + ->($cmd->[0] eq 'wipefs' ? '{"signatures":[]}' : '{"report":[{"pv":[]}]}'); + }, + ); + my $cfg = { + ids => + { img => { type => 'iscsi', target => 'iqn.images', content => { images => 1 } } }, + }; + eval { PVE::Storage::assert_san_write_safe($cfg, '/dev/sdx', []) }; + like( + $@, + qr/storage 'img' serves disk images from this target/, + 'shared host: the LUN of the image target is refused', + ); + eval { PVE::Storage::assert_san_write_safe($cfg, '/dev/sdy', []) }; + is($@, '', 'shared host: the LUN of the other target is not'); +} + +# A session device without an identity, for example of a LUN that multipathd is configured to +# ignore, stops writes to another LUN unless its VPD page proves that it is another one. +{ + my $sys = tempdir(CLEANUP => 1); + local $PVE::Multipath::Identity::SYSFS = $sys; + local $PVE::Storage::ISCSIPlugin::SYSFS = $sys; + my sub write_file { + my ($path, $value) = @_; + make_path("$sys/$path" =~ s|/[^/]+$||r); + open(my $fh, '>', "$sys/$path") or die $!; + print $fh $value; + close($fh) or die $!; + } + my sub naa_page { + return pack('C C n', 0, 0x83, 20) . pack('C C x C', 1, 3, 16) . pack('H*', $_[0]); + } + my ($W, $V) = ('600a09803832355367242f747a716b7a', '600a09803832355367242f747a716c30'); + my $lines = {}; + for my $lun ( + [1, 'iqn.data', '10:0:0:1', 'sdv', '65:16', "3$W"], + [2, 'iqn.ignored', '10:0:1:2', 'sdw', '65:32', undef], + ) { + my ($sid, $target, $hctl, $name, $devt, $wwid) = @$lun; + my ($scsi_target) = $hctl =~ m/^(\d+:\d+:\d+):/; + my $session = "devices/platform/host10/session$sid"; + my $dir = "$session/target$scsi_target/$hctl"; + write_file("class/iscsi_session/session$sid/targetname", "$target\n"); + symlink("$sys/$session", "$sys/class/iscsi_session/session$sid/device") or die $!; + write_file("$dir/type", "0\n"); + write_file("$dir/block/$name/dev", "$devt\n"); + make_path("$sys/$dir/block/$name/holders", "$sys/bus/scsi/devices", "$sys/class/block"); + symlink("$sys/$dir", "$sys/bus/scsi/devices/$hctl") or die $!; + symlink("$sys/$dir", "$sys/$dir/block/$name/device") or die $!; + symlink("$sys/$dir/block/$name", "$sys/class/block/$name") or die $!; + $lines->{$name} = + { devt => $devt, hctl => $hctl, init => 'ok', target => $target, wwid => $wwid } + if defined($wwid); + } + write_file('bus/scsi/devices/10:0:0:1/vpd_pg83', naa_page($W)); + my $multipath = Test::MockModule->new('PVE::Multipath'); + $multipath->mock(query_path_identities => sub { $lines }); + my $san_identity = $storage->original('san_device_identity'); + $storage->mock( + san_device_identity => sub { $san_identity->($_[0], $_[1], $sys, $_[3]) }, + run_command => sub { + my ($cmd, %opt) = @_; + $opt{outfunc} + ->($cmd->[0] eq 'wipefs' ? '{"signatures":[]}' : '{"report":[{"pv":[]}]}'); + }, + ); + my $write = sub { + eval { PVE::Storage::assert_san_write_safe({ ids => {} }, '/dev/sdv', []) }; + return $@; + }; + like( + $write->(), + qr/the identity of 'sdw' could not be verified/, + 'unidentified session device: refused without a VPD page', + ); + write_file('bus/scsi/devices/10:0:1:2/vpd_pg83', naa_page($W)); + like( + $write->(), + qr/the identity of 'sdw' could not be verified/, + 'unidentified session device: refused if it names the NAA of the LUN', + ); + write_file('bus/scsi/devices/10:0:1:2/vpd_pg83', naa_page($V)); + is($write->(), '', 'unidentified session device: not if it names the NAA of another LUN'); + $lines->{sdv}->{wwid} = '2EUIV'; + like( + $write->(), + qr/the identity of 'sdw' could not be verified/, + 'unidentified session device: refused for a LUN without an NAA WWID', + ); + $lines->{sdv}->{wwid} = "3$W"; + $lines->{sdw} = { devt => '65:32', hctl => '10:0:1:2', init => 'ok', wwid => "3$W" }; + like( + $write->(), + qr/the identity of 'sdw' could not be verified/, + 'unidentified session device: refused if the daemon names it, with an identity that did not' + . ' resolve', + ); + delete $lines->{sdw}; + + # a session device that only the strict walk of the sessions finds + my $collect = \&PVE::Multipath::Identity::collect; + $identity->mock( + collect => sub { + my $result = $collect->(@_); + delete $result->{paths}->{sdw}; + for my $key (keys $result->{luns}->%*) { + delete $result->{luns}->{$key} + if grep { $_ eq 'sdw' } ($result->{luns}->{$key}->{paths} // [])->@*; + } + return $result; + }, + ); + like( + $write->(), + qr/the identity of 'sdw' could not be verified/, + 'unidentified session device: refused if only the strict session walk finds it', + ); + $identity->unmock('collect'); + + delete $lines->{sdv}; + like($write->(), qr/unresolved LUN identity/, 'unidentified device to write: refused'); + eval { $san_identity->('/dev/sdv', $collect->(all => 1), $sys, 1) }; + like($@, qr/unresolved LUN identity/, 'unidentified reference: refused as a member, too'); +} + +done_testing(); diff --git a/src/test/run_test_lvm_base_volume.pl b/src/test/run_test_lvm_base_volume.pl index 208e7668..d16c0f70 100755 --- a/src/test/run_test_lvm_base_volume.pl +++ b/src/test/run_test_lvm_base_volume.pl @@ -28,8 +28,14 @@ $storage->mock( }, ); +my $diskmanage = Test::MockModule->new('PVE::Diskmanage'); +$diskmanage->mock(verify_blockdev_path => sub { $_[0] }); + my $lvm = Test::MockModule->new('PVE::Storage::LVMPlugin'); -$lvm->mock(lvm_create_volume_group => sub { push @calls, 'lvm_create_volume_group' }); +$lvm->mock( + assert_whole_lun => sub { }, + lvm_create_volume_group => sub { push @calls, 'lvm_create_volume_group' }, +); my $scfg = { base => 'san:0.0.0.scsi-test', vgname => 'vg', shared => 1 }; diff --git a/src/test/run_wipe_partition_tests.pl b/src/test/run_wipe_partition_tests.pl index 29ba74a2..3faa6032 100755 --- a/src/test/run_wipe_partition_tests.pl +++ b/src/test/run_wipe_partition_tests.pl @@ -16,7 +16,7 @@ my $disk = Test::MockModule->new('PVE::Diskmanage'); my $tools = Test::MockModule->new('PVE::Tools'); my $users = \&PVE::Diskmanage::block_device_users; my $parts = \&PVE::Diskmanage::wipe_partitions; -my (@commands, $drop_kernel); +my (@commands, $remove_error, $new_holder, $new_opener, $drop_kernel); my sub put { my ($path, $value) = @_; my ($dir) = "$root/$path" =~ m{^(.*)/[^/]+$}; @@ -33,45 +33,139 @@ my sub device { } my sub reset_case { remove_tree("$root/sys", "$root/proc"); - device('sda', '8:0', ''); - device('sda1', '8:1', ''); - put('sys/class/block/sda1/partition', '1'); - symlink('../sda1', "$root/sys/class/block/sda/sda1") or die $!; + device('dm-0', '253:0', 'mpath-test'); + device('dm-1', '253:1', 'part1-mpath-test'); + symlink('../../dm-1', "$root/sys/class/block/dm-0/holders/dm-1") or die $!; put('proc/self/mountinfo', '1 0 0:1 / / rw - rootfs rootfs rw'); put('proc/swaps', "Filename\tType\tSize\tUsed\tPriority"); @commands = (); - $drop_kernel = 0; + ($remove_error, $new_holder, $new_opener, $drop_kernel) = (0, 0, 0, 0); } $disk->mock( wipe_partitions => sub { $parts->($_[0], "$root/sys") }, block_device_users => sub { $users->($_[0], { %{ $_[1] // {} }, sysfs => "$root/sys", procfs => "$root/proc" }); }, - block_device_number => sub { '8:1' }, + block_device_number => sub { + my ($path) = @_; + my $target = -l $path ? readlink($path) : $path; + return $target eq '/dev/dm-0' ? '253:0' : '253:1'; + }, is_partition => sub { 0 }, dir_glob_foreach => sub { }, run_command => sub { my ($cmd) = @_; push @commands, $cmd; - if ($drop_kernel && $cmd->[0] eq 'wipefs' && $cmd->[-1] eq '/dev/sda') { - unlink("$root/sys/class/block/sda/sda1") or die $!; + if ($drop_kernel && $cmd->[0] eq 'wipefs' && $cmd->[-1] eq '/dev/dm-0') { + unlink("$root/sys/class/block/dm-0/sda1") or die $!; + } + if ($cmd->[0] eq 'dmsetup') { + die "remove failed\n" if $remove_error; + unlink("$root/sys/class/block/dm-0/holders/dm-1") or die $!; + if ($new_holder) { + device('dm-2', '253:2', 'LVM-test'); + symlink('../../dm-2', "$root/sys/class/block/dm-0/holders/dm-2") or die $!; + } + if ($new_opener) { + make_path("$root/proc/999998/fd"); + symlink('/dev/dm-0', "$root/proc/999998/fd/3") or die $!; + put('proc/999998/comm', 'qemu'); + } } }, ); $tools->mock(file_get_contents => sub { "8192\n" }); +my sub wipe { + eval { + PVE::Diskmanage::wipe_blockdev( + '/dev/dm-0', + PVE::Diskmanage::wipe_partitions('/dev/dm-0'), + ); + }; + return $@; +} reset_case(); +is_deeply(PVE::Diskmanage::wipe_partitions('/dev/dm-0'), ['dm-1'], 'exact kpartx child'); +is(wipe(), '', 'unused partition map can be wiped'); +is_deeply( + \@commands, + [ + ['wipefs', '--all', '/dev/dm-1'], + ['dmsetup', 'remove', '--retry', '/dev/dm-1'], + ['wipefs', '--all', '/dev/dm-0'], + ['dd', 'if=/dev/zero', 'of=/dev/dm-0', 'bs=1M', 'conv=fdatasync', 'count=4'], + ], + 'child signatures erased while mapped, ordinary removal precedes parent writes', +); +for my $case ( + [ + 'mounted', + sub { put('proc/self/mountinfo', '1 0 253:1 / /mnt rw - ext4 /dev/dm-1 rw') }, + qr/mounted/, + ], + [ + 'open', + sub { + make_path("$root/proc/999999/fd"); + symlink('/dev/dm-1', "$root/proc/999999/fd/3") or die $!; + put('proc/999999/comm', 'qemu'); + }, + qr/has dm-1 open/, + ], + [ + 'foreign UUID', + sub { put('sys/class/block/dm-1/dm/uuid', 'part1-mpath-other') }, + qr/held by/, + ], + ['opaque holder', sub { put('sys/class/block/dm-1/dm/uuid', 'LVM-test') }, qr/held by/], + [ + 'missing UUID', + sub { unlink("$root/sys/class/block/dm-1/dm/uuid") or die $! }, + qr/unable to identify/, + ], +) { + reset_case(); + $case->[1]->(); + like(wipe(), $case->[2], "$case->[0] refuses"); + is_deeply(\@commands, [], "$case->[0]: no writes"); +} +reset_case(); +$remove_error = 1; +like(wipe(), qr/remove failed/, 'failed child removal refuses'); +is(scalar(@commands), 2, 'parent untouched after failed removal'); +reset_case(); +$new_holder = 1; +like(wipe(), qr/held by dm-2/, 'new holder refuses'); +is(scalar(@commands), 2, 'parent untouched after unexpected holder'); +reset_case(); +$new_opener = 1; +like(wipe(), qr/has dm-0 open/, 'a process opening the parent meanwhile refuses'); +is(scalar(@commands), 2, 'parent untouched after a new opener'); +reset_case(); +device('dm-2', '253:2', 'part2-mpath-test'); +symlink('../../dm-2', "$root/sys/class/block/dm-1/holders/dm-2") or die $!; +is_deeply(PVE::Diskmanage::wipe_partitions('/dev/dm-0'), ['dm-2', 'dm-1'], 'recursive postorder'); +put('proc/self/mountinfo', '1 0 253:2 / /nested rw - ext4 /dev/dm-2 rw'); +like(wipe(), qr/dm-2 mounted/, 'nested child mount refuses'); +is_deeply(\@commands, [], 'nested users checked before any write'); + +reset_case(); +unlink("$root/sys/class/block/dm-0/holders/dm-1") or die $!; +device('sda1', '8:1', ''); +put('sys/class/block/sda1/partition', '1'); +symlink('../sda1', "$root/sys/class/block/dm-0/sda1") or die $!; $drop_kernel = 1; my $removed_kernel; eval { PVE::Diskmanage::wipe_blockdev( - '/dev/sda', + '/dev/dm-0', ['sda1'], { verify => sub { $removed_kernel = $_[0]->{sda1}; my $expected = $removed_kernel ? [] : ['sda1']; is_deeply( - PVE::Diskmanage::wipe_partitions('/dev/sda'), + PVE::Diskmanage::wipe_partitions('/dev/dm-0'), $expected, 'kernel partition topology matches deliberate removals', ); @@ -97,11 +191,11 @@ $disk->mock( ); my @verified; PVE::Diskmanage::wipe_blockdev( - '/dev/sda', - ['sda1'], + '/dev/dm-0', + ['dm-1'], { handle => $parent, - children => { 'sda1' => { stamp => 'test', dm_uuid => 'part1-mpath-test' } }, + children => { 'dm-1' => { stamp => 'test', dm_uuid => 'part1-mpath-test' } }, verify => sub { push @verified, { %{ $_[0] } } }, }, ); @@ -112,11 +206,182 @@ is_deeply( ); is_deeply( $commands[1], + ['dmsetup', 'remove', '--retry', '--uuid', 'part1-mpath-test'], + 'remove selects verified UUID, not reusable dm name', +); +is_deeply( + $commands[2], ['wipefs', '--all', "/proc/$$/fd/$parent_fd"], 'parent signatures use retained descriptor', ); -like($commands[2]->[2], qr{^of=/proc/$$/fd/$parent_fd$}, 'zeroing uses retained descriptor'); +like($commands[3]->[2], qr{^of=/proc/$$/fd/$parent_fd$}, 'zeroing uses retained descriptor'); ok(@verified >= 4, 'topology verified between destructive commands'); -is_deeply($verified[-1], {}, 'unchanged kernel partitions remain verified'); +is_deeply($verified[-1], { 'dm-1' => 1 }, 'removed children excluded from later verification'); + +reset_case(); +$disk->mock(verify_blockdev_path => sub { $_[0] }); +is_deeply( + PVE::Diskmanage::block_device_usage_hint('/dev/dm-0'), + { 'in-use' => 'no', 'in-use-by' => '' }, + 'unused kpartx children are not users', +); +put('sys/class/block/dm-0/dm/name', 'array-lun'); +put('sys/class/block/dm-1/dm/name', 'array-lun-part1'); +put('proc/self/mountinfo', '1 0 253:1 / /mounted\\040data rw - ext4 /dev/dm-1 rw'); +is_deeply( + PVE::Diskmanage::block_device_usage_hint('/dev/dm-0'), + { 'in-use' => 'yes', 'in-use-by' => 'array-lun-part1 mounted at /mounted data' }, + 'hint includes named child mounts', +); +put('proc/self/mountinfo', '1 0 0:1 / / rw - rootfs rootfs rw'); +device('dm-2', '253:2', 'LVM-test'); +put('sys/class/block/dm-2/dm/name', 'vg-data'); +symlink('../../dm-2', "$root/sys/class/block/dm-1/holders/dm-2") or die $!; +is_deeply( + PVE::Diskmanage::block_device_usage_hint('/dev/dm-0'), + { 'in-use' => 'yes', 'in-use-by' => 'array-lun-part1 held by vg-data' }, + 'hint names non-partition holders below kpartx children', +); +like(wipe(), qr/array-lun-part1 held by vg-data/, 'worker refusal names dm holders too'); +unlink("$root/sys/class/block/dm-2/dm/name") or die $!; +like( + PVE::Diskmanage::block_device_usage_hint('/dev/dm-0')->{'in-use-by'}, + qr/held by dm-2/, + 'kernel-name fallback for missing dm name', +); +unlink("$root/sys/class/block/dm-1/holders/dm-2") or die $!; +put('proc/swaps', 'deliberately invalid'); +make_path("$root/proc/999999/fd"); +symlink('/dev/dm-1', "$root/proc/999999/fd/3") or die $!; +is( + PVE::Diskmanage::block_device_usage_hint('/dev/dm-0')->{'in-use'}, + 'no', + 'hint excludes swap and process descriptor probes', +); +put('proc/self/mountinfo', 'invalid'); +is( + PVE::Diskmanage::block_device_usage_hint('/dev/dm-0')->{'in-use'}, + 'unknown', + 'failed metadata probe is not an unused result', +); + +# the paths of a map count with 'map-wwid', for example a logical volume that LVM activated on a +# path before the map existed +reset_case(); +put('sys/class/block/sdb/dev', '8:16'); +make_path("$root/sys/class/block/sdb/holders", "$root/sys/class/block/dm-0/slaves"); +symlink('../../sdb', "$root/sys/class/block/dm-0/slaves/sdb") or die $!; +symlink('../../dm-0', "$root/sys/class/block/sdb/holders/dm-0") or die $!; +my sub map_hint { + my ($wwid) = @_; + return PVE::Diskmanage::block_device_usage_hint( + '/dev/dm-0', + { 'map-wwid' => $wwid // 'test', sysfs => "$root/sys" }, + ); +} +is_deeply( + map_hint(), + { 'in-use' => 'no', 'in-use-by' => '' }, + 'map paths: held by the map only, with unused kpartx children', +); +device('dm-3', '253:3', 'LVM-other'); +put('sys/class/block/dm-3/dm/name', 'oldvg-data'); +symlink('../../dm-3', "$root/sys/class/block/sdb/holders/dm-3") or die $!; +is_deeply( + map_hint(), + { 'in-use' => 'yes', 'in-use-by' => 'sdb held by oldvg-data' }, + 'map paths: a logical volume on a path is a user', +); +is( + PVE::Diskmanage::block_device_usage_hint('/dev/dm-0')->{'in-use'}, + 'no', + 'map paths: not looked at without the WWID of the map', +); +is(map_hint('other')->{'in-use'}, 'unknown', 'map paths: unknown for a map of another WWID'); +unlink("$root/sys/class/block/sdb/holders/dm-3") or die $!; +put('proc/self/mountinfo', '1 0 8:16 / /raw rw - ext4 /dev/sdb rw'); +is(map_hint()->{'in-use-by'}, 'sdb mounted at /raw', 'map paths: a mounted path is a user'); +put('proc/self/mountinfo', '1 0 0:1 / / rw - rootfs rootfs rw'); +remove_tree("$root/sys/class/block/sdb"); +is(map_hint()->{'in-use'}, 'unknown', 'map paths: unknown when a path goes away meanwhile'); +device('dm-4', '253:4', 'LVM-map'); +put('sys/class/block/dm-4/dm/name', 'vg-map'); +symlink('../../dm-4', "$root/sys/class/block/dm-0/holders/dm-4") or die $!; +is_deeply( + map_hint(), + { 'in-use' => 'yes', 'in-use-by' => 'dm-0 held by vg-map' }, + 'map paths: a user of the map stays one if the check of a path fails', +); + +# the map changes while its paths are checked +my $during; +my sub path_check_mock { + $disk->mock( + block_device_users => sub { + my $found = + $users->($_[0], { %{ $_[1] // {} }, sysfs => "$root/sys", procfs => "$root/proc" }); + $during->() if $during && grep { $_ eq 'sdb' } $_[0]->@*; + return $found; + }, + ); +} +path_check_mock(); +reset_case(); +put('sys/class/block/sdb/dev', '8:16'); +make_path("$root/sys/class/block/sdb/holders", "$root/sys/class/block/dm-0/slaves"); +symlink('../../sdb', "$root/sys/class/block/dm-0/slaves/sdb") or die $!; +symlink('../../dm-0', "$root/sys/class/block/sdb/holders/dm-0") or die $!; +$during = sub { + put('sys/class/block/sdc/dev', '8:32'); + make_path("$root/sys/class/block/sdc/holders"); + symlink('../../sdc', "$root/sys/class/block/dm-0/slaves/sdc"); + symlink('../../dm-0', "$root/sys/class/block/sdc/holders/dm-0"); + device('dm-5', '253:5', 'LVM-new'); + put('sys/class/block/dm-5/dm/name', 'vg-new'); + symlink('../../dm-5', "$root/sys/class/block/sdc/holders/dm-5"); +}; +like( + map_hint()->{'in-use-by'}, + qr/^check failed: the paths of 'dm-0' changed/, + 'map paths: unknown if a path is added during the check', +); +$during = undef; +is(map_hint()->{'in-use-by'}, 'sdc held by vg-new', 'map paths: and its user found the next time'); +unlink("$root/sys/class/block/sdc/holders/dm-5") or die $!; +$during = sub { put('sys/class/block/dm-0/dm/uuid', 'mpath-other') }; +like( + map_hint()->{'in-use-by'}, + qr/^check failed: 'dm-0' is not the multipath map of 'test'/, + 'map paths: unknown if the map changes its identity during the check', +); +$during = undef; +# a user on one path stays one if the check of another path fails +reset_case(); +put('sys/class/block/sdb/dev', '8:16'); +make_path("$root/sys/class/block/sdb/holders", "$root/sys/class/block/dm-0/slaves"); +symlink('../../sdb', "$root/sys/class/block/dm-0/slaves/sdb") or die $!; +symlink('../../dm-0', "$root/sys/class/block/sdb/holders/dm-0") or die $!; +device('dm-3', '253:3', 'LVM-other'); +put('sys/class/block/dm-3/dm/name', 'oldvg-data'); +symlink('../../dm-3', "$root/sys/class/block/sdb/holders/dm-3") or die $!; +symlink('../../sdc', "$root/sys/class/block/dm-0/slaves/sdc") or die $!; +is_deeply( + map_hint(), + { 'in-use' => 'yes', 'in-use-by' => 'sdb held by oldvg-data' }, + 'map paths: a user of a path stays one if the check of another path fails', +); +$disk->mock( + block_device_users => sub { + $users->($_[0], { %{ $_[1] // {} }, sysfs => "$root/sys", procfs => "$root/proc" }); + }, +); + +@commands = (); +PVE::Diskmanage::udevadm_trigger('/dev/dm-0', '/dev/sda1'); +is_deeply( + \@commands, + [['udevadm', 'trigger', '--settle', '/dev/dm-0', '/dev/sda1']], + 'device-specific trigger waits for udev processing', +); done_testing(); diff --git a/src/test/run_wipe_readiness_tests.pl b/src/test/run_wipe_readiness_tests.pl new file mode 100755 index 00000000..ef5977a3 --- /dev/null +++ b/src/test/run_wipe_readiness_tests.pl @@ -0,0 +1,138 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use File::Path qw(make_path remove_tree); +use File::Temp qw(tempdir); +use Test::MockModule; +use Test::More; + +use lib ('.', '..'); +use PVE::Diskmanage; +use PVE::Multipath; +use PVE::Multipath::Identity; + +my $root = tempdir(CLEANUP => 1); +my $disk = Test::MockModule->new('PVE::Diskmanage'); +my $mp = Test::MockModule->new('PVE::Multipath'); +my $identity = Test::MockModule->new('PVE::Multipath::Identity'); +my ($ids, $maps, $nvme); +my sub put { + my ($path, $value) = @_; + my ($dir) = "$root/$path" =~ m{^(.*)/[^/]+$}; + make_path($dir); + open(my $fh, '>', "$root/$path") or die $!; + print $fh "$value\n"; + close($fh) or die $!; +} +my sub device { + my ($name, $number) = @_; + put("sys/class/block/$name/dev", $number); + put("sys/class/block/$name/size", '8192'); + make_path("$root/sys/class/block/$name/holders"); +} +$disk->mock( + is_partition => sub { 0 }, + is_san_device => sub { 1 }, + get_nvme_path_status => sub { $nvme }, + block_device_number => sub { '8:16' }, +); +$mp->mock(get_maps => sub { $maps }); +my %other_lun; +$identity->mock( + collect => sub { $ids }, + path_wwid => sub { $_[0]->{paths}->{ $_[1] }->{wwid} }, + path_is_other_lun => sub { $other_lun{ $_[1] } && $_[2] eq 'test' }, +); +my sub reset_case { + remove_tree("$root/sys", "$root/proc"); + device('sda', '8:0'); + put('sys/class/block/sda/device/state', 'running'); + $ids = { paths => { sda => { wwid => 'test' } } }; + $maps = []; + $nvme = {}; + put('proc/self/mountinfo', '1 0 0:1 / / rw - rootfs rootfs rw'); + put('proc/swaps', "Filename\tType\tSize\tUsed\tPriority"); +} +my sub check { + my ($dev) = @_; + eval { + PVE::Diskmanage::assert_block_device_ready( + "/dev/$dev", + { sysfs => "$root/sys", procfs => "$root/proc" }, + ); + }; + return $@; +} +reset_case(); +is(check('sda'), '', 'single running path needs no path count'); +put('sys/class/block/sda/device/state', 'offline'); +like(check('sda'), qr/not running/, 'offline single path refuses'); +put('sys/class/block/sda/device/state', 'running'); +device('sdb', '8:16'); +$ids->{paths}->{sdb} = { wwid => 'test' }; +like(check('sda'), qr/multiple paths/, 'unmapped same-identity sibling refuses'); +$ids->{paths}->{sdb}->{wwid} = 'other'; +is(check('sda'), '', 'different identity is not a sibling'); +# a local disk that multipathd does not list +$ids->{paths}->{sdz} = { wwid => undef }; +like( + check('sda'), + qr/cannot exclude 'sdz' as another path of this LUN/, + 'path without identity refuses', +); +$other_lun{sdz} = 1; +is(check('sda'), '', 'path without identity that is provably another LUN does not'); +delete $ids->{paths}->{sdz}; +$ids->{paths}->{sdb}->{wwid} = 'test'; +device('dm-0', '253:0'); +put('sys/class/block/dm-0/dm/uuid', 'mpath-test'); +make_path("$root/sys/class/block/dm-0/slaves"); +symlink('../../sda', "$root/sys/class/block/dm-0/slaves/sda") or die $!; +symlink('../../dm-0', "$root/sys/class/block/sda/holders/dm-0") or die $!; +my $path = { dev => 'sda', 'dm-state' => 'active', 'check-state' => 'ready' }; +$maps = [{ sysfs => 'dm-0', wwid => 'test', 'path-groups' => [{ paths => [$path] }] }]; +is(check('dm-0'), '', 'map with current usable path allowed'); + +for my $state ('faulty', 'unchecked', 'shaky') { + $path->{'check-state'} = $state; + like(check('dm-0'), qr/no usable path/, "$state checker refuses despite SCSI running"); +} +$path->{'check-state'} = 'ready'; +$path->{'dm-state'} = 'failed'; +like(check('dm-0'), qr/no usable path/, 'failed dm path refuses'); +$path->{'dm-state'} = 'active'; +$maps->[0]->{wwid} = 'other'; +like(check('dm-0'), qr/no usable path/, 'daemon map identity must match sysfs'); +$maps->[0]->{wwid} = 'test'; +make_path("$root/proc/999999/fd"); +symlink('/dev/sdb', "$root/proc/999999/fd/3") or die $!; +put('proc/999999/comm', 'qemu'); +like(check('dm-0'), qr/qemu .* has sdb open/, 'raw open of unmapped sibling refuses map wipe'); +symlink('../../sdb', "$root/sys/class/block/dm-0/slaves/sdb") or die $!; +symlink('../../dm-0', "$root/sys/class/block/sdb/holders/dm-0") or die $!; +like(check('dm-0'), qr/qemu .* has sdb open/, 'raw open of map member refuses'); +symlink('/usr/sbin/multipathd', "$root/proc/999999/exe") or die $!; +is(check('dm-0'), '', 'only multipathd checker descriptors excluded'); +unlink("$root/proc/999999/exe") or die $!; +symlink('/usr/bin/qemu', "$root/proc/999999/exe") or die $!; +like(check('dm-0'), qr/has sdb open/, 'other executables remain users'); +reset_case(); +device('nvme0n1', '259:0'); +put('sys/class/block/nvme0n1/wwid', 'eui.0123456789abcdef'); +put('sys/class/block/nvme0n1/device/state', 'live'); +is(check('nvme0n1'), '', 'single live NVMe controller needs no path count'); +$nvme = { nvme0n1 => { active => 0 } }; +like( + check('nvme0n1'), + qr/no usable path/, + 'native head with no ANA-usable path refuses despite live state', +); +$nvme->{nvme0n1}->{active} = 1; +is(check('nvme0n1'), '', 'ANA-usable native head allowed'); +device('nvme1n1', '259:1'); +put('sys/class/block/nvme1n1/wwid', 'eui.0123456789abcdef'); +like(check('nvme0n1'), qr/multiple devices/, 'uncoalesced same-identity NVMe namespaces refuse'); + +done_testing(); diff --git a/src/test/run_wipe_reference_tests.pl b/src/test/run_wipe_reference_tests.pl index 610cb691..c47f7c00 100755 --- a/src/test/run_wipe_reference_tests.pl +++ b/src/test/run_wipe_reference_tests.pl @@ -13,9 +13,11 @@ use PVE::API2::Disks; my $root = tempdir(CLEANUP => 1); my $disk = Test::MockModule->new('PVE::Diskmanage'); +my $storage = Test::MockModule->new('PVE::Storage'); my $rpc = Test::MockModule->new('PVE::RPCEnvironment'); +my $identity = Test::MockModule->new('PVE::Multipath::Identity'); my $reference = \&PVE::Diskmanage::block_device_reference; -my ($resolved, $worker, $locked, $written); +my ($resolved, $worker, $locked, $probes, $written, $rebind); my sub put { my ($path, $value) = @_; my ($dir) = "$root/$path" =~ m{^(.*)/[^/]+$}; @@ -24,8 +26,10 @@ my sub put { print $fh "$value\n"; close($fh) or die $!; } -put('class/block/sda/dev', '8:0'); -put('class/block/sdb/dev', '8:16'); +for my $n (0, 1, 2) { + put("class/block/dm-$n/dev", "253:$n"); + put("class/block/dm-$n/dm/uuid", $n == 2 ? 'mpath-other' : 'mpath-test:123'); +} { package ReferenceTestRPC; @@ -40,17 +44,12 @@ $disk->mock( ok($locked, 're-resolution occurs after locks') if $_[1]; $reference->($_[0], $_[1], $root); }, - locked_disk_action => sub { - $locked = 1; - eval { $_[0]->() }; - my $err = $@; - $locked = 0; - die $err if $err; - }, + locked_disk_action => sub { $_[0]->() }, block_device_users => sub { [] }, - is_mounted => sub { undef }, - has_holder => sub { undef }, wipe_partitions => sub { [] }, + is_san_device => sub { 1 }, + is_multipath_device => sub { 1 }, + assert_block_device_ready => sub { }, open_block_device => sub { \*STDOUT }, wipe_blockdev => sub { $_[2]->{verify}->(); @@ -59,22 +58,89 @@ $disk->mock( }, udevadm_trigger => sub { }, ); +$storage->mock( + config => sub { { ids => {} } }, + lock_storage_config => sub { + local $@; + $locked = 1; + eval { $_[0]->() }; + my $err = $@; + $locked = 0; + die $err if $err; + }, + assert_san_write_safe => sub { + $probes++; + $resolved = $rebind == 2 ? '/dev/dm-1' : '/dev/dm-2' if $rebind; + }, +); my $method = PVE::API2::Disks->map_method_by_name('wipe_disk')->{code}; my sub queue { my ($path) = @_; - $written = undef; - $resolved = '/dev/sda'; + ($probes, $written, $rebind) = (0, undef, 0); + $resolved = '/dev/dm-0'; $method->({ node => 'test', disk => $path }); } -queue('/dev/sda'); +my $canonical = '/dev/disk/by-id/dm-uuid-mpath-test:123'; +queue($canonical); +$resolved = '/dev/dm-1'; eval { $worker->() }; -is($@, '', 'delayed worker accepts the same device instance'); -is($written, '/dev/sda', 'worker uses the verified device'); -queue('/dev/sda'); -$resolved = '/dev/sdb'; +is($@, '', 'delayed worker accepts the same identity after renumbering'); +is($written, '/dev/dm-1', 'worker never uses the old reusable kernel name'); +for my $path ($canonical, '/dev/mapper/friendly') { + queue($path); + $resolved = '/dev/dm-2'; + eval { $worker->() }; + like($@, qr/identity (?:changed|does not match)/, 'rebound alias refuses'); + is($probes, 0, 'rebound alias refuses before device-reading probes'); + ok(!defined($written), 'rebound alias does not write'); +} +queue($canonical); +$rebind = 1; eval { $worker->() }; -like($@, qr/identity changed/, 'rebound reference refuses'); -ok(!defined($written), 'rebound reference does not write'); +like($@, qr/identity does not match/, 'rebind during guard probes refuses'); +ok(!defined($written), 'rebind during guard probes does not write'); +queue($canonical); +$rebind = 2; +eval { $worker->() }; +is( + $@, + "cannot use '/dev/dm-0' - device topology changed\n", + 'shared instance check refuses same-identity replacement after probing', +); +ok(!defined($written), 'same-identity replacement during probes does not write'); +$resolved = '/dev/dm-2'; +eval { $method->({ node => 'test', disk => $canonical }) }; +like($@, qr/identity does not match/, 'canonical name verified at request time too'); + +$disk->unmock('block_device_reference'); +put('class/block/nvme0n1/dev', '259:0'); +put('class/block/nvme0n1/wwid', 'eui.0123456789abcdef'); +$resolved = '/dev/nvme0n1'; +is( + $reference->('/dev/disk/by-id/nvme-eui.0123456789abcdef', undef, $root)->{key}, + 'nvme:eui.0123456789abcdef', + 'NVMe canonical identity verified', +); +eval { $reference->('/dev/disk/by-id/nvme-eui.1111111111111111', undef, $root) }; +like($@, qr/identity does not match/, 'wrong NVMe identity refuses'); +put('class/block/sda/dev', '8:0'); +$identity->mock(collect => sub { {} }, path_wwid => sub { '3600abc' }); +$resolved = '/dev/sda'; +is( + $reference->('/dev/disk/by-id/scsi-3600abc', undef, $root)->{key}, + 'scsi:3600abc', + 'SCSI by-id verified against live identity', +); +eval { $reference->('/dev/disk/by-id/scsi-3600def', undef, $root) }; +like($@, qr/identity does not match/, 'wrong SCSI identity refuses'); + +is( + $reference->('/dev/disk/by-id/wwn-0x600abc', undef, $root)->{key}, + 'scsi:3600abc', + 'WWN alias verified against NAA identity', +); +eval { $reference->('/dev/disk/by-id/wwn-0x600def', undef, $root) }; +like($@, qr/identity does not match/, 'wrong WWN refuses'); $disk->unmock('open_block_device'); put('handle', 'not a device'); @@ -86,6 +152,56 @@ my $handle = PVE::Diskmanage::open_block_device("$root/handle", '1:8:0'); ok(defined(fileno($handle)), 'matching opened reference retained'); close($handle) or die $!; +$disk->unmock('block_device_stamp'); +my %links = ( + '/dev/mapper/friendly' => '/dev/dm-0', + '/dev/disk/by-id/dm-uuid-mpath-test:123' => '/dev/dm-0', + '/dev/nvme0n1' => '/dev/nvme0n1', + '/dev/disk/by-id/nvme-eui.0123456789abcdef' => '/dev/nvme0n1', + '/dev/sda' => '/dev/sda', + '/dev/disk/by-id/scsi-3600abc' => '/dev/sda', +); +$disk->mock(verify_blockdev_path => sub { $links{ $_[0] } // die "missing link\n" }); +is( + PVE::Diskmanage::canonical_blockdev_link('/dev/mapper/friendly', {}, 'test:123', $root), + $canonical, + 'scan canonical map link comes from UUID, not the friendly name', +); +is( + PVE::Diskmanage::canonical_blockdev_link('/dev/mapper/friendly', {}, 'other', $root), + undef, + 'scan omits link when the map identity changed', +); +$links{$canonical} = '/dev/dm-2'; +is( + PVE::Diskmanage::canonical_blockdev_link('/dev/mapper/friendly', {}, 'test:123', $root), + undef, + 'scan omits a rebound canonical link', +); +delete $links{$canonical}; +is( + PVE::Diskmanage::canonical_blockdev_link('/dev/mapper/friendly', {}, 'test:123', $root), + undef, + 'scan omits a missing canonical link', +); +is( + PVE::Diskmanage::canonical_blockdev_link('/dev/nvme0n1', {}, undef, $root), + '/dev/disk/by-id/nvme-eui.0123456789abcdef', + 'scan uses identity-bearing NVMe link', +); +is( + PVE::Diskmanage::canonical_blockdev_link('/dev/sda', {}, '3600abc', $root), + '/dev/disk/by-id/scsi-3600abc', + 'scan uses verified single-path SCSI link', +); +delete $links{'/dev/disk/by-id/scsi-3600abc'}; +$links{'/dev/disk/by-id/wwn-0x600abc'} = '/dev/sda'; +is( + PVE::Diskmanage::canonical_blockdev_link('/dev/sda', {}, '3600abc', $root), + '/dev/disk/by-id/wwn-0x600abc', + 'scan can use the corresponding WWN link', +); + subtest 'checked wipe keeps verification order and retained handle' => sub { my @events; my $handle; @@ -110,14 +226,18 @@ subtest 'checked wipe keeps verification order and retained handle' => sub { ok(defined(fileno($handle)), 'parent handle stays open through trigger'); }, ); - PVE::Diskmanage::wipe_blockdev_checked('/dev/sda', {}); + PVE::Diskmanage::wipe_blockdev_checked('/dev/sda', {}, sub { push @events, 'storage' }); is_deeply( \@events, - [qw(reference partitions reference partitions open users + [qw(reference partitions ready reference partitions open storage ready users reference partitions wipe reference partitions trigger)], - 'retained open, users, verification, wipe and trigger order', + 'readiness, retained open, storage check, users, verification, wipe and trigger order', ); close($handle); + @events = (); + eval { PVE::Diskmanage::wipe_blockdev_checked('/dev/sda', {}, undef) }; + like($@, qr/device topology changed/, 'local-to-SAN transition refuses without callback'); + is_deeply(\@events, ['reference'], 'transition refuses before readiness, opens or writes'); }; done_testing(); diff --git a/src/test/run_wipedisk_tests.pl b/src/test/run_wipedisk_tests.pl index f110a2f4..dafd08a8 100755 --- a/src/test/run_wipedisk_tests.pl +++ b/src/test/run_wipedisk_tests.pl @@ -12,6 +12,23 @@ use PVE::API2::Disks; my $disk = Test::MockModule->new('PVE::Diskmanage'); my $rpc = Test::MockModule->new('PVE::RPCEnvironment'); my ($worker, $users, $probe_error, $wiped); +my ($san, $storage_locked, $guard_error) = (0, 0, undef); +my $storage = Test::MockModule->new('PVE::Storage'); +$storage->mock( + config => sub { { ids => {} } }, + lock_storage_config => sub { + $storage_locked = 1; + my $res = eval { $_[0]->() }; + my $err = $@; + $storage_locked = 0; + die $err if $err; + return $res; + }, + assert_san_write_safe => sub { + ok($storage_locked, 'SAN guard under storage configuration lock'); + die $guard_error if $guard_error; + }, +); my $inside_worker = 0; { @@ -21,23 +38,29 @@ my $inside_worker = 0; } $rpc->mock(get => sub { bless {}, 'WipeTestRPC' }); $disk->mock( - verify_blockdev_path => sub { '/dev/sda' }, + verify_blockdev_path => sub { '/dev/dm-9' }, block_device_reference => sub { + # a local disk is wiped without the cluster-wide lock, which needs quorum + is($storage_locked, $san ? 1 : 0, 'storage lock only for a SAN device') if $_[1]; return { - device => '/dev/sda', - stamp => '1:8:0', + device => '/dev/dm-9', + stamp => '1:253:9', key => 'scsi:test', partition => '', }; }, open_block_device => sub { \*STDOUT }, locked_disk_action => sub { $_[0]->() }, + is_san_device => sub { $san }, + is_multipath_device => sub { 0 }, is_mounted => sub { undef }, has_holder => sub { undef }, wipe_partitions => sub { [] }, + assert_block_device_ready => sub { ok($inside_worker, 'readiness checked in worker') }, block_device_users => sub { + return [] if $_[1]->{'metadata-only'}; ok($inside_worker, 'users checked in the worker'); - is_deeply($_[0], ['sda'], 'resolved name checked, including partitions'); + is_deeply($_[0], ['dm-9'], 'resolved name checked, including partitions'); die $probe_error if $probe_error; return $users; }, @@ -45,15 +68,26 @@ $disk->mock( udevadm_trigger => sub { }, ); my $method = PVE::API2::Disks->map_method_by_name('wipe_disk'); +my $pattern = $method->{parameters}->{properties}->{disk}->{pattern}; +for my $path ( + '/dev/sda1', + '/dev/dm-9', + '/dev/mapper/array_lun-1.0', + '/dev/disk/by-id/scsi-123', + '/dev/disk/by-id/dm-uuid-mpath-naa:123', +) { + like($path, qr/$pattern/, 'valid device path'); +} +unlike('/dev/x;touch', qr/$pattern/, 'metacharacter refused'); for my $case ( [[], undef, 1, qr/^$/], - [['qemu (PID 7) has sda open'], undef, 0, qr/cannot wipe.*qemu/], + [['qemu (PID 7) has dm-9 open'], undef, 0, qr/cannot wipe.*qemu/], [[], "probe failed\n", 0, qr/probe failed/], ) { ($users, $probe_error, my $expected, my $error) = @$case; $wiped = 0; $inside_worker = 0; - $method->{code}->({ node => 'test', disk => '/dev/sda' }); + $method->{code}->({ node => 'test', disk => '/dev/mapper/array_lun-1.0' }); is($wiped, 0, 'request does not wipe before worker'); $inside_worker = 1; eval { $worker->() }; @@ -61,4 +95,16 @@ for my $case ( is($wiped, $expected, 'only unused devices wiped'); } +$san = 1; +$users = []; +$probe_error = undef; +for my $error (undef, "storage reference\n") { + $guard_error = $error; + $wiped = 0; + $method->{code}->({ node => 'test', disk => '/dev/dm-9' }); + eval { $worker->() }; + is($@, $error // '', 'SAN guard outcome'); + is($wiped, $error ? 0 : 1, 'referenced SAN device not wiped'); +} + done_testing(); -- 2.47.3