From: Friedrich Weber <f.weber@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH manager v3 5/5] fix #4474: ui: guest stop: offer to overrule active shutdown tasks
Date: Fri, 12 Apr 2024 16:15:53 +0200 [thread overview]
Message-ID: <20240412141553.430554-6-f.weber@proxmox.com> (raw)
In-Reply-To: <20240412141553.430554-1-f.weber@proxmox.com>
Implement a new "guest stop" confirmation message box which first
checks if there is an active shutdown task for the same guest that is
visible to the logged-in user. If there is at least one, the dialog
displays an additional default-on checkbox for overruling active
shutdown tasks. If the user confirms and the checkbox is checked, the
UI sends a guest stop API request with the `overrule-shutdown`
parameter set to 1. If there are no active shutdown tasks, or the
checkbox is unchecked, the UI sends a guest stop API request without
`overrule-shutdown`.
To avoid an additional API request for querying active shutdown tasks,
check the UI's current view of cluster tasks instead, which is fetched
from the `pve-cluster-tasks` store.
As the UI might hold an outdated task list, there are some
opportunities for races, e.g., the UI may miss a new shutdown task or
consider a shutdown task active even though it has already terminated.
These races either result in a surviving shutdown task that the user
still needs to abort manually, or a superfluous `override-shutdown=1`
parameter that does not actually abort any tasks. Since "stop
overrules shutdown" is merely a convenience feature, both outcomes
seem bearable.
The confirmation message box is now always marked as dangerous (with a
warning sign icon), whereas previously it was only marked dangerous if
the stop issued from the guest panel, but not when issued from the
resource tree command menu.
Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
---
Notes:
changes v2 -> v3:
- aligned permission checks with changed backend logic
- replace access of `this.vm` with `me.vm`
changes v1 -> v2:
- instead of a second modal dialog that offers to overrule shutdown
tasks, display an additional checkbox in the "guest stop"
confirmation dialog if there is a running matching shutdown task
- spin out pve-cluster-tasks store fix into its own patch
www/manager6/Makefile | 1 +
| 8 +++-
www/manager6/lxc/Config.js | 8 ++--
| 8 +++-
www/manager6/qemu/Config.js | 8 ++--
www/manager6/window/GuestStop.js | 79 ++++++++++++++++++++++++++++++++
6 files changed, 104 insertions(+), 8 deletions(-)
create mode 100644 www/manager6/window/GuestStop.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index c756cae6..30c56bb9 100644
--- a/www/manager6/Makefile
+++ b/www/manager6/Makefile
@@ -131,6 +131,7 @@ JSSRC= \
window/ScheduleSimulator.js \
window/Wizard.js \
window/GuestDiskReassign.js \
+ window/GuestStop.js \
window/TreeSettingsEdit.js \
window/PCIMapEdit.js \
window/USBMapEdit.js \
--git a/www/manager6/lxc/CmdMenu.js b/www/manager6/lxc/CmdMenu.js
index b1403fc6..76b39423 100644
--- a/www/manager6/lxc/CmdMenu.js
+++ b/www/manager6/lxc/CmdMenu.js
@@ -66,7 +66,13 @@ Ext.define('PVE.lxc.CmdMenu', {
iconCls: 'fa fa-fw fa-stop',
disabled: stopped,
tooltip: Ext.String.format(gettext('Stop {0} immediately'), 'CT'),
- handler: () => confirmedVMCommand('stop'),
+ handler: () => {
+ Ext.create('PVE.GuestStop', {
+ nodename: info.node,
+ vm: info,
+ autoShow: true,
+ });
+ },
},
{
text: gettext('Reboot'),
diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js
index 4516ee8f..082913e1 100644
--- a/www/manager6/lxc/Config.js
+++ b/www/manager6/lxc/Config.js
@@ -77,11 +77,13 @@ Ext.define('PVE.lxc.Config', {
{
text: gettext('Stop'),
disabled: !caps.vms['VM.PowerMgmt'],
- confirmMsg: Proxmox.Utils.format_task_description('vzstop', vmid),
tooltip: Ext.String.format(gettext('Stop {0} immediately'), 'CT'),
- dangerous: true,
handler: function() {
- vm_command("stop");
+ Ext.create('PVE.GuestStop', {
+ nodename: nodename,
+ vm: vm,
+ autoShow: true,
+ });
},
iconCls: 'fa fa-stop',
}],
--git a/www/manager6/qemu/CmdMenu.js b/www/manager6/qemu/CmdMenu.js
index 4f59d5f7..834577e7 100644
--- a/www/manager6/qemu/CmdMenu.js
+++ b/www/manager6/qemu/CmdMenu.js
@@ -93,7 +93,13 @@ Ext.define('PVE.qemu.CmdMenu', {
iconCls: 'fa fa-fw fa-stop',
disabled: stopped,
tooltip: Ext.String.format(gettext('Stop {0} immediately'), 'VM'),
- handler: () => confirmedVMCommand('stop'),
+ handler: () => {
+ Ext.create('PVE.GuestStop', {
+ nodename: info.node,
+ vm: info,
+ autoShow: true,
+ });
+ },
},
{
text: gettext('Reboot'),
diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
index 2f9605a4..f28ee67b 100644
--- a/www/manager6/qemu/Config.js
+++ b/www/manager6/qemu/Config.js
@@ -176,11 +176,13 @@ Ext.define('PVE.qemu.Config', {
}, {
text: gettext('Stop'),
disabled: !caps.vms['VM.PowerMgmt'],
- dangerous: true,
tooltip: Ext.String.format(gettext('Stop {0} immediately'), 'VM'),
- confirmMsg: Proxmox.Utils.format_task_description('qmstop', vmid),
handler: function() {
- vm_command("stop", { timeout: 30 });
+ Ext.create('PVE.GuestStop', {
+ nodename: nodename,
+ vm: vm,
+ autoShow: true,
+ });
},
iconCls: 'fa fa-stop',
}, {
diff --git a/www/manager6/window/GuestStop.js b/www/manager6/window/GuestStop.js
new file mode 100644
index 00000000..16d05a70
--- /dev/null
+++ b/www/manager6/window/GuestStop.js
@@ -0,0 +1,79 @@
+Ext.define('PVE.GuestStop', {
+ extend: 'Ext.window.MessageBox',
+
+ closeAction: 'destroy',
+
+ initComponent: function() {
+ let me = this;
+
+ if (!me.nodename) {
+ throw "no node name specified";
+ }
+ if (!me.vm) {
+ throw "no vm specified";
+ }
+
+ let vmid = me.vm.vmid;
+ let vmidString = vmid.toString();
+ let guestType = me.vm.type;
+ let overruleTaskType = guestType === 'qemu' ? 'qmshutdown' : 'vzshutdown';
+
+ me.taskType = guestType === 'qemu' ? 'qmstop' : 'vzstop';
+ me.url = `/nodes/${me.nodename}/${guestType}/${vmid}/status/stop`;
+
+ let caps = Ext.state.Manager.get('GuiCap');
+ let hasSysModify = caps.nodes['Sys.Modify'];
+
+ // offer to overrule if there is at least one matching shutdown task
+ // and the guest is not HA-enabled.
+ // allow users to abort tasks started by one of their API tokens
+ let shutdownTaskIdx = Ext.getStore('pve-cluster-tasks')?.findBy(task =>
+ (hasSysModify || task.data.user === Proxmox.UserName) &&
+ task.data.id === vmidString &&
+ task.data.status === undefined &&
+ task.data.type === overruleTaskType,
+ );
+ let haEnabled = me.vm.hastate && me.vm.hastate !== 'unmanaged';
+ me.askOverrule = !haEnabled && shutdownTaskIdx >= 0;
+
+ me.callParent();
+
+ me.promptContainer.add({
+ xtype: 'proxmoxcheckbox',
+ name: 'overrule-shutdown',
+ checked: true,
+ boxLabel: gettext('Overrule active shutdown tasks'),
+ hidden: !me.askOverrule,
+ });
+ },
+
+ handler: function(btn) {
+ let me = this;
+ if (btn === 'yes') {
+ let checkbox = me.promptContainer.down('proxmoxcheckbox[name=overrule-shutdown]');
+ let overruleShutdown = me.askOverrule && checkbox.getSubmitValue();
+ let params = overruleShutdown ? { 'overrule-shutdown': 1 } : undefined;
+ Proxmox.Utils.API2Request({
+ url: me.url,
+ waitMsgTarget: me,
+ method: 'POST',
+ params: params,
+ failure: function(response, opts) {
+ Ext.Msg.alert('Error', response.htmlStatus);
+ },
+ });
+ }
+ },
+
+ show: function() {
+ let me = this;
+ let cfg = {
+ title: gettext('Confirm'),
+ icon: Ext.Msg.WARNING,
+ msg: Proxmox.Utils.format_task_description(me.taskType, me.vm.vmid),
+ buttons: Ext.Msg.YESNO,
+ callback: btn => me.handler(btn),
+ };
+ me.callParent([cfg]);
+ },
+});
--
2.39.2
next prev parent reply other threads:[~2024-04-12 14:17 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-12 14:15 [pve-devel] [PATCH guest-common/container/qemu-server/manager v3 0/5] fix #4474: stop tasks may overrule " Friedrich Weber
2024-04-12 14:15 ` [pve-devel] [PATCH guest-common v3 1/5] guest helpers: add helper to abort active guest tasks of a certain type Friedrich Weber
2024-04-17 18:44 ` [pve-devel] applied: " Thomas Lamprecht
2024-04-12 14:15 ` [pve-devel] [PATCH container v3 2/5] fix #4474: lxc api: add overrule-shutdown parameter to stop endpoint Friedrich Weber
2024-04-17 18:44 ` [pve-devel] applied: " Thomas Lamprecht
2024-04-12 14:15 ` [pve-devel] [PATCH qemu-server v3 3/5] fix #4474: qemu " Friedrich Weber
2024-04-17 18:44 ` [pve-devel] applied: " Thomas Lamprecht
2024-04-12 14:15 ` [pve-devel] [PATCH manager v3 4/5] ui: fix typo to make pve-cluster-tasks store globally available Friedrich Weber
2024-04-17 18:45 ` [pve-devel] applied: " Thomas Lamprecht
2024-04-12 14:15 ` Friedrich Weber [this message]
2024-04-19 10:17 ` [pve-devel] [PATCH manager v3 5/5] fix #4474: ui: guest stop: offer to overrule active shutdown tasks Dominik Csapak
2024-04-21 8:28 ` Thomas Lamprecht
2024-04-20 18:34 ` [pve-devel] applied: " Thomas Lamprecht
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240412141553.430554-6-f.weber@proxmox.com \
--to=f.weber@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal