[pve-devel] [PATCH v2 qemu-server/manager/docs 0/3] fix #4140: Add option to disable QGA fs-freeze/-thaw on backup

[pve-devel] [PATCH v2 qemu-server/manager/docs 0/3] fix #4140: Add option to disable QGA fs-freeze/-thaw on backup

[Christoph Heiss]

TL;DR: On Windows guests, the QEMU guest agent calls into the VSS
(Volume Shadow Copy Service) subsystem, which has potential to mess up
applications which hook into this themselves like SQL Server, as
described in the bug report [0].

Building upon the proposal in that report, add an (default-enabled)
option to the QGA configuration to disable the behavior manually if
really needed. Due to its potential to wreak havoc, show the user a
warning when disabled.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=4140

v1: https://lists.proxmox.com/pipermail/pve-devel/2023-February/055656.html

Christoph Heiss (1):
      vzdump: Add VM QGA option to skip fs-freeze/-thaw on backup

 PVE/QemuServer.pm        | 8 +++++++-
 PVE/VZDump/QemuServer.pm | 6 ++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

Christoph Heiss (1):
      ui: qga: Add option to turn off QGA fs-freeze/-thaw on backup

 www/manager6/Utils.js                     |  2 ++
 www/manager6/form/AgentFeatureSelector.js | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

Christoph Heiss (1):
      qm: Add section explaining fs-freeze/-thaw QGA option

 qm.adoc | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

--
2.39.1

[pve-devel] [PATCH v2 qemu-server 1/3] vzdump: Add VM QGA option to skip fs-freeze/-thaw on backup

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
 * Rename option from 'fsfreeze_thaw' to 'freeze-fs-on-backup'
 * Adapt option description as suggested
 * Fix option check in qga_fs_freeze()

 PVE/QemuServer.pm        | 8 +++++++-
 PVE/VZDump/QemuServer.pm | 6 ++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index e4d1a70..20bdb36 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,
+    },
+    'freeze-fs-on-backup' => {
+	description => "Freeze/thaw guest filesystems on backup for consistency.",
+	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..30baa46 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -862,6 +862,12 @@ sub qga_fs_freeze {
 	return;
     }

+    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 $@;
--
2.39.1

[pve-devel] applied: [PATCH v2 qemu-server 1/3] vzdump: Add VM QGA option to skip fs-freeze/-thaw on backup

[Thomas Lamprecht]

Am 23/02/2023 um 15:18 schrieb Christoph Heiss:
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> Changes v1 -> v2:
>  * Rename option from 'fsfreeze_thaw' to 'freeze-fs-on-backup'
>  * Adapt option description as suggested
>  * Fix option check in qga_fs_freeze()
> 
>  PVE/QemuServer.pm        | 8 +++++++-
>  PVE/VZDump/QemuServer.pm | 6 ++++++
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
>

applied, thanks!

FWIW, I forgot to write in my review of v1 that having the disabled-in-config
check done before the is-qga-running one would have felt _slightly_ more sensible
to me, as no use in checking for availability of a service if one isn't going
to use it anyway - but, this is only a small nit and could be easily addressed
anytime - so mostly mentioning it to get it off my mind ;-)

[pve-devel] [PATCH v2 manager 2/3] ui: qga: Add option to turn off QGA fs-freeze/-thaw on backup

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
 * Rename option from 'fsfreeze_thaw' to 'freeze-fs-on-backup'
 * Adapt option descriptions as suggested

 www/manager6/Utils.js                     |  2 ++
 www/manager6/form/AgentFeatureSelector.js | 31 ++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index d8c0bf5a..e6c7861a 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -475,6 +475,8 @@ Ext.define('PVE.Utils', {
 		    virtio: "VirtIO",
 		};
 		displayText = map[value] || Proxmox.Utils.unknownText;
+	    } else if (key === 'freeze-fs-on-backup' && PVE.Parser.parseBoolean(value)) {
+		continue;
 	    } else if (PVE.Parser.parseBoolean(value)) {
 		displayText = Proxmox.Utils.enabledText;
 	    }
diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
index 0dcc6ecb..651b7124 100644
--- a/www/manager6/form/AgentFeatureSelector.js
+++ b/www/manager6/form/AgentFeatureSelector.js
@@ -21,6 +21,27 @@ Ext.define('PVE.form.AgentFeatureSelector', {
 	    },
 	    disabled: true,
 	},
+	{
+	    xtype: 'proxmoxcheckbox',
+	    boxLabel: gettext('Freeze/thaw guest filesystems on backup for consistency'),
+	    name: 'freeze-fs-on-backup',
+	    reference: 'freeze_fs_on_backup',
+	    bind: {
+		disabled: '{!enabled.checked}',
+	    },
+	    disabled: true,
+	    uncheckedValue: '0',
+	    defaultValue: '1',
+	},
+	{
+	    xtype: 'displayfield',
+	    userCls: 'pmx-hint',
+	    value: gettext('Freeze/thaw for guest filesystems disabled. '
+		+ 'This can lead to inconsistent disk backups.'),
+	    bind: {
+		hidden: '{freeze_fs_on_backup.checked}',
+	    },
+	},
 	{
 	    xtype: 'displayfield',
 	    userCls: 'pmx-hint',
@@ -47,12 +68,20 @@ Ext.define('PVE.form.AgentFeatureSelector', {
     ],

     onGetValues: function(values) {
-	var agentstr = PVE.Parser.printPropertyString(values, 'enabled');
+	if (PVE.Parser.parseBoolean(values['freeze-fs-on-backup'])) {
+	    delete values['freeze-fs-on-backup'];
+	}
+
+	let agentstr = PVE.Parser.printPropertyString(values, 'enabled');
 	return { agent: agentstr };
     },

     setValues: function(values) {
 	let res = PVE.Parser.parsePropertyString(values.agent, 'enabled');
+	if (!Ext.isDefined(res['freeze-fs-on-backup'])) {
+	    res['freeze-fs-on-backup'] = 1;
+	}
+
 	this.callParent([res]);
     },
 });
--
2.39.1

Re: [pve-devel] [PATCH v2 manager 2/3] ui: qga: Add option to turn off QGA fs-freeze/-thaw on backup

[Thomas Lamprecht]

Am 23/02/2023 um 15:18 schrieb Christoph Heiss:
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> Changes v1 -> v2:
>  * Rename option from 'fsfreeze_thaw' to 'freeze-fs-on-backup'
>  * Adapt option descriptions as suggested
> 
>  www/manager6/Utils.js                     |  2 ++
>  www/manager6/form/AgentFeatureSelector.js | 31 ++++++++++++++++++++++-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
> index d8c0bf5a..e6c7861a 100644
> --- a/www/manager6/Utils.js
> +++ b/www/manager6/Utils.js
> @@ -475,6 +475,8 @@ Ext.define('PVE.Utils', {
>  		    virtio: "VirtIO",
>  		};
>  		displayText = map[value] || Proxmox.Utils.unknownText;
> +	    } else if (key === 'freeze-fs-on-backup' && PVE.Parser.parseBoolean(value)) {
> +		continue;
>  	    } else if (PVE.Parser.parseBoolean(value)) {
>  		displayText = Proxmox.Utils.enabledText;
>  	    }
> diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
> index 0dcc6ecb..651b7124 100644
> --- a/www/manager6/form/AgentFeatureSelector.js
> +++ b/www/manager6/form/AgentFeatureSelector.js
> @@ -21,6 +21,27 @@ Ext.define('PVE.form.AgentFeatureSelector', {
>  	    },
>  	    disabled: true,
>  	},
> +	{
> +	    xtype: 'proxmoxcheckbox',
> +	    boxLabel: gettext('Freeze/thaw guest filesystems on backup for consistency'),
> +	    name: 'freeze-fs-on-backup',
> +	    reference: 'freeze_fs_on_backup',
> +	    bind: {
> +		disabled: '{!enabled.checked}',
> +	    },
> +	    disabled: true,
> +	    uncheckedValue: '0',
> +	    defaultValue: '1',
> +	},
> +	{
> +	    xtype: 'displayfield',
> +	    userCls: 'pmx-hint',
> +	    value: gettext('Freeze/thaw for guest filesystems disabled. '
> +		+ 'This can lead to inconsistent disk backups.'),

gettext need to be in a single line - or fix the "parser" that scans for the
messages in i18n ;-)

> +	    bind: {
> +		hidden: '{freeze_fs_on_backup.checked}',
> +	    },
> +	},
>  	{
>  	    xtype: 'displayfield',
>  	    userCls: 'pmx-hint',
> @@ -47,12 +68,20 @@ Ext.define('PVE.form.AgentFeatureSelector', {
>      ],
> 
>      onGetValues: function(values) {
> -	var agentstr = PVE.Parser.printPropertyString(values, 'enabled');
> +	if (PVE.Parser.parseBoolean(values['freeze-fs-on-backup'])) {
> +	    delete values['freeze-fs-on-backup'];
> +	}
> +
> +	let agentstr = PVE.Parser.printPropertyString(values, 'enabled');
>  	return { agent: agentstr };
>      },
> 
>      setValues: function(values) {
>  	let res = PVE.Parser.parsePropertyString(values.agent, 'enabled');
> +	if (!Ext.isDefined(res['freeze-fs-on-backup'])) {
> +	    res['freeze-fs-on-backup'] = 1;
> +	}
> +
>  	this.callParent([res]);
>      },
>  });
> --
> 2.39.1
> 
> 
> 

Re: [pve-devel] [PATCH v2 manager 2/3] ui: qga: Add option to turn off QGA fs-freeze/-thaw on backup

[Christoph Heiss]

On Tue, Mar 21, 2023 at 01:31:44PM +0100, Thomas Lamprecht wrote:
> Am 23/02/2023 um 15:18 schrieb Christoph Heiss:
> > [..]
> > +	{
> > +	    xtype: 'displayfield',
> > +	    userCls: 'pmx-hint',
> > +	    value: gettext('Freeze/thaw for guest filesystems disabled. '
> > +		+ 'This can lead to inconsistent disk backups.'),
>
> gettext need to be in a single line - or fix the "parser" that scans for the
> messages in i18n ;-)
Okay, thanks for the hint! Guess I'll drop the the line wrapping for now.
(But the latter would be nice too as a kind of "future-proofing" I
suppose, I'll probably take a shot at it ..)

>
> > [..]

[pve-devel] [PATCH v2 docs 3/3] qm: Add section explaining fs-freeze/-thaw QGA option

[Christoph Heiss]

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v1 -> v2:
 * Incorporate suggestions made by Thomas

 qm.adoc | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/qm.adoc b/qm.adoc
index 8a49283..93ec29d 100644
--- a/qm.adoc
+++ b/qm.adoc
@@ -1061,6 +1061,28 @@ operations that have the potential to write out zeros to the storage:

 On a thin provisioned storage, this can help to free up unused space.

+Filesystem Freeze & Thaw on Backup
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+By default, guest filesystems are synced via the 'fs-freeze' QEMU Guest Agent
+Command when a backup is performed, to provide consistency.
+
+On Windows guests, some applications might handle consistent backups themselves
+by hooking into the Windows VSS (Volume Shadow Copy Service) layer, a
+'fs-freeze' then might interfere with that. For example, it has been observed
+that calling 'fs-freeze' with some SQL Servers triggers VSS to call the SQL
+Writer VSS module in a mode that breaks the SQL Server backup chain for
+differential backups.
+
+For such setups you can configure {pve} to not issue a freeze-and-thaw cycle on
+backup by setting the `freeze-fs-on-backup` QGA option to `0`. This can also be
+done via the GUI with the 'Freeze/thaw guest filesystems on backup for
+consistency' option.
+
+NOTE: Disabling this option can potentially lead to backups with inconsistent
+filesystems and should therefore only be disabled if you know what you are
+doing.
+
 Troubleshooting
 ^^^^^^^^^^^^^^^

--
2.39.1

[pve-devel] applied: [PATCH v2 docs 3/3] qm: Add section explaining fs-freeze/-thaw QGA option

[Thomas Lamprecht]

Am 23/02/2023 um 15:18 schrieb Christoph Heiss:
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
> Changes v1 -> v2:
>  * Incorporate suggestions made by Thomas
> 
>  qm.adoc | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
>

applied, with a trivial merge conflict due to additions in the context addressed, thanks!