[pve-devel] [PATCH v5 manager 1/2] vzdump: move remaining guest include logic to single method

[Aaron Lauterer]

On 6/5/20 8:39 PM, Thomas Lamprecht wrote:
> On 5/6/20 11:57 AM, Aaron Lauterer wrote:
[...]
>> @@ -1169,34 +1158,39 @@ sub get_included_guests {
>>   
>>       my $nodename = PVE::INotify::nodename();
>>       my $vmids = [];
>> +    my $vmids_per_node = {};
>> +
>> +    my $vmlist = PVE::Cluster::get_vmlist();
>>   
>> -    # convert string lists to arrays
>>       if ($job->{pool}) {
>>   	$vmids = PVE::API2Tools::get_resource_pool_guest_members($job->{pool});
>> +    } elsif ($job->{vmid}) {
>> +	$vmids = [ PVE::Tools::split_list($job->{vmid}) ];
>>       } else {
>> -	$vmids = [ PVE::Tools::split_list(extract_param($job, 'vmid')) ];
>> +	# all or exclude
>> +	my @exclude = PVE::Tools::split_list($job->{exclude});
>> +	@exclude = @{PVE::VZDump::check_vmids(@exclude)};
>> +	my $excludehash = { map { $_ => 1 } @exclude };
>> +
>> +	for my $id (keys %{ $vmlist->{ids} }) {
> 
> you could sort it here already, avoiding the sort after the loop.
> 
> Whole thing could also be written as:
> 
> $vmids = [ grep { !$excludehash->{$_} } sort keys $vmlist->{ids}->%* ];
> 
> but no hard feeling here.
> 
>> +	    next if $excludehash->{$id};
>> +	    push @$vmids, $id;
>> +	}
>>       }
>> +    $vmids = [ sort {$a <=> $b} @$vmids];
>> 

I just realized that there was a reason to sort the vmids at this point. 
When doing the sorting here we sort them no matter the source. When 
selecting the VMs manually, the order in which they are selected in the 
GUI is the order in which they will be stored in the vzdump.cron file.

If we move the sorting into that for loop we will only sort them when we 
have an `all` and/or `exclude` job.