[pve-devel] [PATCH storage v2 1/3] refactor disk/storage checks for Disk API

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
changes from v1:
* new in v2
* only refactors the checks according to thomas feedback

 PVE/API2/Disks/Directory.pm |  9 ++-------
 PVE/API2/Disks/LVM.pm       |  2 +-
 PVE/API2/Disks/LVMThin.pm   |  9 ++-------
 PVE/API2/Disks/ZFS.pm       |  8 ++------
 PVE/Diskmanage.pm           |  8 ++++++++
 PVE/Storage.pm              | 12 ++++++++++++
 6 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/PVE/API2/Disks/Directory.pm b/PVE/API2/Disks/Directory.pm
index 9d27762..f076ee7 100644
--- a/PVE/API2/Disks/Directory.pm
+++ b/PVE/API2/Disks/Directory.pm
@@ -201,13 +201,8 @@ __PACKAGE__->register_method ({
 	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";
-	}
+	PVE::Diskmanage::check_unused($dev);
+	PVE::Storage::check_available($name);
 
 	my $worker = sub {
 	    my $path = "/mnt/pve/$name";
diff --git a/PVE/API2/Disks/LVM.pm b/PVE/API2/Disks/LVM.pm
index 19fba07..009ee0d 100644
--- a/PVE/API2/Disks/LVM.pm
+++ b/PVE/API2/Disks/LVM.pm
@@ -149,7 +149,7 @@ __PACKAGE__->register_method ({
 	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);
+	PVE::Diskmanage::check_unused($dev);
 
 	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..62d3b61 100644
--- a/PVE/API2/Disks/LVMThin.pm
+++ b/PVE/API2/Disks/LVMThin.pm
@@ -103,13 +103,8 @@ __PACKAGE__->register_method ({
 	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";
-	}
+	PVE::Diskmanage::check_unused($dev);
+	PVE::Storage::check_available($name);
 
 	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..19ccedf 100644
--- a/PVE/API2/Disks/ZFS.pm
+++ b/PVE/API2/Disks/ZFS.pm
@@ -341,14 +341,10 @@ __PACKAGE__->register_method ({
 
 	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);
+	    PVE::Diskmanage::check_unused($dev);
 	}
 
-	my $cfg = PVE::Storage::config();
-
-	if (my $scfg = PVE::Storage::storage_config($cfg, $name, 1)) {
-	    die "storage ID '$name' already defined\n";
-	}
+	PVE::Storage::check_available($name);
 
 	my $numdisks = scalar(@$devs);
 	my $mindisks = {
diff --git a/PVE/Diskmanage.pm b/PVE/Diskmanage.pm
index 1938fa8..5a5fc87 100644
--- a/PVE/Diskmanage.pm
+++ b/PVE/Diskmanage.pm
@@ -602,4 +602,12 @@ sub locked_disk_action {
     return $res;
 }
 
+sub check_unused {
+    my ($dev) = @_;
+
+    die "device $dev is already in use\n" if disk_is_used($dev);
+
+    return undef;
+}
+
 1;
diff --git a/PVE/Storage.pm b/PVE/Storage.pm
index f9732fe..81f9b67 100755
--- a/PVE/Storage.pm
+++ b/PVE/Storage.pm
@@ -1647,4 +1647,16 @@ sub get_bandwidth_limit {
     return $override;
 }
 
+# checks if the storage id is available and dies if not
+sub check_available {
+    my ($id) = @_;
+
+    my $cfg = config();
+    if (my $scfg = storage_config($cfg, $id, 1)) {
+	die "storage ID '$id' already defined\n";
+    }
+
+    return undef;
+}
+
 1;
-- 
2.11.0