[pve-devel] [PATCH-SERIES qemu-server/manager] fix #5440: HMP/disk timeout improvements

[pve-devel] [PATCH-SERIES qemu-server/manager] fix #5440: HMP/disk timeout improvements

[Fiona Ebner]

Currently, all HMP commands (issued as a "human-monitor-command" QMP
command) use the default timeout for QMP commands of just 5 seconds.

For drive-related commands, this is not always enough, as reported in
bug #5440. Other drive-related QMP commands use 10 minutes, so it is
natural to increase the limit for the HMP command for detaching a
drive to that too.

Other HMP commands that are issued are limited by being in a
synchronous API call or interactive monitor session, but can still
benefit from an increased timeout over the 5 second default.

Lastly, the UI is adapted to use the asynchronous API call when
hot-unplugging a drive, to be more in-line with the backend.


qemu-server:

Fiona Ebner (5):
  monitor: allow passing timeout for a HMP command
  api: human monitor: increase timeout to 25 seconds
  vzdump: increase timeout for attaching drives to 60 seconds
  cli: qm: increase timeout for monitor commands to 30 seconds
  fix #5440: hmp helpers: drive{add,del}: increase timeout

 PVE/API2/Qemu.pm          | 2 +-
 PVE/CLI/qm.pm             | 2 +-
 PVE/QemuServer.pm         | 4 ++--
 PVE/QemuServer/Monitor.pm | 4 ++--
 PVE/VZDump/QemuServer.pm  | 4 ++--
 5 files changed, 8 insertions(+), 8 deletions(-)


pve-manager:

Fiona Ebner (2):
  ui: qemu: hardware: use background delay for asynchronous remove tasks
  ui: qemu: hardware: use asynchronous remove API call for disk
    hot-unplug

 www/manager6/qemu/HardwareView.js | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server 1/5] monitor: allow passing timeout for a HMP command

[Fiona Ebner]

Passing the timeout key with an explicit value of undef is fine,
because both the absence of the timeout key and an explicit value of
undef will lead to $timeout being undef in the qmp_cmd() function.

In preparation to increase the timeout for certain (e.g. disk-related)
HMP commands.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer/Monitor.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/QemuServer/Monitor.pm b/PVE/QemuServer/Monitor.pm
index a7567518..937006ae 100644
--- a/PVE/QemuServer/Monitor.pm
+++ b/PVE/QemuServer/Monitor.pm
@@ -50,11 +50,11 @@ sub mon_cmd {
 }
 
 sub hmp_cmd {
-    my ($vmid, $cmdline) = @_;
+    my ($vmid, $cmdline, $timeout) = @_;
 
     my $cmd = {
 	execute => 'human-monitor-command',
-	arguments => { 'command-line' => $cmdline },
+	arguments => { 'command-line' => $cmdline, timeout => $timeout },
     };
 
     return qmp_cmd($vmid, $cmd);
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server 2/5] api: human monitor: increase timeout to 25 seconds

[Fiona Ebner]

The default timeout is 5 seconds, but some HMP commands (e.g.
disk-related ones) might take longer than that. The API call is
synchronous, so has to complete within 30 seconds, and since there is
no other costly operation, use 25 seconds.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/API2/Qemu.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/API2/Qemu.pm b/PVE/API2/Qemu.pm
index 2a349c8c..8127ec02 100644
--- a/PVE/API2/Qemu.pm
+++ b/PVE/API2/Qemu.pm
@@ -4919,7 +4919,7 @@ __PACKAGE__->register_method({
 
 	my $res = '';
 	eval {
-	    $res = PVE::QemuServer::Monitor::hmp_cmd($vmid, $param->{command});
+	    $res = PVE::QemuServer::Monitor::hmp_cmd($vmid, $param->{command}, 25);
 	};
 	$res = "ERROR: $@" if $@;
 
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server 3/5] vzdump: increase timeout for attaching drives to 60 seconds

[Fiona Ebner]

The default timeout for HMP commands is 5 seconds and while it should
be rather fast to attach a new drive to QEMU, a system can be very
busy during backup, so future-proof and increase to 60 seconds.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/VZDump/QemuServer.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/VZDump/QemuServer.pm b/PVE/VZDump/QemuServer.pm
index 8c97ee62..6f0656fe 100644
--- a/PVE/VZDump/QemuServer.pm
+++ b/PVE/VZDump/QemuServer.pm
@@ -461,7 +461,7 @@ my $attach_tpmstate_drive = sub {
 
     my $drive = "file=$task->{tpmpath},if=none,read-only=on,id=drive-tpmstate0-backup";
     $drive =~ s/\\/\\\\/g;
-    my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_add auto \"$drive\"");
+    my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_add auto \"$drive\"", 60);
     die "attaching TPM drive failed - $ret\n" if $ret !~ m/OK/s;
 };
 
@@ -606,7 +606,7 @@ my sub attach_fleecing_images {
 	    # fleecing image when allocating.
 	    $drive .= ",size=$di->{size}" if $format eq 'raw';
 	    $drive =~ s/\\/\\\\/g;
-	    my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_add auto \"$drive\"");
+	    my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_add auto \"$drive\"", 60);
 	    die "attaching fleecing image $volid failed - $ret\n" if $ret !~ m/OK/s;
 	}
     }
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server 4/5] cli: qm: increase timeout for monitor commands to 30 seconds

[Fiona Ebner]

The default timeout is 5 seconds, but some HMP commands (e.g.
disk-related ones) might take longer than that. It's still an
interactive session, so use 30 seconds for now. Should there be any
user-complains about frequent timeouts, it could still be increased
further.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/CLI/qm.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/CLI/qm.pm b/PVE/CLI/qm.pm
index b105830f..d3dbf7b4 100755
--- a/PVE/CLI/qm.pm
+++ b/PVE/CLI/qm.pm
@@ -512,7 +512,7 @@ __PACKAGE__->register_method ({
 	    next if $input =~ m/^\s*$/;
 	    last if $input =~ m/^\s*q(uit)?\s*$/;
 
-	    eval { print PVE::QemuServer::Monitor::hmp_cmd($vmid, $input) };
+	    eval { print PVE::QemuServer::Monitor::hmp_cmd($vmid, $input, 30) };
 	    print "ERROR: $@" if $@;
 	}
 
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server 5/5] fix #5440: hmp helpers: drive{add, del}: increase timeout

[Fiona Ebner]

The default timeout for HMP commands is 5 seconds.

While it should be rather fast to attach a new drive to QEMU, a busy
system might take longer, so future-proof and increase to 60 seconds.

On the other hand, detaching a drive needs to complete any pending IO
on it, so use the same 10 minutes timeout that's used for
drive-related QMP commands.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 82e7d6a6..ba3d9f26 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4377,7 +4377,7 @@ sub qemu_driveadd {
     my $io_uring = min_version($kvmver, 6, 0);
     my $drive = print_drive_commandline_full($storecfg, $vmid, $device, undef, $io_uring);
     $drive =~ s/\\/\\\\/g;
-    my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_add auto \"$drive\"");
+    my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_add auto \"$drive\"", 60);
 
     # If the command succeeds qemu prints: "OK"
     return 1 if $ret =~ m/OK/s;
@@ -4388,7 +4388,7 @@ sub qemu_driveadd {
 sub qemu_drivedel {
     my ($vmid, $deviceid) = @_;
 
-    my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_del drive-$deviceid");
+    my $ret = PVE::QemuServer::Monitor::hmp_cmd($vmid, "drive_del drive-$deviceid", 10 * 60);
     $ret =~ s/^\s+//;
 
     return 1 if $ret eq "";
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager 1/2] ui: qemu: hardware: use background delay for asynchronous remove tasks

[Fiona Ebner]

Avoids spawning a progress window for tasks that do complete more
quickly than the background delay.

Currently, the remove task is only asynchronous (i.e. using POST) when
it's for an unused disk, but this might change in the future (e.g. for
hot-unplug).

When adding a disk, a background delay of 5 seconds is already used.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 www/manager6/qemu/HardwareView.js | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index 672a7e1a..6dcba071 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -486,17 +486,19 @@ Ext.define('PVE.qemu.HardwareView', {
 		return msg;
 	    },
 	    handler: function(btn, e, rec) {
+		let params = { 'delete': rec.data.key };
+		if (btn.RESTMethod === 'POST') {
+		    params.background_delay = 5;
+		}
 		Proxmox.Utils.API2Request({
 		    url: '/api2/extjs/' + baseurl,
 		    waitMsgTarget: me,
 		    method: btn.RESTMethod,
-		    params: {
-			'delete': rec.data.key,
-		    },
+		    params: params,
 		    callback: () => me.reload(),
 		    failure: response => Ext.Msg.alert('Error', response.htmlStatus),
 		    success: function(response, options) {
-			if (btn.RESTMethod === 'POST') {
+			if (btn.RESTMethod === 'POST' && response.result.data !== null) {
 			    Ext.create('Proxmox.window.TaskProgress', {
 				autoShow: true,
 				upid: response.result.data,
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH manager 2/2] ui: qemu: hardware: use asynchronous remove API call for disk hot-unplug

[Fiona Ebner]

The backend uses a 10 minute timeout for disk hot-unplug, so avoid
using the synchronous call which only has a 30 second timeout.

Commit 3b2e557f ("close #584: ui qemu: changed remove unused disk to
asynchron call") introduced the necessary functionality when removing
unused disks.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 www/manager6/qemu/HardwareView.js | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/www/manager6/qemu/HardwareView.js b/www/manager6/qemu/HardwareView.js
index 6dcba071..e1902695 100644
--- a/www/manager6/qemu/HardwareView.js
+++ b/www/manager6/qemu/HardwareView.js
@@ -608,6 +608,7 @@ Ext.define('PVE.qemu.HardwareView', {
 
 	    const deleted = !!rec.data.delete;
 	    const pending = deleted || me.hasPendingChanges(key);
+	    const isRunning = me.pveSelNode.data.running;
 
 	    const isCloudInit = isCloudInitKey(value);
 	    const isCDRom = value && !!value.toString().match(/media=cdrom/);
@@ -616,7 +617,7 @@ Ext.define('PVE.qemu.HardwareView', {
 	    const isUsedDisk = !isUnusedDisk && row.isOnStorageBus && !isCDRom;
 	    const isDisk = isUnusedDisk || isUsedDisk;
 	    const isEfi = key === 'efidisk0';
-	    const tpmMoveable = key === 'tpmstate0' && !me.pveSelNode.data.running;
+	    const tpmMoveable = key === 'tpmstate0' && !isRunning;
 
 	    let cannotDelete = deleted || row.never_delete;
 	    cannotDelete ||= isCDRom && !cdromCap;
@@ -625,7 +626,7 @@ Ext.define('PVE.qemu.HardwareView', {
 	    remove_btn.setDisabled(cannotDelete);
 
 	    remove_btn.setText(isUsedDisk && !isCloudInit ? remove_btn.altText : remove_btn.defaultText);
-	    remove_btn.RESTMethod = isUnusedDisk ? 'POST':'PUT';
+	    remove_btn.RESTMethod = isUnusedDisk || (isDisk && isRunning) ? 'POST' : 'PUT';
 
 	    edit_btn.setDisabled(
 	        deleted || !row.editor || isCloudInit || (isCDRom && !cdromCap) || (isDisk && !diskCap));
-- 
2.39.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] applied-series: [PATCH-SERIES qemu-server/manager] fix #5440: HMP/disk timeout improvements

[Fabian Grünbichler]

thanks!

On May 3, 2024 1:19 pm, Fiona Ebner wrote:
> Currently, all HMP commands (issued as a "human-monitor-command" QMP
> command) use the default timeout for QMP commands of just 5 seconds.
> 
> For drive-related commands, this is not always enough, as reported in
> bug #5440. Other drive-related QMP commands use 10 minutes, so it is
> natural to increase the limit for the HMP command for detaching a
> drive to that too.
> 
> Other HMP commands that are issued are limited by being in a
> synchronous API call or interactive monitor session, but can still
> benefit from an increased timeout over the 5 second default.
> 
> Lastly, the UI is adapted to use the asynchronous API call when
> hot-unplugging a drive, to be more in-line with the backend.
> 
> 
> qemu-server:
> 
> Fiona Ebner (5):
>   monitor: allow passing timeout for a HMP command
>   api: human monitor: increase timeout to 25 seconds
>   vzdump: increase timeout for attaching drives to 60 seconds
>   cli: qm: increase timeout for monitor commands to 30 seconds
>   fix #5440: hmp helpers: drive{add,del}: increase timeout
> 
>  PVE/API2/Qemu.pm          | 2 +-
>  PVE/CLI/qm.pm             | 2 +-
>  PVE/QemuServer.pm         | 4 ++--
>  PVE/QemuServer/Monitor.pm | 4 ++--
>  PVE/VZDump/QemuServer.pm  | 4 ++--
>  5 files changed, 8 insertions(+), 8 deletions(-)
> 
> 
> pve-manager:
> 
> Fiona Ebner (2):
>   ui: qemu: hardware: use background delay for asynchronous remove tasks
>   ui: qemu: hardware: use asynchronous remove API call for disk
>     hot-unplug
> 
>  www/manager6/qemu/HardwareView.js | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel