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

[Fiona Ebner <f.ebner@proxmox.com>]

Looks quite good already! And I like the addition of the entry for
backup too.

Am 30.09.25 um 3:01 PM schrieb Nicolas Frey:
> Adds snapshot and manual backup shortcut to VM/CT right-click context
> menu.
> 
> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6209
> 

Nit: please drop the blank line between the trailers

> Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
> ---
>  www/manager6/lxc/CmdMenu.js  | 45 ++++++++++++++++++++++++++++++++++++
>  www/manager6/qemu/CmdMenu.js | 45 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 90 insertions(+)
> 
> diff --git a/www/manager6/lxc/CmdMenu.js b/www/manager6/lxc/CmdMenu.js
> index cd60c967..6ab8b366 100644
> --- a/www/manager6/lxc/CmdMenu.js
> +++ b/www/manager6/lxc/CmdMenu.js
> @@ -30,6 +30,17 @@ Ext.define('PVE.lxc.CmdMenu', {
>              });
>          };
>  
> +        let snapshotFeature;
> +
> +        Proxmox.Utils.API2Request({
> +            url: `/nodes/${info.node}/${info.type}/${info.vmid}/feature`,
> +            params: { feature: 'snapshot' },
> +            method: 'GET',
> +            success: function (response, _) {
> +                snapshotFeature = !!response.result.data.hasFeature;

Since the API request is async, the success handler will only be called
after the snapshotFeature variable has already been read from in the
code below.

> +            },
> +        });
> +
>          let caps = Ext.state.Manager.get('GuiCap');
>          let standalone = PVE.Utils.isStandaloneNode();
>  
> @@ -134,6 +145,40 @@ Ext.define('PVE.lxc.CmdMenu', {
>                  },
>              },
>              { xtype: 'menuseparator' },
> +            {
> +                text: gettext('Take Snapshot'),
> +                iconCls: 'fa fa-fw fa-history',
> +                disabled: !!caps.vms['VM.Snapshot'] && snapshotFeature,

The condition should be negated

> +                handler: function () {
> +

Style nit: blank line here should be removed

> +                    let win = Ext.create('PVE.window.Snapshot', {
> +                        nodename: info.node,
> +                        vmid: info.vmid,
> +                        vmname: info.name,
> +                        viewonly: !caps.vms['VM.Snapshot'],

Nit: could also just be 'false' for easier readability (it's always an
isCreate, so it should never have viewonly)

> +                        type: info.type,
> +                        isCreate: true,
> +                        submitText: gettext('Take Snapshot'),
> +                        running: running,
> +                    });
> +                    win.show();
> +                },
> +            },
> +            {
> +                text: gettext('Backup now'),
> +                iconCls: 'fa fa-fw fa-floppy-o',
> +                disabled: !!caps.vms['VM.Snapshot'] && snapshotFeature,

This should rather check only for the backup capability

> +                handler: function () {
> +                    var win = Ext.create('PVE.window.Backup', {
> +                        nodename: info.node,
> +                        vmid: info.vmid,
> +                        vmtype: info.type,
> +                        vmname: info.name,
> +                    });
> +                    win.show();
> +                },
> +            },
> +            { 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 5fa9407d..5f9ba740 100644
> --- a/www/manager6/qemu/CmdMenu.js
> +++ b/www/manager6/qemu/CmdMenu.js
> @@ -31,6 +31,17 @@ Ext.define('PVE.qemu.CmdMenu', {

Same comments as for the lxc context menu apply for the VM context menu


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel