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 19D26D397 for ; Wed, 30 Nov 2022 14:46:19 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EE33E1E3B7 for ; Wed, 30 Nov 2022 14:46:18 +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 for ; Wed, 30 Nov 2022 14:46:18 +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 04BC343A90 for ; Wed, 30 Nov 2022 14:46:18 +0100 (CET) Message-ID: <1483d5e9-56fb-8373-c583-e376ce652ae9@proxmox.com> Date: Wed, 30 Nov 2022 14:46:16 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Content-Language: en-US To: pmg-devel@lists.proxmox.com, Stoiko Ivanov References: <20221130082746.58537-1-s.ivanov@proxmox.com> From: Fiona Ebner In-Reply-To: <20221130082746.58537-1-s.ivanov@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.157 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.258 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 Subject: Re: [pmg-devel] [PATCH pmg-api] backup: restore: keep relevant subdirs in /etc/pmg for inotify X-BeenThere: pmg-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Mail Gateway development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 30 Nov 2022 13:46:19 -0000 Am 30.11.22 um 09:27 schrieb Stoiko Ivanov: > By wiping the subdirectories in /etc/pmg/, we lose the inotify > watchers upon restore (/etc/pmg itself and thus most configs are > currently handled by the keep_root flag to rmtree) > This can lead to inconsistencies after restoring for parts relying on > config in a subdirectory (e.g. /etc/pmg/pbs/pbs.conf). > > This patch keeps a list of relevant config directories in place by > iterating over all entries and checking if they should be kept. > > an alternative approach would be to simply reload pmgdaemon/pmgproxy > upon config-restore, but that feels more likely to miss some > (potentially future) service, expecting inotify to work. > > Reported-by: Fiona Ebner Tested-by: Fiona Ebner > Signed-off-by: Stoiko Ivanov > --- > Huge Thanks to Fiona for not only reporting, but also analyzing the issue! > > src/PMG/Backup.pm | 31 ++++++++++++++++++++++++++++--- > 1 file changed, 28 insertions(+), 3 deletions(-) > > diff --git a/src/PMG/Backup.pm b/src/PMG/Backup.pm > index a46167c..71193e9 100644 > --- a/src/PMG/Backup.pm > +++ b/src/PMG/Backup.pm > @@ -8,7 +8,7 @@ use File::Path; > use POSIX qw(strftime); > > use PVE::JSONSchema qw(get_standard_option); > -use PVE::Tools; > +use PVE::Tools qw(dir_glob_foreach); > > use PMG::pmgcfg; > use PMG::AtomicFile; > @@ -306,8 +306,33 @@ sub pmg_restore { > unlink "$dirname/config/etc/pmg/cluster.conf"; # never restore cluster config > rmtree "$dirname/config/etc/pmg/master"; > > - # remove current config, but keep directory for INotify > - rmtree("/etc/pmg", { keep_root => 1 }); > + # remove current config, but keep directories for INotify > + my $keep_dirs = { > + acme => { accounts => 1 }, > + dkim => 1, > + pbs => 1, > + }; Style nit: indentation I wonder if we should have a helper for removing only files (or alternatively keeping all directories) from a directory structure, rather than passing explicitly what to keep? Otherwise, this list needs to be updated when the directory structure changes. Also might make sense to put it somewhere more central, as it likely will be needed when PVE gets host backup/restore support (for PBS) too? But can still be done when actually needed of course. > + > + my $remove_with_exclude; > + $remove_with_exclude = sub { > + my ($dir, $excl) = @_; > + dir_glob_foreach($dir, '.*', sub { > + my ($fn) = @_; > + return if $fn eq '.'; > + return if $fn eq '..'; > + if ( my $e = $excl->{$fn}) { > + if (ref($e)) { > + $remove_with_exclude->("$dir/$fn", $e); > + } else { > + rmtree("$dir/$fn", { keep_root => 1}); > + } > + } else { > + rmtree("$dir/$fn"); > + } > + }); > + }; > + $remove_with_exclude->('/etc/pmg', $keep_dirs); > + > # copy files > system("cp -a $dirname/config/etc/pmg/* /etc/pmg/") == 0 || > die "unable to restore system configuration: ERROR";