[pve-devel] Ack: Re: [PATCH storage 1/3] rbd: use 'rbd ls' without '-l' to find free names

[Emmanuel Kasper]

I tested this patch OK, and code looks good to me.

On 02/03/2017 01:00 PM, Fabian Grünbichler wrote:
> with more than a few images, 'rbd ls -l' gets rather slow
> compared to a simple 'rbd ls'. since we only need to check
> existing image names for finding a free one, the latter is
> sufficient.
> 
> example with ~400 rbd images:
> $ time rbd ls -p ceph-vm > /dev/null
> real    0m0.027s
> user    0m0.012s
> sys     0m0.008s
> 
> $ time rbd ls -l -p ceph-vm > /dev/null
> real    0m5.250s
> user    0m1.632s
> sys     0m0.584s
> 
> a linked clone of two disks on the same setup accordingly
> also shows a massive speedup:
> 
> $ time qm clone 1000 10000 -snap test
> create linked clone of drive scsi0 (ceph-vm:vm-1000-disk-2)
> clone vm-1000-disk-2: vm-1000-disk-2 snapname test to
> vm-10000-disk-1
> create linked clone of drive scsi1 (ceph-vm:vm-1000-disk-1)
> clone vm-1000-disk-1: vm-1000-disk-1 snapname test to
> vm-10000-disk-2
> 
> real    0m11.157s
> user    0m3.752s
> sys     0m1.308s
> 
> $ time qm clone 1000 10000 -snap test
> create linked clone of drive scsi1 (ceph-vm:vm-1000-disk-1)
> clone vm-1000-disk-1: vm-1000-disk-1 snapname test to
> vm-10000-disk-1
> create linked clone of drive scsi0 (ceph-vm:vm-1000-disk-2)
> clone vm-1000-disk-2: vm-1000-disk-2 snapname test to
> vm-10000-disk-2
> 
> real    0m0.872s
> user    0m0.652s
> sys     0m0.096s
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
> ---
>  PVE/Storage/RBDPlugin.pm | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/PVE/Storage/RBDPlugin.pm b/PVE/Storage/RBDPlugin.pm
> index 989a85e..b44b9ba 100644
> --- a/PVE/Storage/RBDPlugin.pm
> +++ b/PVE/Storage/RBDPlugin.pm
> @@ -338,17 +338,24 @@ sub path {
>  my $find_free_diskname = sub {
>      my ($storeid, $scfg, $vmid) = @_;
>  
> -    my $rbd = rbd_ls($scfg, $storeid);
> -    my $pool =  $scfg->{pool} ? $scfg->{pool} : 'rbd';
> +    my $cmd = &$rbd_cmd($scfg, $storeid, 'ls');
>      my $disk_ids = {};
> -    my $dat = $rbd->{$pool};
>  
> -    foreach my $image (keys %$dat) {
> -	my $volname = $dat->{$image}->{name};
> -	if ($volname =~ m/(vm|base)-$vmid-disk-(\d+)/){
> +    my $parser = sub {
> +	my $line = shift;
> +
> +	if ($line =~  m/^(vm|base)-\Q$vmid\E+-disk-(\d+)$/) {
>  	    $disk_ids->{$2} = 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/;
> +
>      #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}) {
>