On 12/11/19 10:25 AM, Fabian Ebner wrote: > We can use 'list_images' to get the desired volume IDs in > 'find_free_diskname' for most plugins. For the two LVM plugins, 'list_images' > potentially skips untagged volumes, so we keep the custom version. For the > RBD plugin, 'list_images' is much more costly than the custom version, so we > keep the custom version. > > Signed-off-by: Fabian Ebner <f.eb...@proxmox.com> > --- > > Changes from v1: > * Keep the custom versions in LVMPlugin and RBDPlugin > * Do not change the interface for get_next_vm_diskname > > Thanks to Fabian for the suggestions! > > PVE/Storage/GlusterfsPlugin.pm | 15 ++------------- > PVE/Storage/LVMPlugin.pm | 10 +++++++--- > PVE/Storage/LvmThinPlugin.pm | 8 ++------ > PVE/Storage/Plugin.pm | 19 ++++++++++--------- > PVE/Storage/RBDPlugin.pm | 10 +++++----- > PVE/Storage/ZFSPlugin.pm | 2 +- > PVE/Storage/ZFSPoolPlugin.pm | 16 +++------------- > 7 files changed, 30 insertions(+), 50 deletions(-) >
applied, thanks! Did a small refactoring as follow-up (see below) > diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm > index d0f1df6..73f80af 100644 > --- a/PVE/Storage/Plugin.pm > +++ b/PVE/Storage/Plugin.pm > @@ -572,18 +572,19 @@ 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 $disks = $class->list_images($storeid, $scfg, $vmid); > > my $disk_list = []; > > - if (defined(my $dh = IO::Dir->new($imgdir))) { > - @$disk_list = $dh->read(); > - $dh->close(); > + foreach my $disk (@{$disks}) { > + push @{$disk_list}, $disk->{volid}; > } for above I followed up with: diff --git a/PVE/Storage/Plugin.pm b/PVE/Storage/Plugin.pm index 73f80af..353632c 100644 --- a/PVE/Storage/Plugin.pm +++ b/PVE/Storage/Plugin.pm @@ -577,11 +577,7 @@ sub find_free_diskname { my $disks = $class->list_images($storeid, $scfg, $vmid); - my $disk_list = []; - - foreach my $disk (@{$disks}) { - push @{$disk_list}, $disk->{volid}; - } + my $disk_list = [ map { $_->{volid} } @$disks ]; return get_next_vm_diskname($disk_list, $storeid, $vmid, $fmt, $scfg, $add_fmt_suffix); } _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel