[pve-devel] [PATCH qemu-server] api/migration: fix autocomplete for targetstorage

[Thomas Lamprecht]

On 11/18/19 12:34 PM, Aaron Lauterer wrote:
> Show storages configured for the target node and not for the current one
> because they can be different.
> 
> Duplicated the `complete_storage` sub and extended it to extract the
> targetnode from the parameters to pass it into the storage_check_enabled
> function.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer at proxmox.com>
> ---
>  PVE/API2/Qemu.pm  |  2 +-
>  PVE/QemuServer.pm | 27 +++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
> index 8e162aa..3989633 100644
> --- a/PVE/API2/Qemu.pm
> +++ b/PVE/API2/Qemu.pm
> @@ -3299,7 +3299,7 @@ __PACKAGE__->register_method({
>              targetstorage => get_standard_option('pve-storage-id', {
>  		description => "Default target storage.",
>  		optional => 1,
> -		completion => \&PVE::QemuServer::complete_storage,
> +		completion => \&PVE::QemuServer::complete_migration_storage,
>              }),
>  	    bwlimit => {
>  		description => "Override I/O bandwidth limit (in KiB/s).",
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 54c8c88..5fc2495 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -7459,4 +7459,31 @@ sub complete_storage {
>      return $res;
>  }
>  
> +sub complete_migration_storage {
> +    my @parameters = @_;
> +
> +    # Dumper outut for @parameters:
> +    # $VAR1 = 'migrate';
> +    # $VAR2 = 'targetstorage';
> +    # $VAR3 = '';
> +    # $VAR4 = [
> +    #      '<vmid>',
> +    #      '<targetnode>',
> +    #      further optional parameters
> +    #    ];

instead of this @param index accesses and big comment you could do:

my ($cmd, $param, $current_value, $all_args) = @_;

would make the code easier to read, and more helpful than an unnamed
Dumper output, which lets one still guess what what is :)

Looks OK besides that.

> +    my $targetnode = $parameters[3][1];
> +
> +    my $cfg = PVE::Storage::config();
> +    my $ids = $cfg->{ids};
> +
> +    my $res = [];
> +    foreach my $sid (keys %$ids) {
> +	next if !PVE::Storage::storage_check_enabled($cfg, $sid, $targetnode, 1);
> +	next if !$ids->{$sid}->{content}->{images};
> +	push @$res, $sid;
> +    }
> +
> +    return $res;
> +}
> +
>  1;
>