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 01CD81FF0AF for ; Fri, 06 Nov 2026 01:51:49 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id D6CDD217E0; Fri, 06 Nov 2026 01:50:58 +0100 (CET) From: Thomas Lamprecht To: pve-devel@lists.proxmox.com Subject: [PATCH v3 storage 15/76] disks: list: add include-remote parameter Date: Fri, 6 Nov 2026 00:30:50 +0100 Message-ID: <20261105233336.2178064-16-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: 1793926178255 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.332 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) 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 Message-ID-Hash: MCXEMA4P3BIRPMSUIU5UMQWQKOWT2DWF X-Message-ID-Hash: MCXEMA4P3BIRPMSUIU5UMQWQKOWT2DWF X-MailFrom: t.lamprecht@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: From: Dietmar Maurer 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 [TL: drop redundant comments] Signed-off-by: Thomas Lamprecht --- src/PVE/API2/Disks.pm | 20 +++++- src/PVE/Diskmanage.pm | 7 ++ src/test/Makefile | 5 +- src/test/disk_api_test.pm | 113 +++++++++++++++++++++++++++++++++ src/test/run_disk_api_tests.pl | 11 ++++ 5 files changed, 154 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 f448f8bd..ada836b6 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,26 @@ __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) { + 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 bc81e854..6ea9f2e3 100644 --- a/src/PVE/Diskmanage.pm +++ b/src/PVE/Diskmanage.pm @@ -523,6 +523,13 @@ sub get_nvme_transport { return $transport; } +# SAS does not distinguish local from remote attachments, so it is not classified as remote. +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 415d62bf..ea11d399 100644 --- a/src/test/Makefile +++ b/src/test/Makefile @@ -1,6 +1,6 @@ all: test -test: test_iscsi test_zfspoolplugin test_lvmplugin test_lvm_create test_lvm_lock_retry test_lvm_base_volume test_lvm_hints test_lvm_status test_status_command test_disklist test_bwlimit test_plugin test_ovf test_volume_access test_san_libraries +test: test_iscsi test_zfspoolplugin test_lvmplugin test_lvm_create test_lvm_lock_retry test_lvm_base_volume test_lvm_hints test_lvm_status test_status_command test_disklist test_disk_api test_bwlimit test_plugin test_ovf test_volume_access test_san_libraries test_zfspoolplugin: run_test_zfspoolplugin.pl ./run_test_zfspoolplugin.pl @@ -32,6 +32,9 @@ test_status_command: run_status_command_tests.pl run_status_command_contention_t 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 00000000..c7735c0f --- /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 00000000..75fc98c6 --- /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