[pve-devel] [PATCH guest-common 1/6] Cleanup fix for stateless jobs.

[Thomas Lamprecht]

On 12/13/2017 03:46 PM, Wolfgang Link wrote:
> If a VM config was stolen or migrated through HA there is no state on that node.
> It must be possible to cleanup the remote-side without state.
> ---
>  PVE/Replication.pm | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/PVE/Replication.pm b/PVE/Replication.pm
> index c25ed44..ce2109e 100644
> --- a/PVE/Replication.pm
> +++ b/PVE/Replication.pm
> @@ -200,8 +200,14 @@ sub replicate {
>  
>  	if ($remove_job eq 'full' && $jobcfg->{target} ne $local_node) {
>  	    # remove all remote volumes
> +	    my $store_list = [];
> +	    foreach my $volid (@$sorted_volids) {
> +		my ($storeid) = PVE::Storage::parse_volume_id($volid);
> +		push @$store_list, $storeid;
> +	    }

FYI, this five lines could be written as:

my @store_list = map { PVE::Storage::parse_volume_id($_) } @$sorted_volids;

# then below:
remote_prepare_local_job($ssh_info, $jobid, $vmid, [], \@store_list, 0, undef, 1, $logfunc);

map[1] can be used for array to array mappings quite well, it goes
through the whole array and applies the code in its { BLOCK },
the return value (which is the last statement if no return is there)
is the element of a new array. Also array to hash mapping can be done
quite nicely, as hashes in perl can be seens as fancy arrays with even
number of elements.

It's, IMO, expressive and makes the code easier/quicker to read.
But, just a note, your way works naturally too :)

> +
>  	    my $ssh_info = PVE::Cluster::get_ssh_info($jobcfg->{target});
> -	    remote_prepare_local_job($ssh_info, $jobid, $vmid, [], $state->{storeid_list}, 0, undef, 1, $logfunc);
> +	    remote_prepare_local_job($ssh_info, $jobid, $vmid, [], $store_list, 0, undef, 1, $logfunc);
>  
>  	}
>  	# remove all local replication snapshots (lastsync => 0)
> 

[1] https://perldoc.perl.org/functions/map.html