[pve-devel] [PATCH pve-manager v3 1/1] ui: fix #6209: create snapshots and backups from context menu

[Shannon Sterz]

some small comments in-line

On Mon Oct 6, 2025 at 3:33 PM CEST, Nicolas Frey wrote:
> Adds snapshot and manual backup shortcut to VM/CT right-click context
> menu.
>
> On the previous iteration (v2), the snapshot feature availablity was
> checked on menu load. It is now called only when the snapshot button
> is clicked, preventing delays in loading the context menu. Testing
> with simulated latency (with setTimeout) showed better UX by
> displaying an error on click rather than preloading the check.
>
> Previously, users couldn't distinguish between a slow API and the
> feature being disabled. The explicit check provides clear feedback.
> The 'disabled' property remains bound to VM.Snapshot to disable
> the option when snapshots aren't allowed anyway.
>
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6209
> Signed-off-by: Nicolas Frey <n.frey at proxmox.com>
> ---
>  www/manager6/lxc/CmdMenu.js  | 46 ++++++++++++++++++++++++++++++++++++
>  www/manager6/qemu/CmdMenu.js | 46 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 92 insertions(+)
>
> diff --git a/www/manager6/lxc/CmdMenu.js b/www/manager6/lxc/CmdMenu.js
> index cd60c967..f0e8f700 100644
> --- a/www/manager6/lxc/CmdMenu.js
> +++ b/www/manager6/lxc/CmdMenu.js
> @@ -134,6 +134,52 @@ Ext.define('PVE.lxc.CmdMenu', {
>                  },
>              },
>              { xtype: 'menuseparator' },
> +            {
> +                text: gettext('Take Snapshot'),
> +                iconCls: 'fa fa-fw fa-history',
> +                disabled: !caps.vms['VM.Snapshot'],
> +                handler: function () {
> +                    Proxmox.Utils.API2Request({
> +                        url: `/nodes/${info.node}/${info.type}/${info.vmid}/feature`,
> +                        params: { feature: 'snapshot' },
> +                        method: 'GET',
> +                        success: function (response) {

nit: you are mixing arrow style functions with anonymous functions here,
imo this should also just be:

(response) => {
    [..]
}

> +                            if (!response.result.data.hasFeature) {
> +                                Ext.Msg.alert(gettext('Error'), 'Cannot take snapshot: Feature not enabled.')

nit: did you run `make tidy`? for me it cleans the line above up to:

                                Ext.Msg.alert(
                                    gettext('Error'),
                                    'Cannot take snapshot: Feature not enabled.',
                                );

> +                                return;
> +                            }
> +                            Ext.create('PVE.window.Snapshot', {
> +                                nodename: info.node,
> +                                vmid: info.vmid,
> +                                vmname: info.name,
> +                                viewonly: false,
> +                                type: info.type,
> +                                isCreate: true,
> +                                submitText: gettext('Take Snapshot'),
> +                                autoShow: true,
> +                                running: running,
> +                            });
> +                        },
> +                        failure: (response, opts) =>
> +                            Ext.Msg.alert(gettext('Error'), response.htmlStatus),
> +                    });
> +                },
> +            },
> +            {
> +                text: gettext('Backup now'),
> +                iconCls: 'fa fa-fw fa-floppy-o',
> +                disabled: !caps.vms['VM.Backup'],
> +                handler: function () {

see comment about arrow function above

> +                    Ext.create('PVE.window.Backup', {
> +                        nodename: info.node,
> +                        vmid: info.vmid,
> +                        vmtype: info.type,
> +                        vmname: info.name,
> +                        autoShow: true,
> +                    });
> +                },
> +            },
> +            { xtype: 'menuseparator' },
>              {
>                  text: gettext('Console'),
>                  iconCls: 'fa fa-fw fa-terminal',
> diff --git a/www/manager6/qemu/CmdMenu.js b/www/manager6/qemu/CmdMenu.js
> index adf64672..3e51be8f 100644
> --- a/www/manager6/qemu/CmdMenu.js
> +++ b/www/manager6/qemu/CmdMenu.js
> @@ -169,6 +169,52 @@ Ext.define('PVE.qemu.CmdMenu', {
>                  },
>              },
>              { xtype: 'menuseparator' },
> +            {
> +                text: gettext('Take Snapshot'),
> +                iconCls: 'fa fa-fw fa-history',
> +                disabled: !caps.vms['VM.Snapshot'],
> +                handler: function () {
> +                    Proxmox.Utils.API2Request({
> +                        url: `/nodes/${info.node}/${info.type}/${info.vmid}/feature`,
> +                        params: { feature: 'snapshot' },
> +                        method: 'GET',
> +                        success: function (response) {

see comment about arrow function above

> +                            if (!response.result.data.hasFeature) {
> +                                Ext.Msg.alert(gettext('Error'), 'Cannot take snapshot: Feature not enabled.')

see comment about `make tidy` above

> +                                return;
> +                            }
> +                            Ext.create('PVE.window.Snapshot', {
> +                                nodename: info.node,
> +                                vmid: info.vmid,
> +                                vmname: info.name,
> +                                viewonly: false,
> +                                type: info.type,
> +                                isCreate: true,
> +                                submitText: gettext('Take Snapshot'),
> +                                autoShow: true,
> +                                running: running,
> +                            });
> +                        },
> +                        failure: (response, opts) =>
> +                            Ext.Msg.alert(gettext('Error'), response.htmlStatus),
> +                    });
> +                },
> +            },
> +            {
> +                text: gettext('Backup now'),
> +                iconCls: 'fa fa-fw fa-floppy-o',
> +                disabled: !caps.vms['VM.Backup'],
> +                handler: function () {

see comment about arrow function above

> +                    Ext.create('PVE.window.Backup', {
> +                        nodename: info.node,
> +                        vmid: info.vmid,
> +                        vmtype: info.type,
> +                        vmname: info.name,
> +                        autoShow: true,
> +                    });
> +                },
> +            },
> +            { xtype: 'menuseparator' },
>              {
>                  text: gettext('Console'),
>                  iconCls: 'fa fa-fw fa-terminal',

one small thought: have you explored whether querying the features of
the guest when opening the context menu and graying out the snapshot
option is viable? imo that would be a nicer user experience, but the
overhead of querying the backend there might be too much.

other than that:

Reviewed-by: Shannon Sterz <s.sterz at proxmox.com>