- Devel - libvirt List Archives

[RFC] Reintroducing customisable hotplug timeouts
by Akash Kulhalli 30 Mar '26

30 Mar '26

I'd like to resurface an earlier proposal [1] to make the unplug timeout user-configurable. With production guests running sustained, non-trivial workloads, we find that hot(un)plug operations regularly takes longer than the current default of 5s. However, the inability to easily configure this timeout means that in the worst case a recompilation with appropriate values set is required for each deployment, which is cumbersome to maintain and patch. The previous proposal [1] also initially made the timeout configurable, however that was specifically for the ppc64 architecture. Therefore, imposing this change for an entire architecture made sense, since it was a universal requirement for that platform. With deployment environments as diverse and varied as x86, having a single, inconfigurable value does not make as much sense now. For x86, such a change is not ideal since the desired timeout value depends very heavily on the deployment environment in use - if it is being performed by a human anything more than 10s is an undesirable user experience [2], as pointed out in the original discussion. However, more relaxed timeout values (e.g. 15-20s) can be more acceptable if the operations are background/ not directly user-facing. It is in these cases where the ability to change the timeout more easily becomes more desirable. To prevent misconfiguration, while maintaining the same out-of-box experience for existing users, I'd also like to propose the following- 1. As mentioned in the original proposal [1], the timeout cannot be set to a value lower than the default timeout of 5s; in these cases a warning will be emitted and the timeout will be reset to the default value. 2. Including an upper cap would also be desirable, in my opinion - any API consumer should not wait endlessly for an async operation to finish. 3. This configuration option will only be described in qemu.conf, not imposed - the out-of-box experience will remain unchanged at the current default (5s) for everyone who does not wish to tweak/is unaware of this value. An administrator who knowingly changes these values is therefore not blindsided by any perceived "hangs" resulting from the use of an increased timeout window. Since this would be a clearly documented and explicitly opt-in behaviour, the benefits in the form of easy modification based on evolving operational needs would be quite significant. Given the varying nature of production environments and specific operational needs at scale, it would therefore be beneficial to reconsider the merits of the original proposal. [1] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/TXYV… [2] https://www.nngroup.com/articles/response-times-3-important-limits/

4 12

[libvirt PATCH] hypervisor: Call virWaitForDevices() after detaching host devices
by Pavel Hrdina 27 Mar '26

27 Mar '26

From: Pavel Hrdina <phrdina(a)redhat.com> On systems with selinux enabled starting a VM with managed host device using IOMMUFD backend can run into race-condition where both libvirt and udev are setting selinux label on /dev/vfio/devices/vfioX device. If udev is the last one to set selinux label starting VM fails with: error: internal error: QEMU unexpectedly closed the monitor (vm='test'): 2026-03-26T15:47:36.620422Z qemu-kvm: -device {"driver":"vfio-pci","id":"hostdev0","iommufd":"iommufd0","fd":"20","bus":"pci.7","addr":"0x0"}: vfio hostdev0: Failed to add fd 20 to KVM VFIO device: Invalid argument We need to wait for udev to finish processing all events. Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com> --- src/hypervisor/virhostdev.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c index f4d2176204..b063a44f62 100644 --- a/src/hypervisor/virhostdev.c +++ b/src/hypervisor/virhostdev.c @@ -782,6 +782,9 @@ virHostdevPreparePCIDevicesImpl(virHostdevManager *mgr, } } + /* Step 2.5: Wait for udev to handle all events for devices. */ + virWaitForDevices(); + /* At this point, all devices are attached to the stub driver and have * been marked as inactive */ -- 2.53.0

2 1

[PATCH] virpci: Stop writing out of bounds in virPCIDeviceReadClass()
by Michal Privoznik 27 Mar '26

27 Mar '26

From: Michal Privoznik <mprivozn(a)redhat.com> Inside of virPCIDeviceReadClass() there's a call to virFileReadAll(). This reads contents of given file into a buffer (id_str). To make sure the buffer is NUL terminated string there's then write of NUL byte at 9th position of the buffer. Well, this is redundant as virFileReadAll() made sure the buffer is properly terminated on success (transitively¸ via saferead_lim()). But it is also wrong, because there's no guarantee the file is more than 8 bytes long. Just remove the NUL termination and rely on virFileReadAll() to properly terminate the buffer. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/util/virpci.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/util/virpci.c b/src/util/virpci.c index ca6f2e8210..2e32ed17ff 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -487,7 +487,6 @@ virPCIDeviceReadClass(virPCIDevice *dev, uint16_t *device_class) if (virFileReadAll(path, 9, &id_str) < 0) return -1; - id_str[8] = '\0'; if (virStrToLong_ui(id_str, NULL, 16, &value) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unusual value in %1$s/devices/%2$s/class: %3$s"), -- 2.52.0

3 2

[PATCH 0/6] Fix current CI failures and document behaviour
by Peter Krempa 27 Mar '26

27 Mar '26

Fix the CI failure I've caused, one more instance of the same problem that didn't yet cause write beyound buffer, document all the functions to prevent users from guessing and remove redundant code. The last function also optimizes further to prevent over-allocating the buffers to read the files. Peter Krempa (6): virPCIDeviceReadClass: Don't write beyond end of buffer from virFileReadAll virSecretLoadValue: Don't re-termiante the buffer virNetDevIPCheckIPv6Forwarding: Don't NUL terminate buffer from virFileReadAll virPCIDeviceReadID: Fix use of 'virFileReadAll' util: virfile: Document the various functions for reading from file/fd util: virfile: Don't over-allocate buffers in saferead_lim src/conf/virsecretobj.c | 3 - src/util/virfile.c | 145 +++++++++++++++++++++++++++++++++++----- src/util/virnetdevip.c | 4 -- src/util/virpci.c | 10 +-- 4 files changed, 135 insertions(+), 27 deletions(-) -- 2.53.0

2 11

[PATCH 0/3] leak fixes and buffer allocation optimization
by Peter Krempa 26 Mar '26

26 Mar '26

Peter Krempa (3): qemu: driver: Don't leak 'qemu_driver->backupPorts' secret: config: Don't leak 'cfg->secretsEncryptionKey' util: file: Shrink buffer returned from 'saferead_lim' to actual size src/qemu/qemu_driver.c | 1 + src/secret/secret_config.c | 1 + src/util/virfile.c | 1 + 3 files changed, 3 insertions(+) -- 2.53.0

2 4

[PATCH 0/3] Add test for virHostCPUGetMap and fix it
by Martin Kletzander 26 Mar '26

26 Mar '26

I found an old patch that fell between the cracks and to show that it can manifest itself I also added a test in PATCH 3/3 which fails without PATCH 2/3, but it shows the explained case from the 2nd patch's commit message. Can be squashed to any of the other two, this is just easier to see I feel like. Felix Huettner (1): virhostcpu: Fix potential use of unallocated memory Martin Kletzander (2): tests: Extend virhostcputest to check virHostCPUGetMap tests/virhostcpudata: Add test case with many offline CPUs src/util/virhostcpu.c | 22 ++++--- .../linux-many-offline/cpu/online | 1 + .../linux-many-offline/cpu/present | 1 + tests/virhostcputest.c | 57 +++++++++++++++++++ 4 files changed, 69 insertions(+), 12 deletions(-) create mode 100644 tests/virhostcpudata/linux-many-offline/cpu/online create mode 100644 tests/virhostcpudata/linux-many-offline/cpu/present -- 2.53.0

2 4

[PATCH v2] virhostcpu: Fix potential use of unallocated memory
by Felix Huettner 26 Mar '26

26 Mar '26

In case of a host that has a large number of cpus offline the count of host cpus and the last bit set in the virHostCPUGetOnlineBitmap might diverge significantly. This can e.g. be the case when disabling smt via /sys/devices/system/cpu/smt/control. On the host this looks like: ``` $ cat /sys/devices/system/cpu/present 0-255 $ cat /sys/devices/system/cpu/online 0-127 ``` However in this case virBitmapToData previously only allocated 16 bytes for the output bitmap. This is becase the last set bit is on the 15th byte. Users of virHostCPUGetMap however rely on the "cpumap" containing enough space for all existing cpus (so they would expect 32 bytes in this case). E.g. cmdNodeCpuMap relies on this for its output. It will then actually read 32 bytes from the start of the "cpumap" address where in this case the last 16 of these bytes are uninitialized. This manifests itself in flapping outputs of "virsh nodecpumap --pretty" like: ``` $ virsh nodecpumap --pretty CPUs present: 256 CPUs online: 128 CPU map: 0-127,192,194,202 $ virsh nodecpumap --pretty CPUs present: 256 CPUs online: 128 CPU map: 0-127,192,194,197 $ virsh nodecpumap --pretty CPUs present: 256 CPUs online: 128 CPU map: 0-127,192,194,196-197 ``` This in turn potentially causes users of this data to report wrong cpu counts. Note that this only seems to happen with at least 256 physical cpus where at least 128 are offline. We fix this by preallocating the expected bitmap size. Signed-off-by: Felix Huettner <felix.huettner(a)stackit.cloud> --- src/util/virhostcpu.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index 5dbcc8987c..4805484b1d 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -1090,28 +1090,26 @@ virHostCPUGetMap(unsigned char **cpumap, unsigned int flags) { g_autoptr(virBitmap) cpus = NULL; - int ret = -1; - int dummy; + int ncpus = virHostCPUGetCount(); virCheckFlags(0, -1); if (!cpumap && !online) - return virHostCPUGetCount(); + return ncpus; if (!(cpus = virHostCPUGetOnlineBitmap())) - goto cleanup; + return -1; + + if (cpumap) { + int len = (ncpus + CHAR_BIT) / CHAR_BIT; + *cpumap = g_new0(unsigned char, len); + virBitmapToDataBuf(cpus, *cpumap, len); + } - if (cpumap) - virBitmapToData(cpus, cpumap, &dummy); if (online) *online = virBitmapCountBits(cpus); - ret = virHostCPUGetCount(); - - cleanup: - if (ret < 0 && cpumap) - VIR_FREE(*cpumap); - return ret; + return ncpus; } base-commit: c5a73f75bc6cae4f466d0a6344d5b3277ac9c2f4 -- 2.43.0

2 1

[PATCH] virxml: Fix virXMLPropTristateBoolAllowDefault() documentation
by Michal Privoznik 26 Mar '26

26 Mar '26

From: Michal Privoznik <mprivozn(a)redhat.com> The documentation to virXMLPropTristateBoolAllowDefault() refers to itself while it meant to refer to virXMLPropTristateBool(). Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com> --- src/util/virxml.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index 274f072598..beb5301378 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -525,13 +525,13 @@ virXMLPropTristateBool(xmlNodePtr node, } -/* Same as virXMLPropTristateBoolAllowDefault, but will accept the - * value 'default' and convert it to VIR_TRISTATE_BOOL_ABSENT instead - * of rejecting it with an error. Should only be used for backwards - * compatibility reasons, and specifically to parse XML files where a - * property having value VIR_TRISTATE_BOOL_ABSENT has historically - * resulted in it being formatted with value 'default' instead of - * being omitted entirely */ +/* Same as virXMLPropTristateBool, but will accept the value 'default' + * and convert it to VIR_TRISTATE_BOOL_ABSENT instead of rejecting it + * with an error. Should only be used for backwards compatibility + * reasons, and specifically to parse XML files where a property + * having value VIR_TRISTATE_BOOL_ABSENT has historically resulted in + * it being formatted with value 'default' instead of being omitted + * entirely. */ int virXMLPropTristateBoolAllowDefault(xmlNodePtr node, const char *name, -- 2.52.0

2 1

[PATCH v2] qemu: fix potential hang in qemuMigrationSrcCancelUnattended during reconnect
by Denis V. Lunev 26 Mar '26

26 Mar '26

When libvirtd reconnects to a running QEMU process that had an in-progress migration, qemuProcessReconnect first connects the monitor and only later recovers the migration job. During this window the async job is VIR_ASYNC_JOB_NONE, so any MIGRATION status events from QEMU are silently dropped by qemuProcessHandleMigrationStatus. If the migration was already cancelled or completed by QEMU during this window, no further events will be emitted. When qemuMigrationSrcCancelUnattended later restores the async job and calls qemuMigrationSrcCancel with wait=true, the wait loop calls qemuDomainObjWait (virCondWait with no timeout) and blocks forever waiting for an event that will never arrive. Fix this by re-querying QEMU migration state with qemuMigrationAnyRefreshStatus after restoring the async job but before calling qemuMigrationSrcCancel. If QEMU has already reached a terminal state, the cancel is skipped. Signed-off-by: Denis V. Lunev <den(a)openvz.org> CC: Jiri Denemark <jdenemar(a)redhat.com> CC: Peter Krempa <pkrempa(a)redhat.com> CC: Michal Privoznik <mprivozn(a)redhat.com> CC: Efim Shevrin <efim.shevrin(a)virtuozzo.com> --- v1 -> v2: Instead of querying QEMU with query-migrate inside qemuMigrationSrcCancel, use qemuMigrationAnyRefreshStatus in qemuMigrationSrcCancelUnattended after restoring the async job to re-check migration state before the actual cancel. src/qemu/qemu_migration.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index fec808ccfb..a4bd7efa09 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -7330,6 +7330,7 @@ int qemuMigrationSrcCancelUnattended(virDomainObj *vm, virDomainJobObj *oldJob) { + virDomainJobStatus migStatus = VIR_DOMAIN_JOB_STATUS_NONE; bool storage = false; size_t i; @@ -7348,11 +7349,20 @@ qemuMigrationSrcCancelUnattended(virDomainObj *vm, VIR_JOB_NONE); } - /* We're inside a MODIFY job and the restored MIGRATION_OUT async job is - * used only for processing migration events from QEMU. Thus we don't want - * to start a nested job for talking to QEMU. + /* Query the actual migration state from QEMU. The state passed to + * qemuProcessRecoverMigrationOut may be stale: QEMU could have + * reached a terminal state between that initial query and the async + * job restore above, with the corresponding event silently dropped. */ - qemuMigrationSrcCancel(vm, VIR_ASYNC_JOB_NONE, true); + qemuMigrationAnyRefreshStatus(vm, VIR_ASYNC_JOB_NONE, &migStatus); + + if (migStatus != VIR_DOMAIN_JOB_STATUS_CANCELED) { + /* We're inside a MODIFY job and the restored MIGRATION_OUT async + * job is used only for processing migration events from QEMU. + * Thus we don't want to start a nested job for talking to QEMU. + */ + qemuMigrationSrcCancel(vm, VIR_ASYNC_JOB_NONE, true); + } virDomainObjEndAsyncJob(vm); -- 2.51.0

2 1

[PATCH] util: Fix comment in virBitmapNewData
by Martin Kletzander 26 Mar '26

26 Mar '26

From: Martin Kletzander <mkletzan(a)redhat.com> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com> --- Pushed as trivial. src/util/virbitmap.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 1a3e75671c25..78545501b791 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -601,8 +601,7 @@ virBitmapNewCopy(virBitmap *src) * Allocate a bitmap from a chunk of data containing bits * information * - * Returns a pointer to the allocated bitmap or NULL if - * memory cannot be allocated. + * Returns a pointer to the allocated bitmap. */ virBitmap * virBitmapNewData(const void *data, -- 2.53.0

1 0