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 9D64C1FF0EF for ; Sun, 02 Aug 2026 05:33:11 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id CADCB2153F; Sun, 02 Aug 2026 05:32:56 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=neatech-ar.20251104.gappssmtp.com; s=20251104; t=1785641564; x=1786246364; darn=lists.proxmox.com; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to:content-type; bh=uP6sPd9wo/s1qo4B0AggAx2UAkzLZsftW3/2nnqQEnQ=; b=iRW8kRUJdAWcHxXq0S4aL/74WRP5kbUorMH5gGOVUc9vOQ0xfhCQfgyedgstZGZRw0 VdLAy2JQxeCddqbDPOG4noTMTRey6JhK2AKbSo1XlqkWbu7ZKFwBrH/vAX41THyjJT+S 95ULw+LaCvK6vQFBZ7dOCVC6k4v7WWTEekQDq3brWid05rmp3ANfBXC6yjpwVRzFAxuC q+6YjqiCVSfo3X3rZliRf11/SxNkjXILhn8/h0pKMMb1YoQuHONfOZ9lC5FZR6654bdp sZvwLAxKclKDVDbcuZBQkZyoZQ/sukglQ5Q+7VCLyzk4OTbghvTTjkeNPbuoIbaB1bRF Qr3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785641564; x=1786246364; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to:content-type; bh=uP6sPd9wo/s1qo4B0AggAx2UAkzLZsftW3/2nnqQEnQ=; b=j3WC/gKng0U58LlS9eOBy6sz5mMmDmciX4GFyVyJseoMGBLUgksQyf0Ym3znzvw2pl nsDDQ5HY025Cz8g3iljkhqJFd/RtmIR6RsH0mU6QZ+wBlSRiefaqD2KerYHBsy5FMtOb yMmeSm3BuMNVnXYsqyKyDOQ52a8Tl0O016Z+7g9YodUaUF5MfW64tDaphKVGqTm2OFe+ W+LkIHGQzDKU1CrunBzHRzl/lSuZyGlvZPz6/N2jaDwX/FGxdba6fv8iaJObVe2dY7sW Ia75UIoooWSK/6iZ53j3benKCxFXhCsHXle0bZyM+ttTbzQFQQUVbY7kpudl5r9TyK06 RtPg== X-Gm-Message-State: AOJu0YzOvh0WIHSCJdJYy3TbmjLgNM96J9R/tBqlJSSHAcmP9ugIbBco FCgyH90dj72NT0R7JjfZ5/Bn/ymEwk/HAq3Au3qzW15cnVC+vee3vY3L18UY627b+rvkRw/UNdi V3vhWZBQ= X-Gm-Gg: AR+sD122rqxsPMag3ZbhRmqtexzYAOrfDyXSSJeByDpgqL0+VtJKLhnJHNmNVaAxyEs DpZUj+WBsjRDax9kLrK8B9lyrvMWibdENvSy+hzCVBGZG2oEe+dHzY9+LXwtGCVJWK+K9Oj09JS NnT478pUYRlRTNtFMqWf9rnogahpQaDFGg64yGnfGif1Xl475R3/M694D+FuLFeivwsWHh4dwOi DsZfQz6nbQwgtC74wulgw6Dxdp8rQc23GRiXmagVVXqE9Zruavs1U31EFz4bknPSPp15/6JJdXc CFDFrLeSZ4IKnVi24oKMU7SA2E45n3pqFpmORW6QForN4PuB+sv3ymWsrjtMxpO66wyBFotQpMj NuK5tUCCkmQkkxFQKxiYHWebHj5pGLD2OjgkIvSipbZfHfTdfHPsz8ocnRXUxcr15FB+mcE70QW wzapGGr6SvwMRn05hnMZvr7mo+Cml+8ft/G45mVf1ZhheKnQKmmMK9wOKqEGxRaMCibfagQ7zOy mZPaDQhuKCWA7Z/Uqf7nW09ZFbVRN/801WKIi9dnIF8 X-Received: by 2002:a17:90b:4b81:b0:38e:6d4c:14df with SMTP id 98e67ed59e1d1-38fbc3dad68mr6377974a91.1.1785641564344; Sat, 01 Aug 2026 20:32:44 -0700 (PDT) From: Joaquin Varela To: pve-devel@lists.proxmox.com Subject: [PATCH storage v2 3/7] zfsnvme: harden node preflight and storage teardown Date: Sun, 2 Aug 2026 00:31:37 -0300 Message-ID: <5f7bd4d7f31002351998f14b32f0135d6fea99a7.1785636979.git.joaquinvarela@neatech.ar> X-Mailer: git-send-email 2.54.0.windows.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.000 Adjusted score from AWL reputation of From: address DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DMARC_PASS -0.1 DMARC pass policy RCVD_IN_DNSWL_NONE -0.0001 Sender listed at https://www.dnswl.org/, no trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: VHPZ5OTM7YY7SKK5CGICFE5RNFPBNBYM X-Message-ID-Hash: VHPZ5OTM7YY7SKK5CGICFE5RNFPBNBYM X-MailFrom: joaquinvarela@neatech.ar X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Joaquin Varela X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Fail activation before target mutation when a configured host interface is absent. Reconcile controllers one path at a time when their bound interface differs from storage configuration. Before disconnecting a subsystem, reject teardown while a userspace file descriptor or kernel holder consumes one of its namespace heads. Also reject storage deletion while its remote ZFS dataset owns volumes. These guards prevent an idle cluster node from disrupting users on another. Signed-off-by: Joaquin Varela --- src/PVE/Storage/ZFSNVMePlugin.pm | 87 ++++++++++++++++++++++++++++++-- src/test/zfsnvme_test.pm | 44 +++++++++++++++- 2 files changed, 127 insertions(+), 4 deletions(-) diff --git a/src/PVE/Storage/ZFSNVMePlugin.pm b/src/PVE/Storage/ZFSNVMePlugin.pm index 83d086c..d02dff4 100644 --- a/src/PVE/Storage/ZFSNVMePlugin.pm +++ b/src/PVE/Storage/ZFSNVMePlugin.pm @@ -46,9 +46,11 @@ my $RE_DHCHAP_KEY = qr{ }nxx; my $RE_NVME_CONTROLLER = qr{\A nvme [0-9]+ \z}nxx; my $RE_NVME_SUBSYSTEM = qr{\A nvme-subsys [0-9]+ \z}nxx; +my $RE_NVME_NAMESPACE = qr{\A nvme [0-9]+ n [0-9]+ \z}nxx; my $RE_TRADDR = qr{(?: \A | ,) traddr=(?[^,]+)}nxx; my $RE_TRSVCID = qr{(?: \A | ,) trsvcid=(?[^,]+)}nxx; my $RE_HOST_IFACE_ADDRESS = qr{(?: \A | ,) host_iface=(?[^,]+)}nxx; +my $RE_PROC_FD = qr{\A /proc/ (?[0-9]+) /}nxx; sub verify_nvme_nqn($value, $noerr = undef) { @@ -155,6 +157,18 @@ sub _configured_portals($scfg) { return $portals; } +sub _local_iface_exists($iface) { + return -d "/sys/class/net/$iface"; +} + +sub _validate_local_ifaces($portals) { + for my $portal ($portals->@*) { + my $iface = $portal->{host_iface}; + die "NVMe/TCP host interface '$iface' does not exist on this node\n" + if !_local_iface_exists($iface); + } +} + PVE::JSONSchema::register_format('pve-storage-nvme-nqn', \&verify_nvme_nqn); PVE::JSONSchema::register_format('pve-storage-nvme-portals', \&verify_nvme_portals); PVE::JSONSchema::register_format('pve-storage-nvme-host-ifaces', \&verify_nvme_host_ifaces); @@ -385,8 +399,19 @@ sub on_update_hook_full($class, $storeid, $scfg, $update, $delete, $sensitive) { } sub on_delete_hook($class, $storeid, $scfg) { - eval { $class->deactivate_storage($storeid, $scfg) }; - log_warn("failed to disconnect NVMe storage '$storeid': $@") if $@; + # This hook runs only on the API node, while another cluster node can still + # be using the shared storage. Requiring an empty owned dataset makes the + # check cluster-wide without relying on remote process inspection. + my $volumes = $class->zfs_list_zvol($scfg); + my @volumes = sort keys $volumes->%*; + die "refusing to remove NVMe storage '$storeid': it still contains " + . join(', ', @volumes) . "\n" + if @volumes; + + # Refuse the configuration removal while a VM or another process still has + # one of this subsystem's namespaces open. Otherwise deleting the storage + # could turn an administrative mistake into immediate guest I/O errors. + $class->deactivate_storage($storeid, $scfg); eval { PVE::Storage::LunCmd::NVMET::delete_target($scfg) }; log_warn("failed to remove NVMe target for '$storeid': $@") if $@; delete_secret($storeid); @@ -461,6 +486,57 @@ my sub controller_states($nqn) { return $states; } +my sub namespace_devices($nqn) { + my $devices = {}; + + opendir(my $dh, '/sys/class/nvme-subsystem') or return $devices; + while (defined(my $entry = readdir($dh))) { + next if $entry !~ $RE_NVME_SUBSYSTEM; + my $base = "/sys/class/nvme-subsystem/$entry"; + my $subsys = file_read_firstline("$base/subsysnqn"); + next if !defined($subsys) || $subsys ne $nqn; + + opendir(my $subsys_dh, $base) or next; + while (defined(my $device = readdir($subsys_dh))) { + next if $device !~ $RE_NVME_NAMESPACE; + $devices->{"/dev/$device"} = $device if -b "/dev/$device"; + } + closedir($subsys_dh); + } + closedir($dh); + return $devices; +} + +sub _namespace_openers($nqn) { + my $devices = namespace_devices($nqn); + return [] if !$devices->%*; + + my %openers; + for my $fd (glob('/proc/[0-9]*/fd/[0-9]*')) { + my $target = readlink($fd); + next if !defined($target) || !exists($devices->{$target}); + my $pid = $fd =~ $RE_PROC_FD ? $+{pid} : undef; + next if !defined($pid); + my $comm = eval { file_read_firstline("/proc/$pid/comm") } // 'unknown'; + $openers{"$pid:$target"} = "$comm (PID $pid, $target)"; + } + + # Kernel consumers such as device-mapper do not necessarily keep a userspace + # file descriptor open, but expose their dependency in the holders directory. + for my $path (keys $devices->%*) { + my $device = $devices->{$path}; + my $holders = "/sys/class/block/$device/holders"; + opendir(my $holders_dh, $holders) or next; + while (defined(my $holder = readdir($holders_dh))) { + next if $holder eq '.' || $holder eq '..'; + $openers{"holder:$device:$holder"} = "$path held by $holder"; + } + closedir($holders_dh); + } + + return [sort values %openers]; +} + my sub portal_reachable($portal) { my $socket = IO::Socket::IP->new( PeerHost => $portal->{address}, @@ -538,8 +614,9 @@ sub activate_storage($class, $storeid, $scfg, $cache = undef) { die "native NVMe multipath is disabled in the running kernel\n" if (file_read_firstline('/sys/module/nvme_core/parameters/multipath') // 'N') ne 'Y'; - _assert_unique_target($storeid, $scfg); my $portals = _configured_portals($scfg); + _validate_local_ifaces($portals); + _assert_unique_target($storeid, $scfg); my $states = controller_states($scfg->{subsysnqn}); my $force_reconcile = delete($cache->{'zfsnvme-force-reconcile'}->{$storeid}) // 0; @@ -637,6 +714,10 @@ sub activate_storage($class, $storeid, $scfg, $cache = undef) { } sub deactivate_storage($class, $storeid, $scfg, $cache = undef) { + my $openers = _namespace_openers($scfg->{subsysnqn}); + die "refusing to disconnect NVMe storage '$storeid': namespace in use by " + . join(', ', $openers->@*) . "\n" + if $openers->@*; run_command( [$nvme, 'disconnect', '--nqn', $scfg->{subsysnqn}], diff --git a/src/test/zfsnvme_test.pm b/src/test/zfsnvme_test.pm index 3e9e04b..edb8b38 100644 --- a/src/test/zfsnvme_test.pm +++ b/src/test/zfsnvme_test.pm @@ -11,6 +11,8 @@ use PVE::Storage::ZFSPlugin; use Test::MockModule; use Test::More; +my $nvme_mock = Test::MockModule->new('PVE::Storage::ZFSNVMePlugin'); + is( PVE::Storage::ZFSNVMePlugin::verify_nvme_nqn( 'nqn.2014-08.org.nvmexpress:uuid:12345678-1234-1234-1234-123456789abc', @@ -86,6 +88,16 @@ eval { }; like($@, qr/one interface for each/, 'portal and host-interface counts must match'); +$nvme_mock->redefine(_local_iface_exists => sub { return $_[0] eq 'ens20' }); +eval { + PVE::Storage::ZFSNVMePlugin::_validate_local_ifaces([ + { host_iface => 'ens20' }, + { host_iface => 'ens21' }, + ]); +}; +like($@, qr/host interface 'ens21' does not exist/, 'missing local interface fails preflight'); +$nvme_mock->redefine(_local_iface_exists => sub { return 1 }); + eval { PVE::Storage::ZFSNVMePlugin::_assert_unique_target( 'new-storage', @@ -161,7 +173,6 @@ is( eval { PVE::Storage::ZFSNVMePlugin::_validate_secret('plaintext') }; like($@, qr/invalid NVMe DH-HMAC-CHAP/, 'rejects a plaintext secret'); -my $nvme_mock = Test::MockModule->new('PVE::Storage::ZFSNVMePlugin'); $nvme_mock->redefine(zfs_get_lu_name => sub { return '12345678-1234-1234-1234-123456789abc' }); my $scfg = {}; @@ -262,6 +273,21 @@ is_deeply( 'volume listing requires a local or received ownership property', ); +$nvme_mock->redefine( + zfs_list_zvol => sub { return { 'vm-100-disk-0' => 1, 'base-200-disk-0' => 1 } }, +); +eval { + PVE::Storage::ZFSNVMePlugin->on_delete_hook( + 'nvmetest', + { subsysnqn => 'nqn.2026-07.example:test' }, + ); +}; +like( + $@, + qr/refusing to remove.*base-200-disk-0, vm-100-disk-0/s, + 'storage removal requires an empty owned dataset on every cluster node', +); + eval { PVE::Storage::ZFSNVMePlugin->volume_resize( {}, 'nvmetest', 'vm-100-disk-0', 2 * 1024 * 1024 * 1024, 1, undef, @@ -273,6 +299,22 @@ like( 'online resize is rejected before mutating the backend', ); +$nvme_mock->redefine( + _namespace_openers => sub { return ['qemu-system-x86_64 (PID 123, /dev/nvme0n1)'] }, +); +eval { + PVE::Storage::ZFSNVMePlugin->deactivate_storage( + 'nvmetest', + { subsysnqn => 'nqn.2026-07.example:test' }, + ); +}; +like( + $@, + qr/refusing to disconnect.*namespace in use by qemu-system-x86_64/s, + 'storage deactivation refuses to remove a namespace opened by a VM', +); +$nvme_mock->redefine(_namespace_openers => sub { return [] }); + my $update_key = 'DHHC-1:01:dXBkYXRlLXRlc3Qta2V5:'; $nvme_mock->redefine(file_read_firstline => sub { return $update_key }); my $zfs_parent_mock = Test::MockModule->new('PVE::Storage::ZFSPlugin'); -- 2.54.0.windows.1