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.eb...@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 _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel