[pve-devel] [PATCH storage v2 3/3] refactor finding next diskname for all plugins

Stoiko Ivanov s.ivanov at proxmox.com
Fri Sep 7 15:08:42 CEST 2018


Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
---
 PVE/Storage/DRBDPlugin.pm      | 15 ++-------------
 PVE/Storage/GlusterfsPlugin.pm | 27 +++++++++------------------
 PVE/Storage/LVMPlugin.pm       | 22 ++++------------------
 PVE/Storage/LvmThinPlugin.pm   |  4 ++--
 PVE/Storage/Plugin.pm          | 25 +++++++++----------------
 PVE/Storage/RBDPlugin.pm       | 20 +++++---------------
 PVE/Storage/SheepdogPlugin.pm  | 21 ++++-----------------
 PVE/Storage/ZFSPoolPlugin.pm   | 19 ++-----------------
 8 files changed, 37 insertions(+), 116 deletions(-)

diff --git a/PVE/Storage/DRBDPlugin.pm b/PVE/Storage/DRBDPlugin.pm
index e0c8260..dbae4d1 100644
--- a/PVE/Storage/DRBDPlugin.pm
+++ b/PVE/Storage/DRBDPlugin.pm
@@ -177,21 +177,10 @@ sub alloc_image {
 
     my $hdl = connect_drbdmanage_service();
     my $volumes = drbd_list_volumes($hdl);
+    my $disk_list = [ keys %$volumes ];
 
     die "volume '$name' already exists\n" if defined($name) && $volumes->{$name};
-    
-    if (!defined($name)) {	
-	for (my $i = 1; $i < 100; $i++) {
-	    my $tn = "vm-$vmid-disk-$i";
-	    if (!defined ($volumes->{$tn})) {
-		$name = $tn;
-		last;
-	    }
-	}
-    }
-
-    die "unable to allocate an image name for VM $vmid in storage '$storeid'\n"
-	if !defined($name);
+    $name //= PVE::Storage::Plugin::get_next_vm_diskname($disk_list, $storeid, $vmid, undef, $scfg);
 
     my ($rc, $res) = $hdl->create_resource($name, {});
     check_drbd_res($rc);
diff --git a/PVE/Storage/GlusterfsPlugin.pm b/PVE/Storage/GlusterfsPlugin.pm
index f3aa030..b6ede44 100644
--- a/PVE/Storage/GlusterfsPlugin.pm
+++ b/PVE/Storage/GlusterfsPlugin.pm
@@ -162,23 +162,14 @@ sub parse_name_dir {
 }
 
 my $find_free_diskname = sub {
-    my ($imgdir, $vmid, $fmt) = @_;
-
-    my $disk_ids = {};
-    PVE::Tools::dir_glob_foreach($imgdir,
-                                 qr!(vm|base)-$vmid-disk-(\d+)\..*!,
-                                 sub {
-                                     my ($fn, $type, $disk) = @_;
-                                     $disk_ids->{$disk} = 1;
-                                 });
-
-    for (my $i = 1; $i < 100; $i++) {
-        if (!$disk_ids->{$i}) {
-            return "vm-$vmid-disk-$i.$fmt";
-        }
-    }
+    my ($imgdir, $vmid, $fmt, $scfg) = @_;
+
+    my $disk_list = [];
+
+    my $dh = IO::Dir->new ($imgdir);
+    @$disk_list = $dh->read() if defined($dh);
 
-    die "unable to allocate a new image name for VM $vmid in '$imgdir'\n";
+    return PVE::Storage::Plugin::get_next_vm_diskname($disk_list, $imgdir, $vmid, $fmt, $scfg, 1);
 };
 
 sub path {
@@ -235,7 +226,7 @@ sub clone_image {
 
     mkpath $imagedir;
 
-    my $name = &$find_free_diskname($imagedir, $vmid, "qcow2");
+    my $name = $find_free_diskname->($imagedir, $vmid, "qcow2", $scfg);
 
     warn "clone $volname: $vtype, $name, $vmid to $name (base=../$basevmid/$basename)\n";
 
@@ -263,7 +254,7 @@ sub alloc_image {
 
     mkpath $imagedir;
 
-    $name = &$find_free_diskname($imagedir, $vmid, $fmt) if !$name;
+    $name = $find_free_diskname->($imagedir, $vmid, $fmt, $scfg) if !$name;
 
     my (undef, $tmpfmt) = parse_name_dir($name);
 
diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
index 19bac55..f4a2566 100644
--- a/PVE/Storage/LVMPlugin.pm
+++ b/PVE/Storage/LVMPlugin.pm
@@ -289,25 +289,11 @@ sub clone_image {
 }
 
 sub lvm_find_free_diskname {
-    my ($lvs, $vg, $storeid, $vmid) = @_;
+    my ($lvs, $vg, $storeid, $vmid, $scfg) = @_;
 
-    my $name;
-
-    my $disk_ids = {};
-    my @vols = keys(%{$lvs->{$vg}});
-
-    foreach my $vol (@vols) {
-	if ($vol =~ m/(vm|base)-\Q$vmid\E-disk-(\d+)/){
-	    $disk_ids->{$2} = 1;
-	}
-    }
-
-    for (my $i = 1; $i < 100; $i++) {
-	return "vm-$vmid-disk-$i" if !$disk_ids->{$i};
-    }
-
-    die "unable to allocate an image name for ID $vmid in storage '$storeid'\n";
+    my $disk_list = [ keys %{$lvs->{$vg}} ];
 
+    return PVE::Storage::Plugin::get_next_vm_diskname($disk_list, $storeid, $vmid, undef, $scfg);
 }
 
 sub alloc_image {
@@ -328,7 +314,7 @@ sub alloc_image {
 
     die "not enough free space ($free < $size)\n" if $free < $size;
 
-    $name = lvm_find_free_diskname(lvm_list_volumes($vg), $vg, $storeid, $vmid)
+    $name = lvm_find_free_diskname(lvm_list_volumes($vg), $vg, $storeid, $vmid, $scfg)
 	if !$name;
 
     my $cmd = ['/sbin/lvcreate', '-aly', '--addtag', "pve-vm-$vmid", '--size', "${size}k", '--name', $name, $vg];
diff --git a/PVE/Storage/LvmThinPlugin.pm b/PVE/Storage/LvmThinPlugin.pm
index d08eb77..122fb37 100644
--- a/PVE/Storage/LvmThinPlugin.pm
+++ b/PVE/Storage/LvmThinPlugin.pm
@@ -97,7 +97,7 @@ sub alloc_image {
 
     my $lvs = PVE::Storage::LVMPlugin::lvm_list_volumes($vg);
 
-    $name = PVE::Storage::LVMPlugin::lvm_find_free_diskname($lvs, $vg, $storeid, $vmid)
+    $name = PVE::Storage::LVMPlugin::lvm_find_free_diskname($lvs, $vg, $storeid, $vmid, $scfg)
 	if !$name;
 
     my $cmd = ['/sbin/lvcreate', '-aly', '-V', "${size}k", '--name', $name,
@@ -270,7 +270,7 @@ sub clone_image {
 
     my $lvs = PVE::Storage::LVMPlugin::lvm_list_volumes($vg);
 
-    my $name =  PVE::Storage::LVMPlugin::lvm_find_free_diskname($lvs, $vg, $storeid, $vmid);
+    my $name =  PVE::Storage::LVMPlugin::lvm_find_free_diskname($lvs, $vg, $storeid, $vmid, $scfg);
 
     my $cmd = ['/sbin/lvcreate', '-n', $name, '-prw', '-kn', '-s', $lv];
     run_command($cmd, errmsg => "clone image '$lv' error");
diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm
index 7516f4e..58209b7 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -569,23 +569,16 @@ sub get_next_vm_diskname {
 }
 
 my $find_free_diskname = sub {
-    my ($imgdir, $vmid, $fmt) = @_;
+    my ($imgdir, $vmid, $fmt, $scfg) = @_;
 
-    my $disk_ids = {};
-    PVE::Tools::dir_glob_foreach($imgdir,
-				 qr!(vm|base)-$vmid-disk-(\d+)\..*!,
-				 sub {
-				     my ($fn, $type, $disk) = @_;
-				     $disk_ids->{$disk} = 1;
-				 });
-
-    for (my $i = 1; $i < 100; $i++) {
-	if (!$disk_ids->{$i}) {
-	    return "vm-$vmid-disk-$i.$fmt";
-	}
+    my $disk_list = [];
+
+    if (defined(my $dh = IO::Dir->new($imgdir))) {
+	@$disk_list = $dh->read();
+	$dh->close();
     }
 
-    die "unable to allocate a new image name for VM $vmid in '$imgdir'\n";
+    return  get_next_vm_diskname($disk_list, $imgdir, $vmid, $fmt, $scfg, 1);
 };
 
 sub clone_image {
@@ -610,7 +603,7 @@ sub clone_image {
 
     mkpath $imagedir;
 
-    my $name = &$find_free_diskname($imagedir, $vmid, "qcow2");
+    my $name = $find_free_diskname->($imagedir, $vmid, "qcow2", $scfg);
 
     warn "clone $volname: $vtype, $name, $vmid to $name (base=../$basevmid/$basename)\n";
 
@@ -642,7 +635,7 @@ sub alloc_image {
 
     mkpath $imagedir;
 
-    $name = &$find_free_diskname($imagedir, $vmid, $fmt) if !$name;
+    $name = $find_free_diskname->($imagedir, $vmid, $fmt, $scfg) if !$name;
 
     my (undef, $tmpfmt) = parse_name_dir($name);
 
diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
index bd8c28d..84b1abd 100644
--- a/PVE/Storage/RBDPlugin.pm
+++ b/PVE/Storage/RBDPlugin.pm
@@ -321,14 +321,11 @@ my $find_free_diskname = sub {
     my ($storeid, $scfg, $vmid) = @_;
 
     my $cmd = &$rbd_cmd($scfg, $storeid, 'ls');
-    my $disk_ids = {};
+    my $disk_list = [];
 
     my $parser = sub {
 	my $line = shift;
-
-	if ($line =~  m/^(vm|base)-\Q$vmid\E+-disk-(\d+)$/) {
-	    $disk_ids->{$2} = 1;
-	}
+	push @$disk_list, $line;
     };
 
     eval {
@@ -338,14 +335,7 @@ my $find_free_diskname = sub {
 
     die $err if $err && $err !~ m/doesn't contain rbd images/;
 
-    #fix: can we search in $rbd hash key with a regex to find (vm|base) ?
-    for (my $i = 1; $i < 100; $i++) {
-        if (!$disk_ids->{$i}) {
-            return "vm-$vmid-disk-$i";
-        }
-    }
-
-    die "unable to allocate an image name for VM $vmid in storage '$storeid'\n";
+    return PVE::Storage::Plugin::get_next_vm_diskname($disk_list, $storeid, $vmid, undef, $scfg);
 };
 
 sub create_base {
@@ -401,7 +391,7 @@ sub clone_image {
     die "$volname is not a base image and snapname is not provided\n" 
 	if !$isBase && !length($snapname);
 
-    my $name = &$find_free_diskname($storeid, $scfg, $vmid);
+    my $name = $find_free_diskname->($storeid, $scfg, $vmid);
 
     warn "clone $volname: $basename snapname $snap to $name\n";
 
@@ -434,7 +424,7 @@ sub alloc_image {
     die "illegal name '$name' - should be 'vm-$vmid-*'\n"
 	if  $name && $name !~ m/^vm-$vmid-/;
 
-    $name = &$find_free_diskname($storeid, $scfg, $vmid) if !$name;
+    $name = $find_free_diskname->($storeid, $scfg, $vmid) if !$name;
 
     my $cmd = &$rbd_cmd($scfg, $storeid, 'create', '--image-format' , 2, '--size', int(($size+1023)/1024), $name);
     run_rbd_command($cmd, errmsg => "rbd create $name' error");
diff --git a/PVE/Storage/SheepdogPlugin.pm b/PVE/Storage/SheepdogPlugin.pm
index f10ef31..5a49ce5 100644
--- a/PVE/Storage/SheepdogPlugin.pm
+++ b/PVE/Storage/SheepdogPlugin.pm
@@ -151,22 +151,9 @@ my $find_free_diskname = sub {
 
     my $sheepdog = sheepdog_ls($scfg, $storeid);
     my $dat = $sheepdog->{$storeid};
-    my $disk_ids = {};
+    my $disk_list = [ keys %$dat ];
 
-    foreach my $image (keys %$dat) {
-	my $volname = $dat->{$image}->{name};
-	if ($volname =~ m/(vm|base)-$vmid-disk-(\d+)/){
-	    $disk_ids->{$2} = 1;
-	}
-    }
-
-    for (my $i = 1; $i < 100; $i++) {
-	if (!$disk_ids->{$i}) {
-	    return "vm-$vmid-disk-$i";
-	}
-    }
-
-    die "unable to allocate an image name for VM $vmid in storage '$storeid'\n";
+    return PVE::Storage::Plugin::get_next_vm_diskname($disk_list, $storeid, $vmid, undef, $scfg);
 };
 
 sub create_base {
@@ -225,7 +212,7 @@ sub clone_image {
 
     die "clone_image only works on base images\n" if !$isBase;
 
-    my $name = &$find_free_diskname($storeid, $scfg, $vmid);
+    my $name = $find_free_diskname->($storeid, $scfg, $vmid);
 
     warn "clone $volname: $basename to $name\n";
 
@@ -243,7 +230,7 @@ sub alloc_image {
     die "illegal name '$name' - sould be 'vm-$vmid-*'\n"
 	if  $name && $name !~ m/^vm-$vmid-/;
 
-    $name = &$find_free_diskname($storeid, $scfg, $vmid) if !$name;
+    $name = $find_free_diskname->($storeid, $scfg, $vmid) if !$name;
 
     my $cmd = &$collie_cmd($scfg, 'vdi', 'create', $name , "${size}k");
 
diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index 32e53aa..6e08457 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -388,26 +388,11 @@ sub zfs_list_zvol {
 sub zfs_find_free_diskname {
     my ($class, $storeid, $scfg, $vmid, $format) = @_;
 
-    my $name = undef;
     my $volumes = $class->zfs_list_zvol($scfg);
-
-    my $disk_ids = {};
     my $dat = $volumes->{$scfg->{pool}};
 
-    foreach my $image (keys %$dat) {
-        my $volname = $dat->{$image}->{name};
-        if ($volname =~ m/(vm|base|subvol|basevol)-$vmid-disk-(\d+)/){
-            $disk_ids->{$2} = 1;
-        }
-    }
-
-    for (my $i = 1; $i < 100; $i++) {
-        if (!$disk_ids->{$i}) {
-            return $format eq 'subvol' ? "subvol-$vmid-disk-$i" : "vm-$vmid-disk-$i";
-        }
-    }
-
-    die "unable to allocate an image name for VM $vmid in storage '$storeid'\n";
+    my $disk_list = [ keys %$dat ];
+    return PVE::Storage::Plugin::get_next_vm_diskname($disk_list, $storeid, $vmid, $format, $scfg);
 }
 
 sub zfs_get_latest_snapshot {
-- 
2.11.0




More information about the pve-devel mailing list