[pve-devel] applied: [PATCH v2 qemu-server] qmp helpers: device add/del: use HMP interface
Thomas Lamprecht
t.lamprecht at proxmox.com
Tue Feb 4 17:22:30 CET 2025
Am 04.02.25 um 16:51 schrieb Fiona Ebner:
> 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.
>
> A timeout of 30 seconds is used rather then relying on the low default
> like before, since device plug operations require actions by the guest
> kernel and might require IO. Device plug is often an interactive
> operation, so a too high timeout could lead to bad UX. Should specific
> devices need a higher timeout, it can still be increased further for
> them in the future.
>
> Signed-off-by: Fiona Ebner <f.ebner at proxmox.com>
> ---
>
> Changes in v2:
> * also mention 'del' in commit title
> * fix commit message (bug #5985 is not actually fixed by this, that
> requires bumping the timeout for netdev_add)
> * use 30 seconds as a timeout (see commit message for rationale)
>
> PVE/QemuServer/QMPHelpers.pm | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
>
applied, thanks!
I reduced the timeout to 25s to ensure we got some headroom to the 30s timeout
that the web UI imposes for API requests, so that anything that doesn't triggers
a task-worker gets a nicer error. We can still increase that, before the change
it was only 5s anyway, so even with "just" 25s it's quite a bit longer now.
More information about the pve-devel
mailing list