gendispatch: Finish rename of the migration argument
This patch is useless.
Either APIs it don't have 'resource' nor 'bandwidth' argument to
begin with, or they serve as a wrapper over different API
(changed in previous commits). Nonetheless, in the name of
consistency, let's just change those variable names.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
src: Unify argument name of virDomainMigratePerform3()
The virDomainMigratePerform3() API declares its last argument as
'bandwidth', though throughout various typedefs, RPC and callback
implementations the name is changed to 'resource'. This creates a
confusion. Unify the name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
src: Unify argument name of virDomainMigrateBegin3()
The virDomainMigrateBegin3() API declares its last argument as
'bandwidth', though throughout various typedefs, RPC and callback
implementations the name is changed to 'resource'. This creates a
confusion. Unify the name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
src: Unify argument name of virDomainMigratePrepareTunnel3()
The virDomainMigratePrepareTunnel3() API declares one of its
argument as 'bandwidth', though throughout various typedefs, RPC
and callback implementations the name is changed to 'resource'.
This creates a confusion. Unify the name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
src: Unify argument name of virDomainMigratePrepare3()
The virDomainMigratePrepare3() API declares one of its argument
as 'bandwidth', though throughout various typedefs, RPC and
callback implementations the name is changed to 'resource'. This
creates a confusion. Unify the name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
src: Unify argument name of virDomainMigratePrepareTunnel()
The virDomainMigratePrepareTunnel() API declares one of its
argument as 'bandwidth', though throughout various typedefs, RPC
and callback implementations the name is changed to 'resource'.
This creates a confusion. Unify the name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
src: Unify argument name of virDomainMigratePrepare2()
The virDomainMigratePrepare2() API declares one of its argument as
'bandwidth', though throughout various typedefs, RPC and callback
implementations the name is changed to 'resource'. This creates a
confusion. Unify the name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
src: Unify argument name of virDomainMigratePerform()
The virDomainMigratePerform() API declares its last argument as
'bandwidth', though throughout various typedefs, RPC and callback
implementations the name is changed to 'resource'. This creates a
confusion. Unify the name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
src: Unify argument name of virDomainMigratePrepare()
The virDomainMigratePrepare() API declares its last argument as
'bandwidth', though throughout various typedefs, RPC and callback
implementations the name is changed to 'resource'. This creates a
confusion. Unify the name.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Akihiko Odaki [Sat, 8 Mar 2025 05:57:41 +0000 (14:57 +0900)]
qemu: Replace usb-storage with usb-bot
usb-storage is a compound device that automatically creates a USB mass
storage device and a SCSI device as its backend. Unfortunately it lacks
some configuration options that are usually present with a SCSI device,
and cannot represent CD-ROM in particular.
Replace usb-storage with usb-bot, which can be combined with a manually
created SCSI device. libvirt will configure the SCSI device in a way
identical with how QEMU does for usb-storage except that now it respects
a configuration option to represent CD-ROM.
Peter Krempa [Thu, 19 Jun 2025 07:46:38 +0000 (09:46 +0200)]
qemuBuildDeviceAddresDriveProps: Prepare for 'drive' address for usb-bot disks
While the 'usb-storage' based disks use the USB address directly, with
'usb-bot' the USB address is on the "controller" part of the device and
the 'scsi-hd/cd' device will use a 'drive' address from qemu's PoV.
Since we do not want to expose the 'usb-bot' as explicit controller
to preserve compatibility with existing configs we plan to upgrade
implement the formatter for 'drive' address when the "diskbus" property
is VIR_DOMAIN_DISK_BUS_USB.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Peter Krempa [Mon, 23 Jun 2025 15:51:16 +0000 (17:51 +0200)]
qemu: Fill in model of 'usb' disks to preserve ABI compatibility
While 'usb-bot' and 'usb-storage' are ABI and migration compatible for
disks it's not the case for cdroms. When migrating from a new config
using 'usb-bot' to an older daemon which would use 'usb-storage' the
guest os will get I/O errors.
Thus we must properly fill in models for 'usb' disks so that cdroms can
be handled properly.
When parsing XML fill in the models and drop the appropriate models when
formatting migratable XML.
The logic is explained in comments in the code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Peter Krempa [Thu, 19 Jun 2025 13:46:46 +0000 (15:46 +0200)]
conf: introduce usb disk models 'usb-storage' and 'usb-bot'
Historically libvirt specified 'usb-storage' as driver for USB disks.
This though combined with '-blockdev' doesn't properly configure the
device to look like CDROM for <disk type='cdrom'>.
'usb-bot' acts like a controler and allows explicitly connecting a
-device to it.
In qemu the devices share implementation so they are effectively
identical and can be used interchangably. There is a difference in how
the storage device itself (the SCSI part) looks when configured properly
as CDROM which is unfortunately not compatible/interchangable.
As this is effectively a bugfix we'll be fixing the behaviour for the
default configuration. The possibility to explicitly set the model is
added as a possibility for working around possible problems if they'd
appear.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Peter Krempa [Thu, 19 Jun 2025 13:31:46 +0000 (15:31 +0200)]
qemuxmlconftest: Invoke "disk-usb-device" case also without QEMU_CAPS_DEVICE_USB_BOT and with ABI_UPDATE
The QEMU_CAPS_DEVICE_USB_BOT device can be compiled out but
realistically it makes no sense to do it thus also makes no sense to
have another variant of input data for it.
Add another invocation of "disk-usb-device" clearing QEMU_CAPS_DEVICE_USB_BOT
to show the fallback code paths.
Also add "ABI_UPDATE" version for the two cases above as the ABI of
usb-bot cdrom is not migration-compatible and we'll be wanting to update
to the fixed configuration.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Peter Krempa [Thu, 19 Jun 2025 08:07:35 +0000 (10:07 +0200)]
qemuxmlconftest: Distribute testing of 'removable' disk property
The 'removable' property is tested for 'usb' and 'scsi' disks. The test
case for 'usb' disks already has another test case for this, so add
testing of 'removable' SCSI disks into the 'disk-scsi' case and remove
the 'disk-device-removable' case completely.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Peter Krempa [Thu, 19 Jun 2025 07:58:10 +0000 (09:58 +0200)]
qemuxmlconftest: Test various combinations of config
Add multiple USB disks to the definition testing a matrix of 'disk' and
'cdrom' <disk> elements with user-aliases, 'serial' and 'removable'
properties configured.
This patch also removes the 'ide' disk which is not related to what
we're testing here.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
util: workaround libxml2 lack of thread safe initialization
The main XML parser code global initializer historically had a mutex
protecting it, and more recently uses a pthread_once. The RelaxNG
code, however, relies on two other global initializers that are
not thread safe, just relying on setting an integer "initialized"
flag.
Calling the relevant initializers from libvirt in a protected global
initializer will protect libvirt's own concurrent usage, however, it
cannot protect against other libraries loaded in process that might
be using libxml2's schema code. Fortunately:
* The chances of other loaded non-libvirt code using libxml is
relatively low
* The chances of other loaded non-libvirt code using the schema
validation / catalog functionality inside libxml is even
lower
* The chances of both libvirt and the non-libvirt usage having
their *1st* usage of libxml2 be concurrent is tiny
IOW, in practice, although our solution doesn't fully fix the thread
safety, it is good enough.
libxml2 should none the less still be fixed to make its global
initializers be thread safe without special actions by its API
consumers[1].
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/788
[1] https://gitlab.gnome.org/GNOME/libxml2/-/merge_requests/326 Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Ján Tomko [Wed, 12 Mar 2025 15:22:25 +0000 (16:22 +0100)]
qemu: introduce QEMU_CAPS_PCI_ID
Introduced by QEMU commit f864a3235ea1d1d714b3cde2d9a810ea6344a7b5
the presence of this attribute allows libvirt to specify the alias
of the AMDVI-PCI device explicitly.
(It was implicit before the introduction of this attribute)
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Peter Krempa [Wed, 18 Jun 2025 06:29:01 +0000 (08:29 +0200)]
virSocketAddrPrefixToNetmask: Prevent undefined behaviour on bitshifts on signed integer
Shifting bits into the sign bit is undefined behaviour in C although
both gcc and clang handle it as expected.
Since the value is used as unsigned convert it to unsigned int. For code
readability use 'if' statement instead of a ternary.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/785 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Tue, 17 Jun 2025 13:01:26 +0000 (15:01 +0200)]
tlscert: Don't force 'keyEncipherment' for ECDSA and ECDH
Per RFC8813 [1] which amends RFC5580 [2] ECDSA, ECDH, and ECMQV
algorithms must not have 'keyEncipherment' present, but our code did
check it. Add exemption for known algorithms which don't use it.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/691 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 17 Jun 2025 08:42:52 +0000 (10:42 +0200)]
storage: disk: Properly handle partition numbers separated by 'p'
The 'p' separator for partitions is now common also for NVMe devices.
Fix the algorithm to extract the partition number to always consider it.
The fix is based on suggestion in the issue mentioned below.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/239 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 17 Jun 2025 08:00:20 +0000 (10:00 +0200)]
virshPrintJobProgress: Don't rewrite migration status line on non-terminals
On non-terminals print each progress report on a new line. Fix based on
suggestion in the issue report.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/756 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Mon, 16 Jun 2025 10:05:24 +0000 (12:05 +0200)]
security_manager: Don't leak seclabel in virSecurityManagerGenLabel()
When a domain is being started, seclabels are generated for it.
This is handled in virSecurityManagerGenLabel() which can either
find pre-existing seclabel in domain def or generate a new one.
At any rate, domainGenSecurityLabel() callback is called and if
it fails then the seclabel is removed from domain definition
using VIR_DELETE_ELEMENT(). While this shrinks down the seclabels
array, it does not free individual item. It has to be freed
manually.
80 bytes in 2 blocks are definitely lost in loss record 1,359 of 1,876
at 0x484CEF3: calloc (vg_replace_malloc.c:1675)
by 0x4F19B29: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.8200.5)
by 0x49E4953: virSecurityLabelDefNew (virseclabel.c:59)
by 0x4BDE0A4: virSecurityManagerGenLabel (security_manager.c:638)
by 0xBA029B7: qemuProcessPrepareDomain (qemu_process.c:6760)
by 0xBA07DF2: qemuProcessStart (qemu_process.c:8369)
by 0xB93DAC0: qemuDomainObjStart (qemu_driver.c:6371)
by 0xB93DE08: qemuDomainCreateWithFlags (qemu_driver.c:6420)
by 0xB93DE86: qemuDomainCreate (qemu_driver.c:6438)
by 0x4CECEA8: virDomainCreate (libvirt-domain.c:7142)
Now, you might think this may lead to a double free, because
@seclabel is freed under the 'cleanup' label (if @generated is
true). But if @generated is true, then just before calling the
callback there's VIR_APPEND_ELEMENT() which clears @seclabel out
turning the free under 'cleanup' label into a NOP.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Mon, 16 Jun 2025 08:28:37 +0000 (10:28 +0200)]
qemu: Be more forgiving when acquiring QUERY job when formatting domain XML
In my previous commit of v11.0.0-rc1~115 I've made QEMU driver
implementation for virDomainGetXMLDesc() (qemuDomainGetXMLDesc())
acquire QERY job. See its commit message for more info. But this
unfortunately broke apps witch fetch domain XML for incoming
migration (like virt-manager). The reason is that for incoming
migration the VIR_ASYNC_JOB_MIGRATION_IN async job is set, but
the mask of allowed synchronous jobs is empty (because QEMU can't
talk on monitor really). This makes virDomainObjBeginJob() fail
which in turn makes qemuDomainGetXMLDesc() fail too.
It makes sense for qemuDomainGetXMLDesc() to acquire the job
(e.g. so that it's coherent with another thread that might be in
the middle of a MODIFY job). But failure to dump XML may be
treated as broken daemon (e.g. virt-manager does so).
Therefore, still try to acquire the QUERY job (if job mask
permits it) but, do not treat failure as an error.
Fixes: 6cc93bf28842526be2fd596a607ebca796b7fb2e
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2369243 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Thu, 27 Jan 2022 13:12:49 +0000 (14:12 +0100)]
virsh: Introduce 'await' command for waiting until target domain state is reached
The new command is meant as syntax sugar for event handling which blocks
virsh until the requested state condition is reached.
The initial implementation adds a condition 'domain-inactive' returning
if the domain is/becomes inactive for whatever reason.
This command is useful for simple scripts e.g. for debugging libvirt
when it allows responding to target state in shell without the need to
fuss too much with polling or writing handlers around 'virsh event'.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 3 Jun 2025 15:25:36 +0000 (17:25 +0200)]
vsh: Add support for commands with more return values
Add a new handler callback for command handlers which will want to
return more than just EXIT_SUCCESS/EXIT_FAILURE.
The new handler allows returning integers. Any negative values are
converted to EXIT_FAILURE, other values are returned as reported in
cases where we forward the command state (non-interactive usage) as
return value of the virt shell program.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 3 Jun 2025 15:18:02 +0000 (17:18 +0200)]
vshCommandRun: Convert to directly return the exit code
Currently the handler functions in the virt shells return only a boolean
signalling if the command was successful or not. In preparation for a
command which will want to return another value (timeout) convert
vshCommand run to actually return the requested exit code instead and
document the conversion from boolean.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 5 Jun 2025 05:52:03 +0000 (07:52 +0200)]
virsh: cmdEvent: Ensure that event callbacks are unregistered before returning
Successful return from 'virConnectDomainEventDeregisterAny' does not
guarantee that there aren't still in-progress events being handled by
the callbacks. Since 'cmdEvent' passes in a slice from an array as the
private data of the callbacks, we must ensure that the array stays in
scope (it's auto-freed) for the whole time there are possible callbacks
being executed.
While in practice this doesn't happen as the callbacks are usually quick
enough to finish while unregistering stuff, placing a 'sleep(1)' into
e.g. 'virshEventLifecyclePrint' and starting a domain results in crash
of virsh with the following backtrace:
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00005557b5cfd343 in virshEventPrintf (data=data@entry=0x5557db9619b0, fmt=fmt@entry=0x5557b5d5e527 "%s") at ../../../libvirt/tools/virsh-domain-event.c:252
Thread 2 (Thread 0x7f59a54b7d00 (LWP 2097121)):
#0 0x00007f59a6cadbf9 in __futex_abstimed_wait_common () at /lib64/libc.so.6
#1 0x00007f59a6cb2cf3 in __pthread_clockjoin_ex () at /lib64/libc.so.6
#2 0x00005557b5cd57f6 in virshDeinit (ctl=0x7ffc7b615140) at ../../../libvirt/tools/virsh.c:408
#3 0x00005557b5cd5391 in main (argc=<optimized out>, argv=<optimized out>) at ../../../libvirt/tools/virsh.c:932
Thread 1 (Thread 0x7f59a51a66c0 (LWP 2097122)):
#0 0x00005557b5cfd343 in virshEventPrintf (data=data@entry=0x5557db9619b0, fmt=fmt@entry=0x5557b5d5e527 "%s") at ../../../libvirt/tools/virsh-domain-event.c:252
#1 0x00005557b5cffa10 in virshEventPrint (data=0x5557db9619b0, buf=0x7f59a51a55c0) at ../../../libvirt/tools/virsh-domain-event.c:290
#2 virshEventLifecyclePrint (conn=<optimized out>, dom=<optimized out>, event=<optimized out>, detail=<optimized out>, opaque=0x5557db9619b0) at ../../../libvirt/
[snipped]
From the backtrace you can see that the 'main()' thread is already
shutting down virsh, which means that 'cmdEvent' terminated and the
private data was freed. The event loop thread is still execing the
callback which accesses the data.
To fix this add a condition and wait on all of the callbacks to be
unregistered first (their private data freeing function will be called).
This bug was observed when I've copied the event code for a new virsh
command which had a bit more involved callbacks.
Fixes: 99fa96c3907 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 5 Jun 2025 13:12:40 +0000 (15:12 +0200)]
docs: Note that zero detection on migration sparsifies image only when discard='unmap' is set
The mirroring job clears the destination to ensure that the guest
visible disk contents are identical to the state on the source. The
image itself is kept sparse only when the disk 'discard' option is set
to 'unmap' (Also the disks would eventually desparsify itself anyways
with disabled discards). Note it in the docs for the user.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Acked-by: Eric Blake <eblake@redhat.com>
Peter Krempa [Fri, 6 Jun 2025 08:02:23 +0000 (10:02 +0200)]
qemu.conf: Improve docs for 'dynamic_ownership' option
Add a note that the user/group can be overriden or relabelling disabled
using per-vm/disk <seclabel> elements instead of disabling it globally.
Add a note that read-only image labels are not restored.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/512 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
docs: outline bug expectations wrt automated tools / AI agents
Bug reports from automated tools and AI agents are time consuming to
triage and have poor signal/noise ratio. Set strong expectations for
any reporters using such tools, in a (likely doomed) attempt to stem
the flow of poor quality reports.
Reviewed-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
We've recently stopped checking for the presence of various
commands at build time, which means that we no longer need
to have the corresponding packages installed in the build
environment.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Honglei Wang <honglei.wang@smartx.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Honglei Wang <honglei.wang@smartx.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Honglei Wang <honglei.wang@smartx.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu_capabilities: Add emulated NVMe disk support to domain capabilities
This is a separate commit for review ease, but who's really going to use
a libvirt with this patch in and the actual functionality missing, that
ain't gonna happen, right?
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Honglei Wang <honglei.wang@smartx.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu_capabilities: Add NVMe controller and disk capabilities
The "nvme" device is the controller and "nvme-ns" are the
namespaces (individual disks) plugged into it.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Honglei Wang <honglei.wang@smartx.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
NVMe disks are essentially a namespace of an NVMe controller, but to
make it easier for the users to just add a disk, the necessary details
like adding the proper controller, setting the serial number for the
controller based on the disk, are done automatically.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Honglei Wang <honglei.wang@smartx.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
We do not guarantee that the disk names will be the same in guest as
they are in the XML, but that should not stop us from trying to be
consistent with the naming. And since we use the same naming as the
linux kernel does, let's stick with it with nvme drives too.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
docs, conf, schemas: Add support for NVMe controller
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Honglei Wang <honglei.wang@smartx.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
nodedev: add nodedev name to mdevctl unsupport msg
Let's add the nodedev name to improve the error message for the user.
Signed-off-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
anonymix007 [Wed, 4 Jun 2025 09:05:23 +0000 (12:05 +0300)]
qemu: capabilities: Check if cpuModels is not NULL before trying to dereference it
accel->cpuModels field might be NULL if QEMU does not return CPU models.
The following backtrace is observed in such cases:
0 virQEMUCapsProbeQMPCPUDefinitions (qemuCaps=qemuCaps@entry=0x7f1890003ae0, accel=accel@entry=0x7f1890003c10, mon=mon@entry=0x7f1890005270)
at ../src/qemu/qemu_capabilities.c:3091
1 0x00007f18b42fa7b1 in virQEMUCapsInitQMPMonitor (qemuCaps=qemuCaps@entry=0x7f1890003ae0, mon=0x7f1890005270) at ../src/qemu/qemu_capabilities.c:5746
2 0x00007f18b42fafaf in virQEMUCapsInitQMPSingle (qemuCaps=qemuCaps@entry=0x7f1890003ae0, libDir=libDir@entry=0x7f186c1e70f0 "/var/lib/libvirt/qemu",
runUid=runUid@entry=955, runGid=runGid@entry=955, onlyTCG=onlyTCG@entry=false) at ../src/qemu/qemu_capabilities.c:5832
3 0x00007f18b42fb1a5 in virQEMUCapsInitQMP (qemuCaps=0x7f1890003ae0, libDir=0x7f186c1e70f0 "/var/lib/libvirt/qemu", runUid=955, runGid=955)
at ../src/qemu/qemu_capabilities.c:5848
4 virQEMUCapsNewForBinaryInternal (hostArch=VIR_ARCH_X86_64, binary=binary@entry=0x7f1868002fc0 "/usr/bin/qemu-system-alpha",
libDir=0x7f186c1e70f0 "/var/lib/libvirt/qemu", runUid=955, runGid=955,
hostCPUSignature=0x7f186c1e9f20 "AuthenticAMD, AMD Ryzen 9 7950X 16-Core Processor, family: 25, model: 97, stepping: 2", microcodeVersion=174068233,
kernelVersion=0x7f186c194200 "6.14.9-arch1-1 #1 SMP PREEMPT_DYNAMIC Thu, 29 May 2025 21:42:15 +0000", cpuData=0x7f186c1ea490)
at ../src/qemu/qemu_capabilities.c:5907
5 0x00007f18b42fb4c9 in virQEMUCapsNewData (binary=0x7f1868002fc0 "/usr/bin/qemu-system-alpha", privData=0x7f186c194280)
at ../src/qemu/qemu_capabilities.c:5942
6 0x00007f18bd42d302 in virFileCacheNewData (cache=0x7f186c193730, name=0x7f1868002fc0 "/usr/bin/qemu-system-alpha") at ../src/util/virfilecache.c:206
7 virFileCacheValidate (cache=cache@entry=0x7f186c193730, name=name@entry=0x7f1868002fc0 "/usr/bin/qemu-system-alpha", data=data@entry=0x7f18b67c37c0)
at ../src/util/virfilecache.c:269
8 0x00007f18bd42d5b8 in virFileCacheLookup (cache=cache@entry=0x7f186c193730, name=name@entry=0x7f1868002fc0 "/usr/bin/qemu-system-alpha")
at ../src/util/virfilecache.c:301
9 0x00007f18b42fb679 in virQEMUCapsCacheLookup (cache=cache@entry=0x7f186c193730, binary=binary@entry=0x7f1868002fc0 "/usr/bin/qemu-system-alpha")
at ../src/qemu/qemu_capabilities.c:6036
10 0x00007f18b42fb785 in virQEMUCapsInitGuest (caps=<optimized out>, cache=<optimized out>, hostarch=VIR_ARCH_X86_64, guestarch=VIR_ARCH_ALPHA)
at ../src/qemu/qemu_capabilities.c:1037
11 virQEMUCapsInit (cache=0x7f186c193730) at ../src/qemu/qemu_capabilities.c:1229
12 0x00007f18b431d311 in virQEMUDriverCreateCapabilities (driver=driver@entry=0x7f186c01f410) at ../src/qemu/qemu_conf.c:1553
13 0x00007f18b431d663 in virQEMUDriverGetCapabilities (driver=0x7f186c01f410, refresh=<optimized out>) at ../src/qemu/qemu_conf.c:1623
14 0x00007f18b435e3e4 in qemuConnectGetVersion (conn=<optimized out>, version=0x7f18b67c39b0) at ../src/qemu/qemu_driver.c:1492
15 0x00007f18bd69c5e8 in virConnectGetVersion (conn=0x55bc5f4cda20, hvVer=hvVer@entry=0x7f18b67c39b0) at ../src/libvirt-host.c:201
16 0x000055bc34ef3627 in remoteDispatchConnectGetVersion (server=0x55bc5f4b93f0, msg=0x55bc5f4cdf60, client=0x55bc5f4c66d0, rerr=0x7f18b67c3a80,
ret=0x55bc5f4b8670) at src/remote/remote_daemon_dispatch_stubs.h:1265
17 remoteDispatchConnectGetVersionHelper (server=0x55bc5f4b93f0, client=0x55bc5f4c66d0, msg=0x55bc5f4cdf60, rerr=0x7f18b67c3a80, args=0x0, ret=0x55bc5f4b8670)
at src/remote/remote_daemon_dispatch_stubs.h:1247
18 0x00007f18bd5506da in virNetServerProgramDispatchCall (prog=0x55bc5f4cae90, server=0x55bc5f4b93f0, client=0x55bc5f4c66d0, msg=0x55bc5f4cdf60)
at ../src/rpc/virnetserverprogram.c:423
19 virNetServerProgramDispatch (prog=0x55bc5f4cae90, server=server@entry=0x55bc5f4b93f0, client=0x55bc5f4c66d0, msg=0x55bc5f4cdf60)
at ../src/rpc/virnetserverprogram.c:299
20 0x00007f18bd556c32 in virNetServerProcessMsg (srv=srv@entry=0x55bc5f4b93f0, client=<optimized out>, prog=<optimized out>, msg=<optimized out>)
at ../src/rpc/virnetserver.c:135
21 0x00007f18bd556f77 in virNetServerHandleJob (jobOpaque=0x55bc5f4d2bb0, opaque=0x55bc5f4b93f0) at ../src/rpc/virnetserver.c:155
22 0x00007f18bd47dd19 in virThreadPoolWorker (opaque=<optimized out>) at ../src/util/virthreadpool.c:164
23 0x00007f18bd47d253 in virThreadHelper (data=0x55bc5f4b7810) at ../src/util/virthread.c:256
24 0x00007f18bce117eb in start_thread (arg=<optimized out>) at pthread_create.c:448
25 0x00007f18bce9518c in __GI___clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78
Michal Privoznik [Mon, 26 May 2025 08:41:00 +0000 (10:41 +0200)]
libxl_capabilities: Make some functions return void
Inside of libxlMakeDomainCapabilities() there are some functions
called and basically all of them never return anything but zero
(indicating success). Yet, they are called in a fashion that
suggests otherwise. Turn those functions into void and drop
checks for their retvals.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
projects / thirdparty / libvirt.git / log
