[pve-devel] [PATCH storage] Use a common find_free_diskname in all plugins
Fabian Ebner
f.ebner at proxmox.com
Mon Dec 9 13:35:10 CET 2019
The local versions of find_free_diskname retrieved the relevant disk list using
plugin-specific code and called get_next_vm_diskname. We can use list_images
instead to allow for a common interface and avoid having those similar methods.
Signed-off-by: Fabian Ebner <f.ebner at proxmox.com>
---
I did not test for Glusterfs, non-thin LVM and ZFS over iSCSI. It seems to work
for the other plugins.
PVE/Storage/GlusterfsPlugin.pm | 15 ++-------------
PVE/Storage/LVMPlugin.pm | 10 +---------
PVE/Storage/LvmThinPlugin.pm | 4 ++--
PVE/Storage/Plugin.pm | 21 ++++++++-------------
PVE/Storage/RBDPlugin.pm | 27 ++-------------------------
PVE/Storage/ZFSPlugin.pm | 2 +-
PVE/Storage/ZFSPoolPlugin.pm | 16 +++-------------
7 files changed, 19 insertions(+), 76 deletions(-)
diff --git a/PVE/Storage/GlusterfsPlugin.pm b/PVE/Storage/GlusterfsPlugin.pm
index b3e5553..2cf2da9 100644
--- a/PVE/Storage/GlusterfsPlugin.pm
+++ b/PVE/Storage/GlusterfsPlugin.pm
@@ -160,17 +160,6 @@ sub parse_name_dir {
die "unable to parse volume filename '$name'\n";
}
-my $find_free_diskname = sub {
- my ($imgdir, $vmid, $fmt, $scfg) = @_;
-
- my $disk_list = [];
-
- my $dh = IO::Dir->new ($imgdir);
- @$disk_list = $dh->read() if defined($dh);
-
- return PVE::Storage::Plugin::get_next_vm_diskname($disk_list, $imgdir, $vmid, $fmt, $scfg, 1);
-};
-
sub path {
my ($class, $scfg, $volname, $storeid, $snapname) = @_;
@@ -225,7 +214,7 @@ sub clone_image {
mkpath $imagedir;
- my $name = $find_free_diskname->($imagedir, $vmid, "qcow2", $scfg);
+ my $name = $class->find_free_diskname($imagedir, $scfg, $vmid, "qcow2", 1);
warn "clone $volname: $vtype, $name, $vmid to $name (base=../$basevmid/$basename)\n";
@@ -253,7 +242,7 @@ sub alloc_image {
mkpath $imagedir;
- $name = $find_free_diskname->($imagedir, $vmid, $fmt, $scfg) if !$name;
+ $name = $class->find_free_diskname($imagedir, $scfg, $vmid, $fmt, 1) if !$name;
my (undef, $tmpfmt) = parse_name_dir($name);
diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm
index c652e2a..13e6409 100644
--- a/PVE/Storage/LVMPlugin.pm
+++ b/PVE/Storage/LVMPlugin.pm
@@ -303,14 +303,6 @@ sub clone_image {
die "can't clone images in lvm storage\n";
}
-sub lvm_find_free_diskname {
- my ($lvs, $vg, $storeid, $vmid, $scfg) = @_;
-
- my $disk_list = [ keys %{$lvs->{$vg}} ];
-
- return PVE::Storage::Plugin::get_next_vm_diskname($disk_list, $storeid, $vmid, undef, $scfg);
-}
-
sub lvcreate {
my ($vg, $name, $size, $tags) = @_;
@@ -345,7 +337,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, $scfg)
+ $name = $class->find_free_diskname($storeid, $scfg, $vmid)
if !$name;
lvcreate($vg, $name, $size, ["pve-vm-$vmid"]);
diff --git a/PVE/Storage/LvmThinPlugin.pm b/PVE/Storage/LvmThinPlugin.pm
index aafc202..b00758d 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, $scfg)
+ $name = $class->find_free_diskname($storeid, $scfg, $vmid)
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, $scfg);
+ my $name = $class->find_free_diskname($storeid, $scfg, $vmid);
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 d0f1df6..54aae41 100644
--- a/PVE/Storage/Plugin.pm
+++ b/PVE/Storage/Plugin.pm
@@ -559,7 +559,7 @@ sub get_next_vm_diskname {
my $disk_ids = {};
foreach my $disk (@$disk_list) {
- my $disknum = $get_vm_disk_number->($disk, $scfg, $vmid, $suffix);
+ my $disknum = $get_vm_disk_number->($disk->{volid}, $scfg, $vmid, $suffix);
$disk_ids->{$disknum} = 1 if defined($disknum);
}
@@ -572,18 +572,13 @@ sub get_next_vm_diskname {
die "unable to allocate an image name for VM $vmid in storage '$storeid'\n"
}
-my $find_free_diskname = sub {
- my ($imgdir, $vmid, $fmt, $scfg) = @_;
+sub find_free_diskname {
+ my ($class, $storeid, $scfg, $vmid, $fmt, $add_fmt_suffix) = @_;
- my $disk_list = [];
+ my $disk_list = $class->list_images($storeid, $scfg, $vmid);
- if (defined(my $dh = IO::Dir->new($imgdir))) {
- @$disk_list = $dh->read();
- $dh->close();
- }
-
- return get_next_vm_diskname($disk_list, $imgdir, $vmid, $fmt, $scfg, 1);
-};
+ return get_next_vm_diskname($disk_list, $storeid, $vmid, $fmt, $scfg, $add_fmt_suffix);
+}
sub clone_image {
my ($class, $scfg, $storeid, $volname, $vmid, $snap) = @_;
@@ -607,7 +602,7 @@ sub clone_image {
mkpath $imagedir;
- my $name = $find_free_diskname->($imagedir, $vmid, "qcow2", $scfg);
+ my $name = $class->find_free_diskname($imagedir, $scfg, $vmid, "qcow2", 1);
warn "clone $volname: $vtype, $name, $vmid to $name (base=../$basevmid/$basename)\n";
@@ -639,7 +634,7 @@ sub alloc_image {
mkpath $imagedir;
- $name = $find_free_diskname->($imagedir, $vmid, $fmt, $scfg) if !$name;
+ $name = $class->find_free_diskname($imagedir, $scfg, $vmid, $fmt, 1) if !$name;
my (undef, $tmpfmt) = parse_name_dir($name);
diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
index 538d6bb..c552785 100644
--- a/PVE/Storage/RBDPlugin.pm
+++ b/PVE/Storage/RBDPlugin.pm
@@ -362,29 +362,6 @@ sub path {
return ($path, $vmid, $vtype);
}
-my $find_free_diskname = sub {
- my ($storeid, $scfg, $vmid) = @_;
-
- my $cmd = &$rbd_cmd($scfg, $storeid, 'ls');
- my $disk_list = [];
-
- my $parser = sub {
- my $line = shift;
- if ($line =~ m/^(.*)$/) { # untaint
- push @$disk_list, $1;
- }
- };
-
- eval {
- run_rbd_command($cmd, errmsg => "rbd error", errfunc => sub {}, outfunc => $parser);
- };
- my $err = $@;
-
- die $err if $err && $err !~ m/doesn't contain rbd images/;
-
- return PVE::Storage::Plugin::get_next_vm_diskname($disk_list, $storeid, $vmid, undef, $scfg);
-};
-
sub create_base {
my ($class, $storeid, $scfg, $volname) = @_;
@@ -438,7 +415,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 = $class->find_free_diskname($storeid, $scfg, $vmid);
warn "clone $volname: $basename snapname $snap to $name\n";
@@ -469,7 +446,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 = $class->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/ZFSPlugin.pm b/PVE/Storage/ZFSPlugin.pm
index 8c6709c..383f0a0 100644
--- a/PVE/Storage/ZFSPlugin.pm
+++ b/PVE/Storage/ZFSPlugin.pm
@@ -294,7 +294,7 @@ sub alloc_image {
my $volname = $name;
- $volname = $class->zfs_find_free_diskname($storeid, $scfg, $vmid, $fmt) if !$volname;
+ $volname = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt) if !$volname;
$class->zfs_create_zvol($scfg, $volname, $size);
diff --git a/PVE/Storage/ZFSPoolPlugin.pm b/PVE/Storage/ZFSPoolPlugin.pm
index 456fb40..12dc2be 100644
--- a/PVE/Storage/ZFSPoolPlugin.pm
+++ b/PVE/Storage/ZFSPoolPlugin.pm
@@ -240,7 +240,7 @@ sub alloc_image {
die "illegal name '$volname' - should be 'vm-$vmid-*'\n"
if $volname && $volname !~ m/^vm-$vmid-/;
- $volname = $class->zfs_find_free_diskname($storeid, $scfg, $vmid, $fmt)
+ $volname = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt)
if !$volname;
$class->zfs_create_zvol($scfg, $volname, $size);
@@ -250,7 +250,7 @@ sub alloc_image {
die "illegal name '$volname' - should be 'subvol-$vmid-*'\n"
if $volname && $volname !~ m/^subvol-$vmid-/;
- $volname = $class->zfs_find_free_diskname($storeid, $scfg, $vmid, $fmt)
+ $volname = $class->find_free_diskname($storeid, $scfg, $vmid, $fmt)
if !$volname;
die "illegal name '$volname' - should be 'subvol-$vmid-*'\n"
@@ -423,16 +423,6 @@ sub zfs_list_zvol {
return $list;
}
-sub zfs_find_free_diskname {
- my ($class, $storeid, $scfg, $vmid, $format) = @_;
-
- my $volumes = $class->zfs_list_zvol($scfg);
- my $dat = $volumes->{$scfg->{pool}};
-
- my $disk_list = [ keys %$dat ];
- return PVE::Storage::Plugin::get_next_vm_diskname($disk_list, $storeid, $vmid, $format, $scfg);
-}
-
sub zfs_get_latest_snapshot {
my ($class, $scfg, $volname) = @_;
@@ -612,7 +602,7 @@ sub clone_image {
die "clone_image only works on base images\n" if !$isBase;
- my $name = $class->zfs_find_free_diskname($storeid, $scfg, $vmid, $format);
+ my $name = $class->find_free_diskname($storeid, $scfg, $vmid, $format);
if ($format eq 'subvol') {
my $size = $class->zfs_request($scfg, undef, 'list', '-H', '-o', 'refquota', "$scfg->{pool}/$basename");
--
2.20.1
More information about the pve-devel
mailing list