all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Friedrich Weber <f.weber@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [RFC manager 1/4] fix #4474: ui: vm stop: ask if active shutdown tasks should be aborted
Date: Thu, 26 Jan 2023 09:32:11 +0100	[thread overview]
Message-ID: <20230126083214.711099-2-f.weber@proxmox.com> (raw)
In-Reply-To: <20230126083214.711099-1-f.weber@proxmox.com>

Before stopping a VM (KVM or LXC), the UI now checks if there is an
active shutdown task for the same VM by the logged-in user. If there
is at least one, the UI asks whether these tasks should be aborted
before trying to stop the VM. If the user answers "Yes", the UI sends
a VM stop request with the `overrule-shutdown` parameter set to 1. If
there are no active shutdown tasks, or the user answers "No", the UI
sends a VM stop request without `overrule-shutdown`.

To avoid an additional API request for querying active shutdown tasks,
we check the UI's current view of cluster tasks instead, which we
fetch 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.

Signed-off-by: Friedrich Weber <f.weber@proxmox.com>
---
 www/manager6/Utils.js        | 27 +++++++++++++++++++++++++++
 www/manager6/dc/Tasks.js     |  2 +-
 www/manager6/lxc/CmdMenu.js  | 14 +++++++++++++-
 www/manager6/lxc/Config.js   |  6 +++++-
 www/manager6/qemu/CmdMenu.js | 14 +++++++++++++-
 www/manager6/qemu/Config.js  |  9 ++++++++-
 6 files changed, 67 insertions(+), 5 deletions(-)

diff --git a/www/manager6/Utils.js b/www/manager6/Utils.js
index 3dd287e3..9e25e647 100644
--- a/www/manager6/Utils.js
+++ b/www/manager6/Utils.js
@@ -1426,6 +1426,33 @@ Ext.define('PVE.Utils', {
 	}
     },
 
+    // If there is an active task of type `taskType` by the current user for the
+    // given VMID, ask whether it should be aborted. Call `callback` accordingly.
+    askForOverruleShutdown: function(guestType, vm, taskType, callback) {
+	const vmidString = vm.vmid.toString();
+	const haEnabled = vm.hastate && vm.hastate !== 'unmanaged';
+	let shutdownTask = Ext.getStore('pve-cluster-tasks')?.findBy(function(task) {
+	    return task.data.user === Proxmox.UserName &&
+		   task.data.id === vmidString &&
+		   task.data.status === undefined &&
+		   task.data.type === taskType;
+	});
+	if (!haEnabled && shutdownTask >= 0) {
+	    const msg = gettext('There are active {0} tasks for {1} {2}. Would you like to abort them before proceeding?');
+	    Ext.Msg.show({
+			 title: gettext('Confirm'),
+			 icon: Ext.Msg.QUESTION,
+			 buttons: Ext.Msg.YESNO,
+			 msg: Ext.String.format(msg, taskType, guestType, vmidString),
+			 callback: function(btn) {
+			     callback(btn === 'yes');
+			 },
+	    });
+	} else {
+	    callback(false);
+	}
+    },
+
     // test automation helper
     call_menu_handler: function(menu, text) {
 	let item = menu.query('menuitem').find(el => el.text === text);
diff --git a/www/manager6/dc/Tasks.js b/www/manager6/dc/Tasks.js
index 5344ede4..2001bf76 100644
--- a/www/manager6/dc/Tasks.js
+++ b/www/manager6/dc/Tasks.js
@@ -11,7 +11,7 @@ Ext.define('PVE.dc.Tasks', {
 	let me = this;
 
 	let taskstore = Ext.create('Proxmox.data.UpdateStore', {
-	    storeid: 'pve-cluster-tasks',
+	    storeId: 'pve-cluster-tasks',
 	    model: 'proxmox-tasks',
 	    proxy: {
 		type: 'proxmox',
diff --git a/www/manager6/lxc/CmdMenu.js b/www/manager6/lxc/CmdMenu.js
index 56f36b5e..01631b1d 100644
--- a/www/manager6/lxc/CmdMenu.js
+++ b/www/manager6/lxc/CmdMenu.js
@@ -66,7 +66,19 @@ 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: () => {
+		    let msg = Proxmox.Utils.format_task_description('vzstop', info.vmid);
+		    Ext.Msg.confirm(gettext('Confirm'), msg, btn => {
+			if (btn === 'yes') {
+			    PVE.Utils.askForOverruleShutdown('CT', info,
+							     'vzshutdown', overruleShutdown => {
+				const param = overruleShutdown ? { 'overrule-shutdown': 1 }
+								     : undefined;
+				vm_command('stop', param);
+			    });
+			}
+		    });
+		},
 	    },
 	    {
 		text: gettext('Reboot'),
diff --git a/www/manager6/lxc/Config.js b/www/manager6/lxc/Config.js
index 23c17d2e..21d93058 100644
--- a/www/manager6/lxc/Config.js
+++ b/www/manager6/lxc/Config.js
@@ -81,7 +81,11 @@ Ext.define('PVE.lxc.Config', {
 		    tooltip: Ext.String.format(gettext('Stop {0} immediately'), 'CT'),
 		    dangerous: true,
 		    handler: function() {
-			vm_command("stop");
+			PVE.Utils.askForOverruleShutdown('CT', vm,
+							 'vzshutdown', overruleShutdown => {
+			    const param = overruleShutdown ? { 'overrule-shutdown': 1 } : undefined;
+			    vm_command('stop', param);
+			});
 		    },
 		    iconCls: 'fa fa-stop',
 		}],
diff --git a/www/manager6/qemu/CmdMenu.js b/www/manager6/qemu/CmdMenu.js
index ccc5f74d..3edd2550 100644
--- a/www/manager6/qemu/CmdMenu.js
+++ b/www/manager6/qemu/CmdMenu.js
@@ -93,7 +93,19 @@ 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: () => {
+		    let msg = Proxmox.Utils.format_task_description('qmstop', info.vmid);
+		    Ext.Msg.confirm(gettext('Confirm'), msg, btn => {
+			if (btn === 'yes') {
+			    PVE.Utils.askForOverruleShutdown('VM', info,
+							     'qmshutdown', overruleShutdown => {
+				const param = overruleShutdown ? { 'overrule-shutdown': 1 }
+								     : undefined;
+				vm_command('stop', param);
+			    });
+			}
+		    });
+		},
 	    },
 	    {
 		text: gettext('Reboot'),
diff --git a/www/manager6/qemu/Config.js b/www/manager6/qemu/Config.js
index 94c540c5..68c41454 100644
--- a/www/manager6/qemu/Config.js
+++ b/www/manager6/qemu/Config.js
@@ -180,7 +180,14 @@ Ext.define('PVE.qemu.Config', {
 		    tooltip: Ext.String.format(gettext('Stop {0} immediately'), 'VM'),
 		    confirmMsg: Proxmox.Utils.format_task_description('qmstop', vmid),
 		    handler: function() {
-			vm_command("stop", { timeout: 30 });
+			PVE.Utils.askForOverruleShutdown('VM', vm,
+							 'qmshutdown', overruleShutdown => {
+			    const param = { timeout: 30 };
+			    if (overruleShutdown) {
+				param['overrule-shutdown'] = 1;
+			    }
+			    vm_command('stop', param);
+			});
 		    },
 		    iconCls: 'fa fa-stop',
 		}, {
-- 
2.30.2





  reply	other threads:[~2023-01-26  8:33 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-26  8:32 [pve-devel] [RFC manager/container/qemu-server/guest-common 0/4] fix #4474: stop tasks may overrule shutdown tasks Friedrich Weber
2023-01-26  8:32 ` Friedrich Weber [this message]
2023-01-26  8:32 ` [pve-devel] [RFC container 2/4] fix #4474: lxc api: add overrule-shutdown parameter to stop endpoint Friedrich Weber
2023-11-17 13:09   ` Wolfgang Bumiller
2023-12-01  9:57     ` Friedrich Weber
2024-01-02 13:34       ` Friedrich Weber
2023-01-26  8:32 ` [pve-devel] [RFC qemu-server 3/4] fix #4474: qemu " Friedrich Weber
2023-11-17 13:12   ` Wolfgang Bumiller
2023-01-26  8:32 ` [pve-devel] [RFC guest-common 4/4] guest helpers: add helper to overrule active tasks of a specific type Friedrich Weber
2023-11-17 12:53   ` Wolfgang Bumiller
2023-12-01  9:57     ` Friedrich Weber
2023-09-27  9:04 ` [pve-devel] [RFC manager/container/qemu-server/guest-common 0/4] fix #4474: stop tasks may overrule shutdown tasks Friedrich Weber
2023-11-17 12:31   ` Wolfgang Bumiller
2023-12-01  9:57     ` Friedrich Weber

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=20230126083214.711099-2-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