* [pve-devel] [PATCH qemu-server] qmp helpers: device add: use HMP interface
@ 2025-02-04 15:24 Fiona Ebner
2025-02-04 15:34 ` Fiona Ebner
2025-02-04 15:51 ` Fiona Ebner
0 siblings, 2 replies; 3+ messages in thread
From: Fiona Ebner @ 2025-02-04 15:24 UTC (permalink / raw)
To: pve-devel
Fixes device hotplug in combination with QEMU 9.2.
QEMU commit be93fd5372 ("qdev-monitor: avoid QemuOpts in QMP device_add")
notes:
> This patch changes the behavior of QMP device_add but not HMP
> device_add. QMP clients that sent incorrectly typed device_add QMP
> commands no longer work. This is a breaking change but clients should be
> using the correct types already.
The qemu_deviceadd() helper does not have the required type
information right now, so switch to using HMP, which still behaves the
same when passing a device commandline string. QEMU commit be93fd5372
fixes passing in complex properties via JSON, but the qemu_deviceadd()
helper never uses any such, as it already only received a string (and
naively split it up).
Use HMP for 'device_del' too, simply to keep the qemu_deviceadd() and
qemu_devicedel() helpers consistent.
Switching back to QMP using the correct types in the JSON can still be
done later. Unfortunately, 'qmp-query-schema' does not provide
device-specific types, so another way is needed.
The timeout of 60 seconds is used, because that is the same that's
currently used by the 'drive_add' HMP commands and a too low timeout
is known to cause issues [0]. While 'drive_del' uses 10 minutes, there
were no issues reported with the 'device_del' operations using the
default timeout until now, but it still makes sense to increase,
because for some devices IO might need to happen during detach, so use
60 seconds for now too.
While this fixes the singular issue reported in [0] by accident,
increasing the QMP timeout like in the patches proposed there will be
needed when switching back to QMP and also covers object_{add,del}.
[0]: https://bugzilla.proxmox.com/show_bug.cgi?id=5985
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
PVE/QemuServer/QMPHelpers.pm | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/PVE/QemuServer/QMPHelpers.pm b/PVE/QemuServer/QMPHelpers.pm
index 1bc9e5dc..332442f3 100644
--- a/PVE/QemuServer/QMPHelpers.pm
+++ b/PVE/QemuServer/QMPHelpers.pm
@@ -25,15 +25,14 @@ sub qemu_deviceadd {
my ($vmid, $devicefull) = @_;
$devicefull = "driver=".$devicefull;
- my %options = split(/[=,]/, $devicefull);
- mon_cmd($vmid, "device_add" , %options);
+ PVE::QemuServer::Monitor::hmp_cmd($vmid, "device_add $devicefull", 60);
}
sub qemu_devicedel {
my ($vmid, $deviceid) = @_;
- my $ret = mon_cmd($vmid, "device_del", id => $deviceid);
+ PVE::QemuServer::Monitor::hmp_cmd($vmid, "device_del $deviceid", 60);
}
sub qemu_objectadd {
--
2.39.5
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pve-devel] [PATCH qemu-server] qmp helpers: device add: use HMP interface
2025-02-04 15:24 [pve-devel] [PATCH qemu-server] qmp helpers: device add: use HMP interface Fiona Ebner
@ 2025-02-04 15:34 ` Fiona Ebner
2025-02-04 15:51 ` Fiona Ebner
1 sibling, 0 replies; 3+ messages in thread
From: Fiona Ebner @ 2025-02-04 15:34 UTC (permalink / raw)
To: pve-devel
Am 04.02.25 um 16:24 schrieb Fiona Ebner:
> The timeout of 60 seconds is used, because that is the same that's
> currently used by the 'drive_add' HMP commands and a too low timeout
> is known to cause issues [0]. While 'drive_del' uses 10 minutes, there
> were no issues reported with the 'device_del' operations using the
> default timeout until now, but it still makes sense to increase,
> because for some devices IO might need to happen during detach, so use
> 60 seconds for now too.
Actually, I'm a bit torn with the 60 seconds. We really didn't have many
reports of issues here and this might feel like too long from a user
perspective. Increasing in specific cases can always still be done, but
device-plug is often done interactively, so might be bad UX to have too
much. The bug report [0] mentions that 7 seconds is enough for the
operation, so maybe using 30 seconds as the timeout is better?
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [pve-devel] [PATCH qemu-server] qmp helpers: device add: use HMP interface
2025-02-04 15:24 [pve-devel] [PATCH qemu-server] qmp helpers: device add: use HMP interface Fiona Ebner
2025-02-04 15:34 ` Fiona Ebner
@ 2025-02-04 15:51 ` Fiona Ebner
1 sibling, 0 replies; 3+ messages in thread
From: Fiona Ebner @ 2025-02-04 15:51 UTC (permalink / raw)
To: pve-devel
Superseded by v2:
https://lore.proxmox.com/pve-devel/20250204155101.151349-1-f.ebner@proxmox.com/T/#u
_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-02-04 15:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-04 15:24 [pve-devel] [PATCH qemu-server] qmp helpers: device add: use HMP interface Fiona Ebner
2025-02-04 15:34 ` Fiona Ebner
2025-02-04 15:51 ` Fiona Ebner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox