[pve-devel] [PATCH storage] fix #1929: refactor requirements check for disks

[Dominik Csapak]

and check correctly that the storage exists only when the user
want to create a storage
this is useful if you want to create the same storage on
multiple nodes

Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
 PVE/API2/Disks/Directory.pm |  9 +--------
 PVE/API2/Disks/LVM.pm       |  3 +--
 PVE/API2/Disks/LVMThin.pm   |  9 +--------
 PVE/API2/Disks/ZFS.pm       | 11 +----------
 PVE/Diskmanage.pm           | 23 +++++++++++++++++++++++
 5 files changed, 27 insertions(+), 28 deletions(-)

diff --git a/PVE/API2/Disks/Directory.pm b/PVE/API2/Disks/Directory.pm
index 9d27762..0a3afdb 100644
--- a/PVE/API2/Disks/Directory.pm
+++ b/PVE/API2/Disks/Directory.pm
@@ -200,14 +200,7 @@ __PACKAGE__->register_method ({
 	my $node = $param->{node};
 	my $type = $param->{filesystem} // 'ext4';
 
-	$dev = PVE::Diskmanage::verify_blockdev_path($dev);
-	die "device $dev is already in use\n" if PVE::Diskmanage::disk_is_used($dev);
-
-	my $cfg = PVE::Storage::config();
-
-	if (my $scfg = PVE::Storage::storage_config($cfg, $name, 1)) {
-	    die "storage ID '$name' already defined\n";
-	}
+	$dev = PVE::Diskmanage::assert_disk_requirements($name, $dev, $param->{add_storage});
 
 	my $worker = sub {
 	    my $path = "/mnt/pve/$name";
diff --git a/PVE/API2/Disks/LVM.pm b/PVE/API2/Disks/LVM.pm
index 19fba07..9c66705 100644
--- a/PVE/API2/Disks/LVM.pm
+++ b/PVE/API2/Disks/LVM.pm
@@ -148,8 +148,7 @@ __PACKAGE__->register_method ({
 	my $dev = $param->{device};
 	my $node = $param->{node};
 
-	$dev = PVE::Diskmanage::verify_blockdev_path($dev);
-	die "device $dev is already in use\n" if PVE::Diskmanage::disk_is_used($dev);
+	$dev = PVE::Diskmanage::assert_disk_requirements($name, $dev, $param->{add_storage});
 
 	my $worker = sub {
 	    PVE::Diskmanage::locked_disk_action(sub {
diff --git a/PVE/API2/Disks/LVMThin.pm b/PVE/API2/Disks/LVMThin.pm
index 5b72c8c..46630e3 100644
--- a/PVE/API2/Disks/LVMThin.pm
+++ b/PVE/API2/Disks/LVMThin.pm
@@ -102,14 +102,7 @@ __PACKAGE__->register_method ({
 	my $dev = $param->{device};
 	my $node = $param->{node};
 
-	$dev = PVE::Diskmanage::verify_blockdev_path($dev);
-	die "device $dev is already in use\n" if PVE::Diskmanage::disk_is_used($dev);
-
-	my $cfg = PVE::Storage::config();
-
-	if (my $scfg = PVE::Storage::storage_config($cfg, $name, 1)) {
-	    die "storage ID '$name' already defined\n";
-	}
+	$dev = PVE::Diskmanage::assert_disk_requirements($name, $dev, $param->{add_storage});
 
 	my $worker = sub {
 	    PVE::Diskmanage::locked_disk_action(sub {
diff --git a/PVE/API2/Disks/ZFS.pm b/PVE/API2/Disks/ZFS.pm
index 3c36ef9..b7329f6 100644
--- a/PVE/API2/Disks/ZFS.pm
+++ b/PVE/API2/Disks/ZFS.pm
@@ -339,16 +339,7 @@ __PACKAGE__->register_method ({
 	my $ashift = $param->{ashift} // 12;
 	my $compression = $param->{compression} // 'on';
 
-	foreach my $dev (@$devs) {
-	    $dev = PVE::Diskmanage::verify_blockdev_path($dev);
-	    die "device $dev is already in use\n" if PVE::Diskmanage::disk_is_used($dev);
-	}
-
-	my $cfg = PVE::Storage::config();
-
-	if (my $scfg = PVE::Storage::storage_config($cfg, $name, 1)) {
-	    die "storage ID '$name' already defined\n";
-	}
+	$devs = PVE::Diskmanage::assert_disk_requirements($name, $devs, $param->{add_storage});
 
 	my $numdisks = scalar(@$devs);
 	my $mindisks = {
diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index 1938fa8..7cc0d0d 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -602,4 +602,27 @@ sub locked_disk_action {
     return $res;
 }
 
+sub assert_disk_requirements {
+    my ($name, $device, $addstorage) = @_;
+
+    if (ref($device) eq 'ARRAY') {
+	foreach my $dev (@$device) {
+	    $dev = verify_blockdev_path($dev);
+	    die "device $dev is already in use\n" if disk_is_used($dev);
+	}
+    } elsif (!ref($device)) {
+	$device = verify_blockdev_path($device);
+	die "device $device is already in use\n" if disk_is_used($device);
+    }
+
+    if ($addstorage) {
+	my $cfg = PVE::Storage::config();
+	if (my $scfg = PVE::Storage::storage_config($cfg, $name, 1)) {
+	    die "storage ID '$name' already defined\n";
+	}
+    }
+
+    return $device;
+}
+
 1;
-- 
2.11.0