public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
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 +
 www/manager6/lxc/CmdMenu.js      |  8 +++-
 www/manager6/lxc/Config.js       |  8 ++--
 www/manager6/qemu/CmdMenu.js     |  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				\
diff --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',
 		}],
diff --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





  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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal