From: Fiona Ebner <f.ebner@proxmox.com>
To: pmg-devel@lists.proxmox.com, Stoiko Ivanov <s.ivanov@proxmox.com>
Subject: Re: [pmg-devel] [PATCH pmg-api] backup: restore: keep relevant subdirs in /etc/pmg for inotify
Date: Wed, 30 Nov 2022 14:46:16 +0100 [thread overview]
Message-ID: <1483d5e9-56fb-8373-c583-e376ce652ae9@proxmox.com> (raw)
In-Reply-To: <20221130082746.58537-1-s.ivanov@proxmox.com>
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 <f.ebner@proxmox.com>
Tested-by: Fiona Ebner <f.ebner@proxmox.com>
> Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com>
> ---
> 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";
next prev parent reply other threads:[~2022-11-30 13:46 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-30 8:27 Stoiko Ivanov
2022-11-30 13:46 ` Fiona Ebner [this message]
2022-11-30 15:05 ` Stoiko Ivanov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1483d5e9-56fb-8373-c583-e376ce652ae9@proxmox.com \
--to=f.ebner@proxmox.com \
--cc=pmg-devel@lists.proxmox.com \
--cc=s.ivanov@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox