Re: [pve-devel] [PATCH v6 storage 1/1] add disk reassign feature

["Dominic Jäger" <d.jaeger@proxmox.com>]

Needs a rebase since the rbd patches

On Fri, Apr 02, 2021 at 12:19:19PM +0200, Aaron Lauterer wrote:
> +sub reassign_volume {
> +    my ($class, $scfg, $storeid, $volname, $target_vmid) = @_;
> +
> +    my $base;;
> +    (undef, $volname, undef, $base) = $class->parse_volname($volname);
> +
> +    if ($base) {
> +	$base = $base . '/';
> +    } else {
> +	$base = '';
> +    }
> +
> +    $class->cluster_lock_storage($storeid, $scfg->{shared}, undef, sub {
> +	my $target_volname = $class->find_free_diskname($storeid, $scfg, $target_vmid);
> +	return $class->rename_volume($scfg, $storeid, $volname, $target_volname, $target_vmid, $base);
> +    });
> +}
> +
> +sub rename_volume {
> +    my ($class, $scfg, $storeid, $source_volname, $target_volname, $vmid, $base) = @_;
> +
> +    $class->zfs_request($scfg, 5, 'rename', "$scfg->{pool}/$source_volname", "$scfg->{pool}/$target_volname");
> +    return "${storeid}:${base}${target_volname}";
> +}
1. imo a ternary would not decrease readability much here and it is only one line
> $base = $base ? "$base/" : "";
2. We could maybe move the if to rename_volume. As far as I see, it is not used
   in reassign_volume and we could avoid silently guaranteeing properties of $base
   across subs.

And there is a double ;; somewhere