re-sending this previous review as I forgot to CC the list when I sent this on 2025-10-03...
Am 02.10.25 um 17:15 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 > Signed-off-by: Nicolas Frey <[email protected]> > --- > www/manager6/lxc/CmdMenu.js | 47 ++++++++++++++++++++++++++++++++++++ > www/manager6/qemu/CmdMenu.js | 47 ++++++++++++++++++++++++++++++++++++ > 2 files changed, 94 insertions(+) > > diff --git a/www/manager6/lxc/CmdMenu.js b/www/manager6/lxc/CmdMenu.js > index cd60c967..bd5fe164 100644 > --- a/www/manager6/lxc/CmdMenu.js > +++ b/www/manager6/lxc/CmdMenu.js > @@ -134,6 +134,40 @@ Ext.define('PVE.lxc.CmdMenu', { > }, > }, > { xtype: 'menuseparator' }, > + { > + text: gettext('Take Snapshot'), > + iconCls: 'fa fa-fw fa-history', > + itemId: 'takeSnapshotBtn', > + disabled: true, // default until async check finishes > + handler: function () { > + let win = Ext.create('PVE.window.Snapshot', { You can use: autoShow: true, here and avoid the need for the extra call to .show() and thus the win variable. > + nodename: info.node, > + vmid: info.vmid, > + vmname: info.name, > + viewonly: false, > + 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.Backup'], > + handler: function () { > + var win = Ext.create('PVE.window.Backup', { same w.r.t. using autoShow > + 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', > @@ -149,5 +183,18 @@ Ext.define('PVE.lxc.CmdMenu', { > ]; > > me.callParent(); > + > + Proxmox.Utils.API2Request({ > + url: `/nodes/${info.node}/${info.type}/${info.vmid}/feature`, > + params: { feature: 'snapshot' }, > + method: 'GET', > + success: function (response) { > + let hasFeature = !!response.result.data.hasFeature; > + let btn = me.down('#takeSnapshotBtn'); > + if (btn) { > + btn.setDisabled(!(caps.vms['VM.Snapshot'] && > hasFeature)); You could skip doing the API call if caps.vms['VM.Snapshot'] is false as optimization, as no point in checking in that case. And while right-click is something a user explicitly needs to do, I'm not sure if it's really worth doing an extra API call every time the menu is opened though, while this is mostly iterating all volumes in the config and checking the storage plugin, it might have a noticeable delay on any non-local PVE, especially on spotty connections. It's a tiny bit worse UX, but as trade-off we could only check if one clicks this menu entry and then show a message or the like. If users complain about UX we can still think about something else. What do you think? FYI, I can be fine with doing it your proposed way, but for these things I'd like to have other potential options at least explored, and having some rationale for why it's fine to do this api call on context menu open in the encoded in the commit message would be definitively warranted. > + } > + }, > + }); > }, > }); > diff --git a/www/manager6/qemu/CmdMenu.js b/www/manager6/qemu/CmdMenu.js > index adf64672..7c9080c5 100644 > --- a/www/manager6/qemu/CmdMenu.js > +++ b/www/manager6/qemu/CmdMenu.js > @@ -169,6 +169,40 @@ Ext.define('PVE.qemu.CmdMenu', { > }, > }, > { xtype: 'menuseparator' }, > + { > + text: gettext('Take Snapshot'), > + iconCls: 'fa fa-fw fa-history', > + itemId: 'takeSnapshotBtn', > + disabled: true, > + handler: function () { > + let win = 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'), > + running: running, > + }); > + win.show(); same w.r.t. using autoShow > + }, > + }, > + { > + text: gettext('Backup now'), > + iconCls: 'fa fa-fw fa-floppy-o', > + disabled: !caps.vms['VM.Backup'], > + handler: function () { > + var win = Ext.create('PVE.window.Backup', { > + nodename: info.node, > + vmid: info.vmid, > + vmtype: info.type, > + vmname: info.name, > + }); > + win.show(); same w.r.t. using autoShow > + }, > + }, > + { xtype: 'menuseparator' }, > { > text: gettext('Console'), > iconCls: 'fa fa-fw fa-terminal', > @@ -194,5 +228,18 @@ Ext.define('PVE.qemu.CmdMenu', { > ]; > > me.callParent(); > + > + Proxmox.Utils.API2Request({ > + url: `/nodes/${info.node}/${info.type}/${info.vmid}/feature`, > + params: { feature: 'snapshot' }, > + method: 'GET', > + success: function (response) { > + let hasFeature = !!response.result.data.hasFeature; > + let btn = me.down('#takeSnapshotBtn'); > + if (btn) { > + btn.setDisabled(!(caps.vms['VM.Snapshot'] && > hasFeature)); > + } > + }, > + }); > }, > }); _______________________________________________ pve-devel mailing list [email protected] https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
