From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 65AD065747 for ; Mon, 7 Mar 2022 14:24:23 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5B2582801A for ; Mon, 7 Mar 2022 14:24:23 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id C19342800E for ; Mon, 7 Mar 2022 14:24:22 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 9418540AE6 for ; Mon, 7 Mar 2022 14:24:22 +0100 (CET) Message-ID: <76db8afe-e907-f94b-5616-8efb15551ff2@proxmox.com> Date: Mon, 7 Mar 2022 14:24:16 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Content-Language: en-US To: pve-devel@lists.proxmox.com, Hannes Laimer References: <79737db6-ec06-91bd-ac17-a441fe2c0591@proxmox.com> <20220307064308.10995-1-h.laimer@proxmox.com> From: Fabian Ebner In-Reply-To: <20220307064308.10995-1-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.125 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pve-devel] [PATCH pve-manager v2] fix #3903: jobs: add remove vmid from jobs helper X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Mar 2022 13:24:23 -0000 Am 07.03.22 um 07:43 schrieb Hannes Laimer: > Signed-off-by: Hannes Laimer > --- > changed back to v1, but without the unnecessary stuff. Thanks for the > feedback @Fabian Ebner > > PVE/Jobs.pm | 17 ++++++++++++++++- > 1 file changed, 16 insertions(+), 1 deletion(-) > > diff --git a/PVE/Jobs.pm b/PVE/Jobs.pm > index ba3685ec..db6fa97d 100644 > --- a/PVE/Jobs.pm > +++ b/PVE/Jobs.pm > @@ -4,7 +4,7 @@ use strict; > use warnings; > use JSON; > > -use PVE::Cluster qw(cfs_read_file cfs_lock_file); > +use PVE::Cluster qw(cfs_read_file cfs_lock_file cfs_write_file); > use PVE::Jobs::Plugin; > use PVE::Jobs::VZDump; > use PVE::Tools; > @@ -274,6 +274,21 @@ sub synchronize_job_states_with_config { > die $@ if $@; > } > > +sub remove_vmid_from_jobs { > + my ($vmid) = @_; > + > + cfs_lock_file('jobs.cfg', undef, sub { > + my $jobs_data = cfs_read_file('jobs.cfg'); > + for my $id (keys %{$jobs_data->{ids}}) { > + my $job = $jobs_data->{ids}->{$id}; > + next if !defined($job->{vmid}); > + $job->{vmid} = join(',', grep { $_ ne $vmid } PVE::Tools::split_list($job->{vmid})); > + delete $jobs_data->{ids}->{$id} if $job->{vmid} eq ''; There is a remove_job() function that's supposed to be called when a job is removed. It'll be called by synchronize_job_states_with_config too, but it'd be cleaner to call it directly. Also, the old behavior is to remove a VM ID upon purge from 'exclude' too. For consistency, we need do that here too. See remove_vmid_from_jobs in guest-common's PVE/VZDump/Plugin.pm for comparison. 'exclude' is specific to backups, so there should be a plugin method for removing a VMID from a job, which the VZDump plugin overrides, and the iterator here should just call the method from the job's plugin. Well, technically, 'vmid' is also specific to backups, because it's not part of the defaultData properties of the generic plugin. > + } > + cfs_write_file('jobs.cfg', $jobs_data); > + }); > +} > + > sub setup_dirs { > mkdir $state_dir; > mkdir $lock_dir;