[PATCH manager v2 0/7] ui: qga: add checkbox for guest-fsfreeze

[PATCH manager v2 0/7] ui: qga: add checkbox for guest-fsfreeze

[Maximiliano Sandoval]

Something that was missing from Fiona's comments was giving the "hint"
CSS class to the "Deprecated" part of the old checkbox's label. I was
not sure how to make that work in ExtJS.

The migration would work as follows:

 - The new key takes preference over the old one so using the old one is OK.
 - If the key is in the config file and explicitly disabled then the old
   checkbox would be displayed
 - But we delete the deprecated key from the config if it was explicitly
   enabled (it is the default) after performing any change

One could additionally do some extra work where setting the new key will also
set the old value, but for the sake of simplicity I think this is a good start.

I added to my TODO list to add a migration check to the pve9to10 script.

Differences from v1:
 - Rephrased the user-facing strings
 - Fix a typo in a commit message
 - Expand some commit messages
 - Hide the checkbox not only if it was undefined, but also if it was
   explicitly set to the default value

Maximiliano Sandoval (7):
  ui: utils: support guest-fsfreeze at render_qga_features
  ui: qga: merge translatable string
  ui: qga: move freeze-fs-on-backup to advanced opts
  ui: qga: note that freeze-fs-on-backup is deprecated
  ui: qga: add checkbox for guest-fsfreeze
  ui: qga: call parameter me instead of this
  ui: qga: hide freeze-fs-on-backup conditionally

 www/manager6/Utils.js                     |  2 +
 www/manager6/form/AgentFeatureSelector.js | 72 ++++++++++++++++++++---
 2 files changed, 66 insertions(+), 8 deletions(-)

-- 
2.47.3

[PATCH manager v2 1/7] ui: utils: support guest-fsfreeze at render_qga_features

[Maximiliano Sandoval]

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 www/manager6/Utils.js | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index bb49a5bc..77dae42e 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -489,6 +489,8 @@ Ext.define('PVE.Utils', {
                     displayText = map[value] || Proxmox.Utils.unknownText;
                 } else if (key === 'freeze-fs-on-backup' && PVE.Parser.parseBoolean(value)) {
                     continue;
+                } else if (key === 'guest-fsfreeze' && PVE.Parser.parseBoolean(value)) {
+                    continue;
                 } else if (PVE.Parser.parseBoolean(value)) {
                     displayText = Proxmox.Utils.enabledText;
                 }
-- 
2.47.3

[PATCH manager v2 2/7] ui: qga: merge translatable string

[Maximiliano Sandoval]

Having the whole string together gives more context (e.g. the gender of
Agent is relevant in many languages) to translators.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 www/manager6/form/AgentFeatureSelector.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
index 2a485f08..a4678b87 100644
--- a/www/manager6/form/AgentFeatureSelector.js
+++ b/www/manager6/form/AgentFeatureSelector.js
@@ -7,7 +7,7 @@ Ext.define('PVE.form.AgentFeatureSelector', {
     items: [
         {
             xtype: 'proxmoxcheckbox',
-            boxLabel: Ext.String.format(gettext('Use {0}'), 'QEMU Guest Agent'),
+            boxLabel: gettext('Use QEMU Guest Agent'),
             name: 'enabled',
             reference: 'enabled',
             uncheckedValue: 0,
-- 
2.47.3

[PATCH manager v2 3/7] ui: qga: move freeze-fs-on-backup to advanced opts

[Maximiliano Sandoval]

This is going to be replaced by the replacement guest-fsfreeze setting.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 www/manager6/form/AgentFeatureSelector.js | 44 +++++++++++------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
index a4678b87..43d83a39 100644
--- a/www/manager6/form/AgentFeatureSelector.js
+++ b/www/manager6/form/AgentFeatureSelector.js
@@ -21,28 +21,6 @@ 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',
@@ -66,6 +44,28 @@ Ext.define('PVE.form.AgentFeatureSelector', {
                 ['isa', 'ISA'],
             ],
         },
+        {
+            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}',
+            },
+        },
     ],
 
     onGetValues: function (values) {
-- 
2.47.3

[PATCH manager v2 4/7] ui: qga: note that freeze-fs-on-backup is deprecated

[Maximiliano Sandoval]

The lenght of this string is offset by the entire row being shown
conditionally.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 www/manager6/form/AgentFeatureSelector.js | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
index 43d83a39..b64837b7 100644
--- a/www/manager6/form/AgentFeatureSelector.js
+++ b/www/manager6/form/AgentFeatureSelector.js
@@ -44,9 +44,12 @@ Ext.define('PVE.form.AgentFeatureSelector', {
                 ['isa', 'ISA'],
             ],
         },
+        // TODO Remove these two items with Proxmox VE 10.
         {
             xtype: 'proxmoxcheckbox',
-            boxLabel: gettext('Freeze/thaw guest filesystems on backup for consistency'),
+            boxLabel: gettext(
+                'Freeze/thaw guest filesystems on backup for consistency. Deprecated in favor of the more general setting.',
+            ),
             name: 'freeze-fs-on-backup',
             reference: 'freeze_fs_on_backup',
             bind: {
-- 
2.47.3

[PATCH manager v2 5/7] ui: qga: add checkbox for guest-fsfreeze

[Maximiliano Sandoval]

We re-use the same components from the 'freeze-fs-on-backup' checkbox
but without referencing that this only affects backups.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 www/manager6/form/AgentFeatureSelector.js | 30 +++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
index b64837b7..36e5bd8a 100644
--- a/www/manager6/form/AgentFeatureSelector.js
+++ b/www/manager6/form/AgentFeatureSelector.js
@@ -21,6 +21,30 @@ Ext.define('PVE.form.AgentFeatureSelector', {
             },
             disabled: true,
         },
+        {
+            xtype: 'proxmoxcheckbox',
+            boxLabel: gettext(
+                'Freeze/thaw guest filesystems during certain operations for consistency',
+            ),
+            name: 'guest-fsfreeze',
+            reference: 'guest_fsfreeze',
+            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 images after performing certain operations.',
+            ),
+            bind: {
+                hidden: '{guest_fsfreeze.checked}',
+            },
+        },
         {
             xtype: 'displayfield',
             userCls: 'pmx-hint',
@@ -75,6 +99,9 @@ Ext.define('PVE.form.AgentFeatureSelector', {
         if (PVE.Parser.parseBoolean(values['freeze-fs-on-backup'])) {
             delete values['freeze-fs-on-backup'];
         }
+        if (PVE.Parser.parseBoolean(values['guest-fsfreeze'])) {
+            delete values['guest-fsfreeze'];
+        }
 
         const agentstr = PVE.Parser.printPropertyString(values, 'enabled');
         return { agent: agentstr };
@@ -85,6 +112,9 @@ Ext.define('PVE.form.AgentFeatureSelector', {
         if (!Ext.isDefined(res['freeze-fs-on-backup'])) {
             res['freeze-fs-on-backup'] = 1;
         }
+        if (!Ext.isDefined(res['guest-fsfreeze'])) {
+            res['guest-fsfreeze'] = 1;
+        }
 
         this.callParent([res]);
     },
-- 
2.47.3

[PATCH manager v2 6/7] ui: qga: call parameter me instead of this

[Maximiliano Sandoval]

This is how it is generally done, we will use `me` in the next commit.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 www/manager6/form/AgentFeatureSelector.js | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
index 36e5bd8a..39ee211a 100644
--- a/www/manager6/form/AgentFeatureSelector.js
+++ b/www/manager6/form/AgentFeatureSelector.js
@@ -108,6 +108,8 @@ Ext.define('PVE.form.AgentFeatureSelector', {
     },
 
     setValues: function (values) {
+        let me = this;
+
         let res = PVE.Parser.parsePropertyString(values.agent, 'enabled');
         if (!Ext.isDefined(res['freeze-fs-on-backup'])) {
             res['freeze-fs-on-backup'] = 1;
@@ -116,6 +118,6 @@ Ext.define('PVE.form.AgentFeatureSelector', {
             res['guest-fsfreeze'] = 1;
         }
 
-        this.callParent([res]);
+        me.callParent([res]);
     },
 });
-- 
2.47.3

[PATCH manager v2 7/7] ui: qga: hide freeze-fs-on-backup conditionally

[Maximiliano Sandoval]

If the freeze-fs-on-backup key is not explicitly set on the config file
or if it is explicitly enabled (which matches its default) we hide the
corresponding checkbox and hint.

Note that when the checkbox is unchecked we delete the value from the
config. A consequence of this is that if the deprecated key is
explicitly set `freeze-fs-on-backup=1`, we open the dialog, change some
unrelated setting (so that the OK button becomes sensitive), press OK to
save the settings, then the deprecated key freeze-fs-on-backup key would
be gone from the config.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 www/manager6/form/AgentFeatureSelector.js | 27 ++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
index 39ee211a..51d9d94e 100644
--- a/www/manager6/form/AgentFeatureSelector.js
+++ b/www/manager6/form/AgentFeatureSelector.js
@@ -2,7 +2,18 @@ Ext.define('PVE.form.AgentFeatureSelector', {
     extend: 'Proxmox.panel.InputPanel',
     alias: ['widget.pveAgentFeatureSelector'],
 
-    viewModel: {},
+    viewModel: {
+        data: {
+            hideFreezeFsOnBackup: true,
+            freezeFsOnBackupChecked: true,
+        },
+
+        formulas: {
+            hideFreezeFsOnBackupNotes: function (get) {
+                return get('freezeFsOnBackupChecked') || get('hideFreezeFsOnBackup');
+            },
+        },
+    },
 
     items: [
         {
@@ -75,9 +86,10 @@ Ext.define('PVE.form.AgentFeatureSelector', {
                 'Freeze/thaw guest filesystems on backup for consistency. Deprecated in favor of the more general setting.',
             ),
             name: 'freeze-fs-on-backup',
-            reference: 'freeze_fs_on_backup',
             bind: {
                 disabled: '{!enabled.checked}',
+                value: '{freezeFsOnBackupChecked}',
+                hidden: '{hideFreezeFsOnBackup}',
             },
             disabled: true,
             uncheckedValue: '0',
@@ -90,7 +102,7 @@ Ext.define('PVE.form.AgentFeatureSelector', {
                 'Freeze/thaw for guest filesystems disabled. This can lead to inconsistent disk backups.',
             ),
             bind: {
-                hidden: '{freeze_fs_on_backup.checked}',
+                hidden: '{hideFreezeFsOnBackupNotes}',
             },
         },
     ],
@@ -109,8 +121,17 @@ Ext.define('PVE.form.AgentFeatureSelector', {
 
     setValues: function (values) {
         let me = this;
+        let vm = me.getViewModel();
 
         let res = PVE.Parser.parsePropertyString(values.agent, 'enabled');
+
+        // We hide the switch for the deprecated freeze-fs-on-backup if the setting was not
+        // explicitly set by the user or if was explicitly enabled.
+        vm.set(
+            'hideFreezeFsOnBackup',
+            !Ext.isDefined(res['freeze-fs-on-backup']) || res['freeze-fs-on-backup'] === '1',
+        );
+
         if (!Ext.isDefined(res['freeze-fs-on-backup'])) {
             res['freeze-fs-on-backup'] = 1;
         }
-- 
2.47.3