[RFC pve-storage 09/27] disks: lvm: allow creating volume groups on multipath devices

[Dietmar Maurer <dietmar@proxmox.com>]

The device checks resolved the mapper symlink to the dm device, which
get_disks does not know, so creating a volume group on a multipath
mapped SAN LUN was rejected with 'not a valid local disk'. Accept
multipath devices that are not in use, so the storage wizard can
initialize a shared LVM volume group directly on a multipath LUN.

Signed-off-by: Dietmar Maurer <dietmar@proxmox.com>
---
 src/PVE/API2/Disks/LVM.pm |  8 +++++---
 src/PVE/Diskmanage.pm     | 27 +++++++++++++++++++++++----
 src/test/disklist_test.pm | 19 +++++++++++++++++++
 3 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/src/PVE/API2/Disks/LVM.pm b/src/PVE/API2/Disks/LVM.pm
index 9f8f951..35051fd 100644
--- a/src/PVE/API2/Disks/LVM.pm
+++ b/src/PVE/API2/Disks/LVM.pm
@@ -131,7 +131,8 @@ __PACKAGE__->register_method({
             name => get_standard_option('pve-storage-id'),
             device => {
                 type => 'string',
-                description => 'The block device you want to create the volume group on',
+                description => 'The block device you want to create the volume group on.'
+                    . ' Multipath mapped devices are supported.',
             },
             add_storage => {
                 description => "Configure storage using the Volume Group",
@@ -153,7 +154,8 @@ __PACKAGE__->register_method({
         my $node = $param->{node};
 
         $dev = PVE::Diskmanage::verify_blockdev_path($dev);
-        PVE::Diskmanage::assert_disk_unused($dev);
+
+        PVE::Diskmanage::assert_disk_unused($dev, 1);
 
         my $storage_params = {
             type => 'lvm',
@@ -176,7 +178,7 @@ __PACKAGE__->register_method({
 
         my $worker = sub {
             PVE::Diskmanage::locked_disk_action(sub {
-                PVE::Diskmanage::assert_disk_unused($dev);
+                PVE::Diskmanage::assert_disk_unused($dev, 1);
                 die "volume group with name '${name}' already exists on node '${node}'\n"
                     if PVE::Storage::LVMPlugin::lvm_vgs()->{$name};
 
diff --git a/src/PVE/Diskmanage.pm b/src/PVE/Diskmanage.pm
index 5ac04dd..ab5db06 100644
--- a/src/PVE/Diskmanage.pm
+++ b/src/PVE/Diskmanage.pm
@@ -74,14 +74,22 @@ sub init_disk {
 }
 
 sub disk_is_used {
-    my ($disk) = @_;
+    my ($disk, $allow_multipath) = @_;
 
     my $dev = $disk;
     $dev =~ s|^/dev/||;
 
     my $disklist = get_disks($dev, 1, 1);
 
-    die "'$disk' is not a valid local disk\n" if !defined($disklist->{$dev});
+    if (!defined($disklist->{$dev})) {
+        if ($allow_multipath) {
+            my $multipath = lookup_multipath_map($disk);
+            return defined($multipath->{used}) ? 1 : 0 if defined($multipath);
+        }
+
+        die "'$disk' is not a valid local disk\n";
+    }
+
     return 1 if $disklist->{$dev}->{used};
 
     return 0;
@@ -904,6 +912,17 @@ sub get_multipath_disks {
     return $disklist;
 }
 
+# Returns the multipath device entry if the given path refers to a multipath mapped device.
+sub lookup_multipath_map {
+    my ($devpath) = @_;
+
+    my $path = abs_path($devpath) // $devpath;
+    my $dev = strip_dev($path);
+    return undef if $dev !~ m/^dm-\d+$/;
+
+    return get_multipath_disks()->{$dev};
+}
+
 sub get_partnum {
     my ($part_path) = @_;
 
@@ -958,8 +977,8 @@ sub locked_disk_action {
 }
 
 sub assert_disk_unused {
-    my ($dev) = @_;
-    die "device '$dev' is already in use\n" if disk_is_used($dev);
+    my ($dev, $allow_multipath) = @_;
+    die "device '$dev' is already in use\n" if disk_is_used($dev, $allow_multipath);
     return;
 }
 
diff --git a/src/test/disklist_test.pm b/src/test/disklist_test.pm
index dc84510..d2ea3b2 100644
--- a/src/test/disklist_test.pm
+++ b/src/test/disklist_test.pm
@@ -236,6 +236,25 @@ sub test_disk_list {
             print Dumper($multipath) if $print;
             $testcount++;
             is_deeply($multipath, $expected_multipath, 'multipath list should be the same');
+
+            eval { PVE::Diskmanage::assert_disk_unused('/dev/dm-0'); };
+            $testcount++;
+            like($@, qr/not a valid local disk/,
+                'multipath devices require an explicit opt-in');
+
+            eval { PVE::Diskmanage::assert_disk_unused('/dev/mapper/mpatha', 1); };
+            $testcount++;
+            is($@, '', 'unused multipath alias should pass');
+
+            for my $dev (qw(dm-1 dm-2 dm-3 dm-4)) {
+                eval { PVE::Diskmanage::assert_disk_unused("/dev/$dev", 1); };
+                $testcount++;
+                like($@, qr/already in use/, "used multipath device $dev should fail");
+            }
+
+            eval { PVE::Diskmanage::assert_disk_unused('/dev/dm-99', 1); };
+            $testcount++;
+            like($@, qr/not a valid local disk/, 'unknown device mapper device should fail');
         }
 
         done_testing($testcount);
-- 
2.47.3