From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 3C7EE1FF0ED for ; Fri, 31 Jul 2026 12:22:47 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id A9CDA21602; Fri, 31 Jul 2026 12:22:04 +0200 (CEST) From: Dietmar Maurer To: pve-devel@lists.proxmox.com Subject: [RFC pve-storage 04/27] disks: list: add include-remote parameter Date: Fri, 31 Jul 2026 12:21:33 +0200 Message-ID: <20260731102156.3947857-5-dietmar@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260731102156.3947857-1-dietmar@proxmox.com> References: <20260731102156.3947857-1-dietmar@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 1 AWL -0.656 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) KAM_LAZY_DOMAIN_SECURITY 1 Sending domain does not have any anti-forgery methods RDNS_NONE 1.274 Delivered to internal network by a host with no rDNS SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_NONE 0.001 SPF: sender does not publish an SPF Record Message-ID-Hash: B2ZZG5UQPBJVRXTFSMAUGIHMHGSURNDB X-Message-ID-Hash: B2ZZG5UQPBJVRXTFSMAUGIHMHGSURNDB X-MailFrom: dietmar@zilli.proxmox.com 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 X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Generic disk selectors are primarily used for node-local storage. Offering devices attached through an identifiable remote transport there makes accidental reuse too easy, regardless of whether the administrator assigned the LUN exclusively or shared it. Require an explicit opt-in for FC, FCoE, iSCSI, and remote NVMe attachments. SAS remains visible because its transport alone does not identify a remote attachment, while NVMe loopback is local. Signed-off-by: Dietmar Maurer --- src/PVE/API2/Disks.pm | 21 +++++- src/PVE/Diskmanage.pm | 9 +++ src/test/Makefile | 5 +- src/test/disk_api_test.pm | 113 +++++++++++++++++++++++++++++++++ src/test/run_disk_api_tests.pl | 11 ++++ 5 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 src/test/disk_api_test.pm create mode 100755 src/test/run_disk_api_tests.pl diff --git a/src/PVE/API2/Disks.pm b/src/PVE/API2/Disks.pm index e64a391..673bbcb 100644 --- a/src/PVE/API2/Disks.pm +++ b/src/PVE/API2/Disks.pm @@ -82,7 +82,7 @@ __PACKAGE__->register_method({ name => 'list', path => 'list', method => 'GET', - description => "List local disks.", + description => "List disks attached to the node.", protected => 1, proxyto => 'node', permissions => { @@ -98,6 +98,14 @@ __PACKAGE__->register_method({ optional => 1, default => 0, }, + 'include-remote' => { + description => "Also include devices attached through a remote storage" + . " transport (for example FC, iSCSI, or NVMe over TCP), which are" + . " excluded by default.", + type => 'boolean', + optional => 1, + default => 0, + }, skipsmart => { description => "Skip smart checks.", type => 'boolean', @@ -155,16 +163,27 @@ __PACKAGE__->register_method({ my $skipsmart = $param->{skipsmart} // 0; my $include_partitions = $param->{'include-partitions'} // 0; + my $include_remote = $param->{'include-remote'} // 0; my $disks = PVE::Diskmanage::get_disks( undef, $skipsmart, $include_partitions, ); + my $by_devpath = {}; + $by_devpath->{ $disks->{$_}->{devpath} } = $disks->{$_} for keys %$disks; + my $type = $param->{type} // ''; my $result = []; foreach my $disk (sort keys %$disks) { my $entry = $disks->{$disk}; + if (!$include_remote) { + # partitions inherit the visibility of the disk they reside on + my $device = $entry; + $device = $by_devpath->{ $entry->{parent} } // $entry + if defined($entry->{parent}); + next if PVE::Diskmanage::is_remote_transport($device->{transport}); + } if ($type eq 'journal_disks') { next if $entry->{osdid} >= 0; if (my $usage = $entry->{used}) { diff --git a/src/PVE/Diskmanage.pm b/src/PVE/Diskmanage.pm index b006ec3..7ab8119 100644 --- a/src/PVE/Diskmanage.pm +++ b/src/PVE/Diskmanage.pm @@ -505,6 +505,15 @@ sub get_nvme_transport { return $transport; } +# Whether a transport belongs to the set of remote storage transports excluded +# from the disk list by default. SAS is not included because its transport does +# not distinguish local from remote attachments. +sub is_remote_transport { + my ($transport) = @_; + + return ($transport // '') =~ m/^(?:fc|fcoe|iscsi|nvme-(?:fc|tcp|rdma))$/; +} + my sub is_ssdlike { my ($type) = @_; return $type eq 'ssd' || $type eq 'nvme'; diff --git a/src/test/Makefile b/src/test/Makefile index ee025bc..4504743 100644 --- a/src/test/Makefile +++ b/src/test/Makefile @@ -1,6 +1,6 @@ all: test -test: test_zfspoolplugin test_lvmplugin test_disklist test_bwlimit test_plugin test_ovf test_volume_access +test: test_zfspoolplugin test_lvmplugin test_disklist test_disk_api test_bwlimit test_plugin test_ovf test_volume_access test_zfspoolplugin: run_test_zfspoolplugin.pl ./run_test_zfspoolplugin.pl @@ -11,6 +11,9 @@ test_lvmplugin: run_test_lvmplugin.pl test_disklist: run_disk_tests.pl ./run_disk_tests.pl +test_disk_api: run_disk_api_tests.pl + ./run_disk_api_tests.pl + test_bwlimit: run_bwlimit_tests.pl ./run_bwlimit_tests.pl diff --git a/src/test/disk_api_test.pm b/src/test/disk_api_test.pm new file mode 100644 index 0000000..c7735c0 --- /dev/null +++ b/src/test/disk_api_test.pm @@ -0,0 +1,113 @@ +package PVE::API2::Disks::Test; + +use strict; +use warnings; + +use lib qw(..); + +use PVE::API2::Disks; +use PVE::Diskmanage; + +use Test::MockModule; +use Test::More; + +my @transport_cases = ( + ['none', undef, 0], + ['unknown', 'unknown', 0], + ['ata', 'ata', 0], + ['sata', 'sata', 0], + ['sas', 'sas', 0], + ['spi', 'spi', 0], + ['sbp', 'sbp', 0], + ['usb', 'usb', 0], + ['fc', 'fc', 1], + ['fcoe', 'fcoe', 1], + ['iscsi', 'iscsi', 1], + ['nvme', 'nvme', 0], + ['virtio', 'virtio', 0], + ['nvme-fc', 'nvme-fc', 1], + ['nvme-tcp', 'nvme-tcp', 1], + ['nvme-rdma', 'nvme-rdma', 1], + ['nvme-loop', 'nvme-loop', 0], +); + +my $disks = {}; +my (@all, @local); +for my $case (@transport_cases) { + my ($name, $transport, $remote) = $case->@*; + my $devpath = "/dev/test-$name"; + my $entry = { + devpath => $devpath, + gpt => 0, + osdid => -1, + }; + $entry->{transport} = $transport if defined($transport); + $disks->{$name} = $entry; + push @all, $devpath; + push @local, $devpath if !$remote; +} + +my $diskmanage_module = Test::MockModule->new('PVE::Diskmanage', no_auto => 1); +$diskmanage_module->mock('get_disks' => sub { return $disks }); + +my $list_handler = PVE::API2::Disks->map_method_by_name('list')->{code}; +my $list_devpaths = sub { + my ($param) = @_; + my $result = $list_handler->({ node => 'test-node', skipsmart => 1, $param->%* }); + return [sort map { $_->{devpath} } $result->@*]; +}; + +is_deeply( + $list_devpaths->({}), [sort @local], 'the default disk list excludes remote transports', +); + +is_deeply( + $list_devpaths->({ type => 'unused' }), + [sort @local], + 'unused disk selection excludes remote transports', +); + +is_deeply( + $list_devpaths->({ 'include-remote' => 1 }), + [sort @all], + 'include-remote returns all transports', +); + +$diskmanage_module->mock( + 'get_disks' => sub { + return { + 'fc-disk' => { + devpath => '/dev/fc-disk', + gpt => 0, + osdid => -1, + transport => 'fc', + }, + 'fc-partition' => { + devpath => '/dev/fc-disk1', + gpt => 0, + osdid => -1, + parent => '/dev/fc-disk', + }, + 'loop-disk' => { + devpath => '/dev/loop-disk', + gpt => 0, + osdid => -1, + transport => 'nvme-loop', + }, + 'loop-partition' => { + devpath => '/dev/loop-disk1', + gpt => 0, + osdid => -1, + parent => '/dev/loop-disk', + }, + }; + }, +); + +is_deeply( + $list_devpaths->({ 'include-partitions' => 1 }), + ['/dev/loop-disk', '/dev/loop-disk1'], + 'partitions inherit the visibility of their parent disk', +); + +done_testing(); diff --git a/src/test/run_disk_api_tests.pl b/src/test/run_disk_api_tests.pl new file mode 100755 index 0000000..75fc98c --- /dev/null +++ b/src/test/run_disk_api_tests.pl @@ -0,0 +1,11 @@ +#!/usr/bin/perl + +use strict; +use warnings; + +use TAP::Harness; + +my $harness = TAP::Harness->new({ verbosity => -2 }); +my $res = $harness->runtests("disk_api_test.pm"); + +exit -1 if !$res || $res->{failed} || $res->{parse_errors}; -- 2.47.3