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 A0F4964350 for ; Wed, 2 Mar 2022 11:13:23 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9E04B28088 for ; Wed, 2 Mar 2022 11:13: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 5EAEF28041 for ; Wed, 2 Mar 2022 11:13: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 3783141CDE for ; Wed, 2 Mar 2022 11:13:22 +0100 (CET) Message-ID: Date: Wed, 2 Mar 2022 11:12:45 +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, h.laimer@proxmox.com References: <20220301085153.31445-1-h.laimer@proxmox.com> <20220301085153.31445-2-h.laimer@proxmox.com> From: Fabian Ebner In-Reply-To: <20220301085153.31445-2-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.129 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 1/3] fix #3903: jobs-plugin: 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: Wed, 02 Mar 2022 10:13:23 -0000 Am 01.03.22 um 09:51 schrieb Hannes Laimer: > Signed-off-by: Hannes Laimer > --- > PVE/Jobs/Plugin.pm | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/PVE/Jobs/Plugin.pm b/PVE/Jobs/Plugin.pm > index 6098360b..4883a193 100644 > --- a/PVE/Jobs/Plugin.pm > +++ b/PVE/Jobs/Plugin.pm > @@ -3,7 +3,7 @@ package PVE::Jobs::Plugin; > use strict; > use warnings; > > -use PVE::Cluster qw(cfs_register_file); > +use PVE::Cluster qw(cfs_register_file cfs_lock_file cfs_read_file cfs_write_file); > > use base qw(PVE::SectionConfig); > > @@ -92,6 +92,23 @@ sub write_config { > $class->SUPER::write_config($filename, $cfg); > } > > +sub remove_vmid_from_jobs { Since this is iterating over jobs of all kinds and not something that should be overridden by derived plugins, it should rather be part of Jobs.pm. If, in the future, plugins ever need more fine-grained control, we can always add a helper method to remove a vmid from a single job and adapt this function to call the helper method from the appropriate plugin for each job. > + my ($vmid) = @_; Style nit: we usually put a blank line after the parameter assignment. > + cfs_lock_file('jobs.cfg', undef, sub { > + my $jobs_data = cfs_read_file('jobs.cfg'); > + while((my $id, my $job) = each (%{$jobs_data->{ids}})){ Style nits: * We don't really use 'each'. One reason is that the state of the iterator is not reset when returning early. Not relevant here, but it's just not nice behavior. * Missing spaces before/after outermost parentheses > + next if !defined($job->{vmid}); > + $job->{vmid} = join(',', grep { $_ ne $vmid } PVE::Tools::split_list($job->{vmid})); > + if ($job->{vmid} eq '') { > + delete $jobs_data->{ids}->{$id}; > + } else { > + $jobs_data->{ids}->{$id} = $job; Isn't this just assigning the same reference again? > + } > + } > + cfs_write_file('jobs.cfg', $jobs_data); > + }); > +} > + > sub run { > my ($class, $cfg) = @_; > # implement in subclass