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

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

[Maximiliano Sandoval]

This was discussed off-list with Fiona.

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 then the old checkbox would dbe displayed
 - But delete the key from the config if the freeze is enabled, since it is the default

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.

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 check if undefined

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

-- 
2.47.3

[PATCH manager 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 2992f655..c71f56ab 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 2/7] ui: qga: merge translatable string

[Maximiliano Sandoval]

Having the whole string together gives more context (e.g. the gender of
Agent is relavent 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 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 4/7] ui: qga: note that freeze-fs-on-backup is deprecated

[Maximiliano Sandoval]

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

diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
index 43d83a39..e7fa929c 100644
--- a/www/manager6/form/AgentFeatureSelector.js
+++ b/www/manager6/form/AgentFeatureSelector.js
@@ -44,9 +44,10 @@ 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('Deprecated. Freeze/thaw guest filesystems on backup for consistency'),
             name: 'freeze-fs-on-backup',
             reference: 'freeze_fs_on_backup',
             bind: {
-- 
2.47.3

[PATCH manager 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 | 28 +++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
index e7fa929c..a3dda630 100644
--- a/www/manager6/form/AgentFeatureSelector.js
+++ b/www/manager6/form/AgentFeatureSelector.js
@@ -21,6 +21,28 @@ Ext.define('PVE.form.AgentFeatureSelector', {
             },
             disabled: true,
         },
+        {
+            xtype: 'proxmoxcheckbox',
+            boxLabel: gettext('Freeze/thaw guest filesystems 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.',
+            ),
+            bind: {
+                hidden: '{guest_fsfreeze.checked}',
+            },
+        },
         {
             xtype: 'displayfield',
             userCls: 'pmx-hint',
@@ -73,6 +95,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 };
@@ -83,6 +108,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 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 a3dda630..1a87182d 100644
--- a/www/manager6/form/AgentFeatureSelector.js
+++ b/www/manager6/form/AgentFeatureSelector.js
@@ -104,6 +104,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;
@@ -112,6 +114,6 @@ Ext.define('PVE.form.AgentFeatureSelector', {
             res['guest-fsfreeze'] = 1;
         }
 
-        this.callParent([res]);
+        me.callParent([res]);
     },
 });
-- 
2.47.3

[PATCH manager 7/7] ui: qga: hide freeze-fs-on-backup check if undefined

[Maximiliano Sandoval]

If the freeze-fs-on-backup key is not explicitly set on the config file
we hide the corresponding checkbox and hint.

Note that when the checkbox is unchecked we delete the value from the
config resulting.

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

diff --git a/www/manager6/form/AgentFeatureSelector.js b/www/manager6/form/AgentFeatureSelector.js
index 1a87182d..deb774c1 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: [
         {
@@ -71,9 +82,10 @@ Ext.define('PVE.form.AgentFeatureSelector', {
             xtype: 'proxmoxcheckbox',
             boxLabel: gettext('Deprecated. Freeze/thaw guest filesystems on backup for consistency'),
             name: 'freeze-fs-on-backup',
-            reference: 'freeze_fs_on_backup',
             bind: {
                 disabled: '{!enabled.checked}',
+                value: '{freezeFsOnBackupChecked}',
+                hidden: '{hideFreezeFsOnBackup}',
             },
             disabled: true,
             uncheckedValue: '0',
@@ -86,7 +98,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}',
             },
         },
     ],
@@ -105,10 +117,13 @@ Ext.define('PVE.form.AgentFeatureSelector', {
 
     setValues: function (values) {
         let me = this;
+        let vm = me.getViewModel();
 
         let res = PVE.Parser.parsePropertyString(values.agent, 'enabled');
         if (!Ext.isDefined(res['freeze-fs-on-backup'])) {
             res['freeze-fs-on-backup'] = 1;
+        } else {
+            vm.set('hideFreezeFsOnBackup', false);
         }
         if (!Ext.isDefined(res['guest-fsfreeze'])) {
             res['guest-fsfreeze'] = 1;
-- 
2.47.3

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

[Maximiliano Sandoval]

Maximiliano Sandoval <m.sandoval@proxmox.com> writes:

> This was discussed off-list with Fiona.
>
> 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 then the old checkbox would dbe displayed
>  - But delete the key from the config if the freeze is enabled, since it is the default

I forgot to mention here that the old key was moved to the advanced
options to prevent having two very similar rows together as it was
deemed to confusing.

-- 
Maximiliano