From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
Christoph Heiss <c.heiss@proxmox.com>
Subject: Re: [pve-devel] [PATCH qemu-server 1/3] vzdump: Add VM QGA option to skip `fs-freeze`/`fs-thaw` on backup
Date: Thu, 23 Feb 2023 11:55:55 +0100 [thread overview]
Message-ID: <e903a490-1ddb-b534-a6be-5bd9b1a43296@proxmox.com> (raw)
In-Reply-To: <20230201125918.372665-2-c.heiss@proxmox.com>
Am 01/02/2023 um 13:59 schrieb Christoph Heiss:
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> PVE/QemuServer.pm | 8 +++++++-
> PVE/VZDump/QemuServer.pm | 5 +++++
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index e4d1a70..e409db1 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -160,7 +160,13 @@ my $agent_fmt = {
> description => "Run fstrim after moving a disk or migrating the VM.",
> type => 'boolean',
> optional => 1,
> - default => 0
> + default => 0,
> + },
> + fsfreeze_thaw => {
Please rename this to:
'freeze-fs-on-backup'
> + description => "Freeze/thaw filesystems on backup for consistency.",
detail/nit: s/filesystems/guest filesystem/
> + type => 'boolean',
> + optional => 1,
> + default => 1,
> },
> type => {
> description => "Select the agent type",
> diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
> index 0eb5ec6..5ff46f7 100644
> --- a/PVE/VZDump/QemuServer.pm
> +++ b/PVE/VZDump/QemuServer.pm
> @@ -862,6 +862,11 @@ sub qga_fs_freeze {
> return;
> }
>
> + if (!PVE::QemuServer::get_qga_key($self->{vmlist}->{$vmid}, 'fsfreeze_thaw')) {
this is wrong as get_qga_key which uses parse_guest_agent which uses parse_property_string
doesn't sets the default from the schema on parse, so you'd also skip if it's not defined
(please negative test changes too)
Rather do something like:
my $freeze = PVE::QemuServer::get_qga_key($self->{vmlist}->{$vmid}, 'freeze-fs-on-backup') // 1;
if (!$freeze) {
# ...
> + $self->loginfo("skipping guest-agent 'fs-freeze', disabled in VM options");
> + return;
> + }
> +
> $self->loginfo("issuing guest-agent 'fs-freeze' command");
> eval { mon_cmd($vmid, "guest-fsfreeze-freeze") };
> $self->logerr($@) if $@;
next prev parent reply other threads:[~2023-02-23 10:55 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-01 12:59 [pve-devel] [PATCH qemu-server/manager/docs 0/3] fix #4140: Add option to disable `fs-freeze`/`-thaw` " Christoph Heiss
2023-02-01 12:59 ` [pve-devel] [PATCH qemu-server 1/3] vzdump: Add VM QGA option to skip `fs-freeze`/`fs-thaw` " Christoph Heiss
2023-02-23 10:55 ` Thomas Lamprecht [this message]
2023-02-23 11:29 ` Christoph Heiss
2023-02-01 12:59 ` [pve-devel] [PATCH manager 2/3] ui: qga: Add option to turn off fs-freeze/thaw for backup Christoph Heiss
2023-02-01 12:59 ` [pve-devel] [PATCH docs 3/3] qm: Add section explaining fs-freeze/thaw QGA option Christoph Heiss
2023-02-23 11:34 ` Thomas Lamprecht
2023-02-23 12:10 ` Christoph Heiss
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=e903a490-1ddb-b534-a6be-5bd9b1a43296@proxmox.com \
--to=t.lamprecht@proxmox.com \
--cc=c.heiss@proxmox.com \
--cc=pve-devel@lists.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal