[pve-devel] [RFC guest-common 1/3] fix #1291: implement remove_vmid_from_cronjobs

[Thomas Lamprecht]

On 6/25/19 2:27 PM, Christian Ebner wrote:
> remove_vmid_from_cronjobs updates the vzdump.cron backup jobs,
> excluding the given vmid.
> 
> Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
> ---
>  PVE/VZDump/Plugin.pm | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/PVE/VZDump/Plugin.pm b/PVE/VZDump/Plugin.pm
> index 9933ef6..28f018b 100644
> --- a/PVE/VZDump/Plugin.pm
> +++ b/PVE/VZDump/Plugin.pm
> @@ -7,6 +7,8 @@ use POSIX qw(strftime);
>  
>  use PVE::Tools;
>  use PVE::SafeSyslog;
> +use PVE::Cluster qw(cfs_read_file cfs_write_file cfs_lock_file);
> +use PVE::API2::Backup;
>  
>  my $log_level = {
>      err =>  'ERROR:',
> @@ -168,4 +170,53 @@ sub cleanup {
>      die "internal error"; # implement in subclass
>  }
>  
> +sub exclude_vmid_from_list {
> +    my ($list, $exclude_vmid) = @_;
> +
> +    my $updated_list = [];
> +    foreach my $vmid (PVE::Tools::split_list($list)) {
> +	push @$updated_list, $vmid if $vmid ne $exclude_vmid;
> +    }

above could also be done like:

return join(',', grep { $_ ne $exclude_vmid } PVE::Tools::split_list($list) };


I mean putting all in a single line is probably not too readable, so
could be split up, but the principle should stay the same.

> +    return join ",", @$updated_list;
> +}
> +
> +sub exclude_vmid_from_jobs {

"remove_vmid_from_jobs" as exclude is a bit confusing in the backup job
context, which has already excludes itself.

> +    my ($jobs, $exclude_vmid) = @_;
> +
> +    my $updated_jobs = [];
> +    foreach my $job (@$jobs) {
> +	if (defined $job->{vmid}) {
> +	    my $list = exclude_vmid_from_list($job->{vmid}, $exclude_vmid);
> +	    if ($list) {
> +		$job->{vmid} = $list;
> +		push @$updated_jobs, $job;
> +	    }
> +	} elsif (defined $job->{exclude}) {
> +	    my $list = exclude_vmid_from_list($job->{exclude}, $exclude_vmid);
> +	    if ($list) {
> +		$job->{exclude} = $list;
> +	    } else {
> +		delete $job->{exclude};
> +	    }
> +	    push @$updated_jobs, $job;
> +	} else {
> +	    push @$updated_jobs, $job;
> +	}
> +    }
> +    return $updated_jobs;
> +}
> +
> +sub remove_vmid_from_cronjobs {

.._from_backup_jobs

> +    my ($vmid) = @_;
> +
> +    my $update_cron = sub {
> +	my $cron_cfg = cfs_read_file('vzdump.cron');

s/cron_cfg/vzdump_cfg/ or vzdump_jobs or the like, cron is here not
really important.. (to confuse people less :))

> +	my $jobs = $cron_cfg->{jobs} || [];
> +	$cron_cfg->{jobs} = exclude_vmid_from_jobs($jobs, $vmid);
> +	cfs_write_file('vzdump.cron', $cron_cfg);
> +    };
> +    cfs_lock_file('vzdump.cron', undef, $update_cron);


FYI: you can also do:

cfs_lock_file('vzdump.cron', undef, sub {
    my $vzdump_jobs = cfs_read_file('vzdump.cron');
    ...
});

(for shorter things like that we often do it that way, albeit it's
not 100% consistent)

> +    die "$@" if ($@);
> +}
> +
>  1;
>