[pve-devel] [PATCH manager 6/7] api/backup: handle new vzdump jobs
Fabian Ebner
f.ebner at proxmox.com
Wed Nov 3 10:05:02 CET 2021
Am 07.10.21 um 10:27 schrieb Dominik Csapak:
> in addition to listing the vzdump.cron jobs, also list from the
> jobs.cfg file.
>
> updates/creations go into the new jobs.cfg only now
> and on update, starttime+dow get converted to a schedule
> this transformation is straight forward, since 'dow'
> is already in a compatible format (e.g. 'mon,tue') and we simply
> append the starttime (if any)
>
> id on creation is optional for now (for api compat), but will
> be autogenerated (uuid). on update, we simply take the id from before
> (the ids of the other entries in vzdump.cron will change but they would
> anyway)
>
> as long as we have the vzdump.cron file, we must lock both
> vzdump.cron and jobs.cfg, since we often update both
>
> we also change the backupinfo api call to read the jobs.cfg too
>
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
> PVE/API2/Backup.pm | 243 +++++++++++++++++++++++++++------
> PVE/API2/Cluster/BackupInfo.pm | 10 ++
> 2 files changed, 208 insertions(+), 45 deletions(-)
>
> diff --git a/PVE/API2/Backup.pm b/PVE/API2/Backup.pm
> index 9dc3b48e..56ed4be8 100644
> --- a/PVE/API2/Backup.pm
> +++ b/PVE/API2/Backup.pm
> @@ -3,6 +3,7 @@ package PVE::API2::Backup;
> use strict;
> use warnings;
> use Digest::SHA;
> +use UUID;
>
> use PVE::SafeSyslog;
> use PVE::Tools qw(extract_param);
> @@ -14,6 +15,7 @@ use PVE::Storage;
> use PVE::Exception qw(raise_param_exc);
> use PVE::VZDump;
> use PVE::VZDump::Common;
> +use PVE::Jobs; # for VZDump Jobs
>
> use base qw(PVE::RESTHandler);
>
> @@ -45,6 +47,19 @@ my $assert_param_permission = sub {
> }
> };
>
> +my $convert_to_schedule = sub {
> + my ($job) = @_;
> +
> + my $starttime = $job->{starttime};
> + my $dow = $job->{dow};
> +
> + if (!$dow || $dow eq ALL_DAYS) {
> + return "$starttime";
> + }
> +
> + return "$dow $starttime";
> +};
> +
> __PACKAGE__->register_method({
> name => 'index',
> path => '',
> @@ -74,8 +89,20 @@ __PACKAGE__->register_method({
> my $user = $rpcenv->get_user();
>
> my $data = cfs_read_file('vzdump.cron');
> + my $jobs_data = cfs_read_file('jobs.cfg');
> + my $order = $jobs_data->{order};
> + my $jobs = $jobs_data->{ids};
>
> my $res = $data->{jobs} || [];
> + foreach my $job (@$res) {
> + $job->{schedule} = $convert_to_schedule->($job);
> + }
> +
> + foreach my $jobid (sort { $order->{$a} <=> $order->{$b} } keys %$jobs) {
> + my $job = $jobs->{$jobid};
> + next if $job->{type} ne 'vzdump';
> + push @$res, $job;
> + }
>
> return $res;
> }});
> @@ -93,11 +120,24 @@ __PACKAGE__->register_method({
> parameters => {
> additionalProperties => 0,
> properties => PVE::VZDump::Common::json_config_properties({
> + id => {
> + type => 'string',
> + description => "Job ID (will be autogenerated).",
> + format => 'pve-configid',
> + optional => 1, # FIXME: make required on 8.0
> + },
> + schedule => {
> + description => "Backup schedule. The format is a subset of `systemd` calendar events.",
> + type => 'string', format => 'pve-calendar-event',
> + maxLength => 128,
> + optional => 1,
> + },
> starttime => {
> type => 'string',
> description => "Job Start time.",
> pattern => '\d{1,2}:\d{1,2}',
> typetext => 'HH:MM',
> + optional => 1,
> },
> dow => {
> type => 'string', format => 'pve-day-of-week-list',
> @@ -127,19 +167,40 @@ __PACKAGE__->register_method({
> $rpcenv->check($user, "/pool/$pool", ['VM.Backup']);
> }
>
> + if (defined($param->{schedule})) {
> + if (defined($param->{starttime})) {
> + raise_param_exc({ starttime => "'starttime' and 'schedule' cannot both be set" });
> + }
Should schedule also clash with dow? Or use requires => 'starttime' like
for the update call?
> + } elsif (!defined($param->{starttime})) {
> + raise_param_exc({ schedule => "neither 'starttime' nor 'schedule' were set" });
> + } else {
> + $param->{schedule} = $convert_to_schedule->($param);
> + }
> +
> + delete $param->{starttime};
> + delete $param->{dow};
>
The schedule conversion/checks could be factored out to avoid
duplication for the update call.
> - my $create_job = sub {
> - my $data = cfs_read_file('vzdump.cron');
> + $param->{enabled} = 1 if !defined($param->{enabled});
> +
> + # autogenerate id for api compatibility FIXME remove with 8.0
> + my $id = extract_param($param, 'id') // uuid();
> +
> + cfs_lock_file('jobs.cfg', undef, sub {
> + my $data = cfs_read_file('jobs.cfg');
> +
> + die "Job '$id' already exists\n"
> + if $data->{ids}->{$id};
>
> - $param->{dow} = ALL_DAYS if !defined($param->{dow});
> - $param->{enabled} = 1 if !defined($param->{enabled});
> PVE::VZDump::verify_vzdump_parameters($param, 1);
> + my $plugin = PVE::Jobs::Plugin->lookup('vzdump');
> + my $opts = $plugin->check_config($id, $param, 1, 1);
>
> - push @{$data->{jobs}}, $param;
> + $data->{ids}->{$id} = $opts;
>
> - cfs_write_file('vzdump.cron', $data);
> - };
> - cfs_lock_file('vzdump.cron', undef, $create_job);
> + PVE::Jobs::create_job($id, 'vzdump');
> +
> + cfs_write_file('jobs.cfg', $data);
> + });
> die "$@" if ($@);
>
> return undef;
> @@ -173,9 +234,16 @@ __PACKAGE__->register_method({
> my $jobs = $data->{jobs} || [];
>
> foreach my $job (@$jobs) {
> - return $job if $job->{id} eq $param->{id};
> + if ($job->{id} eq $param->{id}) {
> + $job->{schedule} = $convert_to_schedule->($job);
> + return $job;
> + }
> }
>
> + my $jobs_data = cfs_read_file('jobs.cfg');
> + my $job = $jobs_data->{ids}->{$param->{id}};
> + return $job if $job && $job->{type} eq 'vzdump';
> +
> raise_param_exc({ id => "No such job '$param->{id}'" });
>
> }});
> @@ -202,6 +270,8 @@ __PACKAGE__->register_method({
> my $rpcenv = PVE::RPCEnvironment::get();
> my $user = $rpcenv->get_user();
>
> + my $id = $param->{id};
> +
> my $delete_job = sub {
> my $data = cfs_read_file('vzdump.cron');
>
> @@ -210,18 +280,32 @@ __PACKAGE__->register_method({
>
> my $found;
> foreach my $job (@$jobs) {
> - if ($job->{id} eq $param->{id}) {
> + if ($job->{id} eq $id) {
> $found = 1;
> } else {
> push @$newjobs, $job;
> }
> }
>
> - raise_param_exc({ id => "No such job '$param->{id}'" }) if !$found;
> + if (!$found) {
> + cfs_lock_file('jobs.cfg', undef, sub {
> + my $jobs_data = cfs_read_file('jobs.cfg');
>
> - $data->{jobs} = $newjobs;
> + if (!defined($jobs_data->{ids}->{$id})) {
> + raise_param_exc({ id => "No such job '$id'" });
> + }
> + delete $jobs_data->{ids}->{$id};
> +
> + PVE::Jobs::remove_job($id, 'vzudmp');
> +
> + cfs_write_file('jobs.cfg', $jobs_data);
> + });
> + die "$@" if $@;
> + } else {
> + $data->{jobs} = $newjobs;
>
> - cfs_write_file('vzdump.cron', $data);
> + cfs_write_file('vzdump.cron', $data);
> + }
> };
> cfs_lock_file('vzdump.cron', undef, $delete_job);
> die "$@" if ($@);
> @@ -243,15 +327,23 @@ __PACKAGE__->register_method({
> additionalProperties => 0,
> properties => PVE::VZDump::Common::json_config_properties({
> id => $vzdump_job_id_prop,
> + schedule => {
> + description => "Backup schedule. The format is a subset of `systemd` calendar events.",
> + type => 'string', format => 'pve-calendar-event',
> + maxLength => 128,
> + optional => 1,
> + },
> starttime => {
> type => 'string',
> description => "Job Start time.",
> pattern => '\d{1,2}:\d{1,2}',
> typetext => 'HH:MM',
> + optional => 1,
> },
> dow => {
> type => 'string', format => 'pve-day-of-week-list',
> optional => 1,
> + requires => 'starttime',
> description => "Day of week selection.",
> },
> delete => {
> @@ -281,57 +373,111 @@ __PACKAGE__->register_method({
> $rpcenv->check($user, "/pool/$pool", ['VM.Backup']);
> }
>
> + if (defined($param->{schedule})) {
> + if (defined($param->{starttime})) {
> + raise_param_exc({ starttime => "'starttime' and 'schedule' cannot both be set" });
> + }
> + } elsif (!defined($param->{starttime})) {
> + raise_param_exc({ schedule => "neither 'starttime' nor 'schedule' were set" });
> + } else {
> + $param->{schedule} = $convert_to_schedule->($param);
> + }
> +
> + delete $param->{starttime};
> + delete $param->{dow};
> +
> + my $id = extract_param($param, 'id');
> + my $delete = extract_param($param, 'delete');
> + if ($delete) {
> + $delete = [PVE::Tools::split_list($delete)];
> + }
> +
> my $update_job = sub {
> my $data = cfs_read_file('vzdump.cron');
> + my $jobs_data = cfs_read_file('jobs.cfg');
>
> my $jobs = $data->{jobs} || [];
>
> die "no options specified\n" if !scalar(keys %$param);
>
> PVE::VZDump::verify_vzdump_parameters($param);
> + my $plugin = PVE::Jobs::Plugin->lookup('vzdump');
> + my $opts = $plugin->check_config($id, $param, 0, 1);
> +
> + # try to find it in old vzdump.cron and convert it to a job
> + my $idx = 0;
> + my $found = 0;
> + foreach my $j (@$jobs) {
> + if ($j->{id} eq $id) {
> + $found = 1;
> + last;
> + }
> + $idx++;
> + }
Haven't tested it, but something like:
my ($idx) = grep { $jobs->[$_]->{id} eq $id } (0 .. scalar(@$jobs)
- 1);
would be a bit shorter, with using defined($idx) instead of $found. Just
an idea though.
>
> - my @delete = PVE::Tools::split_list(extract_param($param, 'delete'));
> + my $job;
> + if ($found) {
> + $job = splice @$jobs, $idx, 1;
> + $job->{schedule} = $convert_to_schedule->($job);
> + delete $job->{starttime};
> + delete $job->{dow};
> + delete $job->{id};
> + $job->{type} = 'vzdump';
> + $jobs_data->{ids}->{$id} = $job;
> + } else {
> + $job = $jobs_data->{ids}->{$id};
> + die "no such vzdump job\n" if !$job || $job->{type} ne 'vzdump';
> + }
>
> - foreach my $job (@$jobs) {
> - if ($job->{id} eq $param->{id}) {
> + foreach my $k (@$delete) {
> + if (!PVE::VZDump::option_exists($k)) {
> + raise_param_exc({ delete => "unknown option '$k'" });
> + }
>
> - foreach my $k (@delete) {
> - if (!PVE::VZDump::option_exists($k)) {
> - raise_param_exc({ delete => "unknown option '$k'" });
> - }
> + delete $job->{$k};
> + }
>
> - delete $job->{$k};
> - }
> + my $schedule_updated = 0;
> + if ($param->{schedule} ne $job->{schedule}) {
> + $schedule_updated = 1;
> + }
>
> - foreach my $k (keys %$param) {
> - $job->{$k} = $param->{$k};
> - }
> + foreach my $k (keys %$param) {
> + $job->{$k} = $param->{$k};
> + }
>
> - $job->{all} = 1 if (defined($job->{exclude}) && !defined($job->{pool}));
> -
> - if (defined($param->{vmid})) {
> - delete $job->{all};
> - delete $job->{exclude};
> - delete $job->{pool};
> - } elsif ($param->{all}) {
> - delete $job->{vmid};
> - delete $job->{pool};
> - } elsif ($job->{pool}) {
> - delete $job->{vmid};
> - delete $job->{all};
> - delete $job->{exclude};
> - }
> + $job->{all} = 1 if (defined($job->{exclude}) && !defined($job->{pool}));
> +
> + if (defined($param->{vmid})) {
> + delete $job->{all};
> + delete $job->{exclude};
> + delete $job->{pool};
> + } elsif ($param->{all}) {
> + delete $job->{vmid};
> + delete $job->{pool};
> + } elsif ($job->{pool}) {
> + delete $job->{vmid};
> + delete $job->{all};
> + delete $job->{exclude};
> + }
>
> - PVE::VZDump::verify_vzdump_parameters($job, 1);
> + PVE::VZDump::verify_vzdump_parameters($job, 1);
>
> - cfs_write_file('vzdump.cron', $data);
> + if ($schedule_updated) {
> + PVE::Jobs::updated_job_schedule($id, 'vzdump');
> + }
>
> - return undef;
> - }
> + if ($found) {
> + cfs_write_file('vzdump.cron', $data);
> }
> - raise_param_exc({ id => "No such job '$param->{id}'" });
> + cfs_write_file('jobs.cfg', $jobs_data); # FIXME LOCKING
Left-over reminder?
> +
> + return undef;
Nit: return is favorable over return undef (matters in list context).
> };
> - cfs_lock_file('vzdump.cron', undef, $update_job);
> + cfs_lock_file('vzdump.cron', undef, sub {
> + cfs_lock_file('jobs.cfg', undef, $update_job);
> + die "$@" if ($@);
> + });
> die "$@" if ($@);
> }});
>
> @@ -422,6 +568,13 @@ __PACKAGE__->register_method({
> last;
> }
> }
> + if (!$job) {
> + my $jobs_data = cfs_read_file('jobs.cfg');
> + my $j = $jobs_data->{ids}->{$param->{id}};
> + if ($j && $j->{type} eq 'vzdump') {
> + $job = $j;
> + }
> + }
> raise_param_exc({ id => "No such job '$param->{id}'" }) if !$job;
>
> my $vmlist = PVE::Cluster::get_vmlist();
> diff --git a/PVE/API2/Cluster/BackupInfo.pm b/PVE/API2/Cluster/BackupInfo.pm
> index 236e171d..101e45a7 100644
> --- a/PVE/API2/Cluster/BackupInfo.pm
> +++ b/PVE/API2/Cluster/BackupInfo.pm
> @@ -28,6 +28,16 @@ sub get_included_vmids {
> push @all_vmids, ( map { @{$_} } values %{$job_included_guests} );
> }
>
> + my $vzjobs = cfs_read_file('jobs.cfg');
> +
> + for my $id (keys %{$vzjobs->{ids}}) {
> + my $job = $vzjobs->{ids}->{$id};
Nit: loop could iterate over values rather than keys.
> + next if $job->{type} ne 'vzdump';
> +
> + my $job_included_guests = PVE::VZDump::get_included_guests($job);
> + push @all_vmids, ( map { @{$_} } values %{$job_included_guests} );
> + }
> +
> return { map { $_ => 1 } @all_vmids };
> }
>
>
More information about the pve-devel
mailing list