From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 244321FF191 for ; Tue, 7 Oct 2025 11:04:25 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9C91813E11; Tue, 7 Oct 2025 11:04:27 +0200 (CEST) Message-ID: <9563cc43-f8ad-4b33-b8ea-a22d22b3405e@proxmox.com> Date: Tue, 7 Oct 2025 11:03:53 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta From: Thomas Lamprecht References: <89366ef9-275a-46ef-b639-9b9d056f0d8e@proxmox.com> Content-Language: en-US To: Proxmox VE development discussion In-Reply-To: <89366ef9-275a-46ef-b639-9b9d056f0d8e@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1759827804594 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.027 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com] Subject: [pve-devel] [PATCH pve-manager v2 2/2] ui: fix #6209: create snapshots and backups from context menu X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox VE development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pve-devel-bounces@lists.proxmox.com Sender: "pve-devel" 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 > --- > 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 pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel