From: Friedrich Weber <f.weber@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH manager v2 6/6] fix #4474: ui: guest stop: offer to overrule active shutdown tasks
Date: Tue, 30 Jan 2024 18:10:57 +0100 [thread overview]
Message-ID: <20240130171057.438025-7-f.weber@proxmox.com> (raw)
In-Reply-To: <20240130171057.438025-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 by 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 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 | 75 ++++++++++++++++++++++++++++++++
6 files changed, 100 insertions(+), 8 deletions(-)
create mode 100644 www/manager6/window/GuestStop.js
diff --git a/www/manager6/Makefile b/www/manager6/Makefile
index dc3c85b1..a7dc2022 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..1fcba17c
--- /dev/null
+++ b/www/manager6/window/GuestStop.js
@@ -0,0 +1,75 @@
+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`;
+
+ // offer to overrule if there is at least one matching shutdown task
+ // and the guest is not HA-enabled
+ let shutdownTaskIdx = Ext.getStore('pve-cluster-tasks')?.findBy(task =>
+ 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, this.vm.vmid),
+ buttons: Ext.Msg.YESNO,
+ callback: btn => me.handler(btn),
+ };
+ me.callParent([cfg]);
+ },
+});
--
2.39.2
next prev parent reply other threads:[~2024-01-30 17:11 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-30 17:10 [pve-devel] [PATCH guest-common/container/qemu-server/manager v2 0/6] fix #4474: stop tasks may overrule " Friedrich Weber
2024-01-30 17:10 ` [pve-devel] [PATCH guest-common v2 1/6] guest helpers: add helper to overrule active tasks of a specific type Friedrich Weber
2024-04-04 15:20 ` Thomas Lamprecht
2024-04-05 13:13 ` Friedrich Weber
2024-04-06 8:37 ` Thomas Lamprecht
2024-04-08 8:38 ` Friedrich Weber
2024-01-30 17:10 ` [pve-devel] [PATCH container v2 2/6] api: status: move config locking from API handler into worker Friedrich Weber
2024-04-04 15:26 ` [pve-devel] applied: " Thomas Lamprecht
2024-04-05 13:16 ` Friedrich Weber
2024-01-30 17:10 ` [pve-devel] [PATCH container v2 3/6] fix #4474: lxc api: add overrule-shutdown parameter to stop endpoint Friedrich Weber
2024-04-06 15:07 ` Thomas Lamprecht
2024-04-08 8:59 ` Friedrich Weber
2024-01-30 17:10 ` [pve-devel] [PATCH qemu-server v2 4/6] fix #4474: qemu " Friedrich Weber
2024-01-30 17:10 ` [pve-devel] [PATCH manager v2 5/6] ui: fix typo to make pve-cluster-tasks store globally available Friedrich Weber
2024-01-30 17:10 ` Friedrich Weber [this message]
2024-04-03 6:55 ` [pve-devel] [PATCH guest-common/container/qemu-server/manager v2 0/6] fix #4474: stop tasks may overrule shutdown tasks 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=20240130171057.438025-7-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