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>
Currently, domain capabilities do not include information about the
supported console device types. While most of the drivers support
'pty' console type, it's not the case for bhyve. Without this
information, management software cannot always generate compatible
domain configuration.
To address that, extend domain capabilities like that:
Previously, the RDP graphics definition parsing were implemented by
string parsing, the virDomainGraphicsDefParseXMLRDP function is
refactored to use the appropriate virXMLProp* utility functions.
Overall parsing logic was not changed and results the same output as
before.
Moreover, 'replaceUser' and 'mutliUser' params type was changed from
bool to tristate type, to avoid unnecessary type convertions.
Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Previously, the fullscreen option were parsed as a tristate but stored
as a bool type, changed the fullscreen option type to tristate bool to
avoid unnecessary type convertions.
Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Previously, the VNC graphics definition parsing were implemented by
string parsing, the virDomainGraphicsDefParseXMLVNC was refactored
to use the appropriate virXMLProp* utility functions. Overall
parsing logic was not changed and results the same output as before.
Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemu: Don't accept VIR_DUMP_LIVE flag in qemuDomainCoreDumpWithFormat()
QEMU can't really do live dumps of guest memory. It's because
inside of dump_init() the vm_stop() is called basically
unconditionally (the only condition is whether vCPUs are
running). Hence, there is no way for us to do live dumps and thus
honor VIR_DUMP_LIVE flag. Instead of silently pretending the flag
works, reject it with appropriate error message.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/646 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Mon, 2 Jun 2025 14:55:16 +0000 (16:55 +0200)]
qemu: command: Don't attempt to set backend MTU for networks which don't use host backend directly
Attempting to set MTU for network types which don't actually use the
network device on the host results in a failure. The 'mtu' property is
also used e.g. for the 'host_mtu' property of e.g. 'virtio-net-pci'
which is applied even in vhost-user mode.
Use the existing switch which selects devices without a network device
backend on the host side and skip setting the MTU.
Tested by running 'passt' in vhost-user mode manually:
Peter Krempa [Mon, 2 Jun 2025 08:26:38 +0000 (10:26 +0200)]
esx: Avoid corner case where esxUtil_ParseDatastorePath could be called with NULL 'datastorePath'
The generated code which parses the data from XML in
esxVI_LookupDatastoreContentByDatastoreName can fill the 'folderPath'
property with NULL if it were missing from the input XML. While this is
not likely when talking to esx it is a possible outcome. Skipp NULL
results.
All other code paths already ensure that the function is not called with
NULL.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/776 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Mon, 2 Jun 2025 08:56:32 +0000 (10:56 +0200)]
docs: Change units to 'kiB' from 'kB'/'kilobytes'/'kb'
Use the short unit for kibibytes instead of the confusing or plainly
wrong units.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/594 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Wed, 28 May 2025 15:36:13 +0000 (17:36 +0200)]
storage_file_probe: Call qcow2GetFeatures from qcow2GetImageSpecific
Parse qcow2 feature flags from qcow2GetImageSpecific. To achieve that
qcow2GetFeatures is refactored to take virStorageSource directly and
fill the data. To avoid the need to pass 'format' the parsing of the
qcow2 version is changed to access the offset directly (same as in
qcow2GetExtensions)
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Move the two functions to where they will be used. Subsequent patches
will refactor the control flow so that they will no longer be declared
ahead of time. Moving them in a separate patch will make the changes in
the refactor more clear to see.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 28 May 2025 15:26:13 +0000 (17:26 +0200)]
storage_file_probe: Move logic from qcow2GetClusterSize to qcow2GetImageSpecific
Move the cluster size parser into the image specific code for qcow2,
which will later allow us to remove the callback for cluster size which
is not used by any other format.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 28 May 2025 15:18:36 +0000 (17:18 +0200)]
storage_file_probe: Refactor qcowXGetBackingStore into specific callbacks for qcow and qcow2
Change qcowXGetBackingStore to use the new function prototype which
fills virStorageSource directly. Convert the copying of the backing file
string from 'g_new0' + 'memcpy' to 'g_strndup' as we treat it as a
string.
Introduce qcowGetImageSpecific (as a wrapper for qcowXGetBackingStore)
and qcow2GetImageSpecific. The latter of the two will be used to collect
all the qcow2-specific code later on, but for now it just parses the
backing store and the format.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 28 May 2025 13:59:33 +0000 (15:59 +0200)]
storage_file_probe: Add image specific callback taking the whole virStorageSource
The callbacks getting just some fields are not flexible and in some
cases cause the metadata to be probed multiple times.
Add a callback that will pass the whole virStorageSource struct being
probed so that the code can be written more efficiently.
As a first step we add just the callback. The specific format helpers
will be refactored and subsequently all the other callbacks will be
removed.
To simplify the refactors that will convert all the code to the new
callbacks the new callback is placed first but the calls to cleanup
previous metadata are moved before it. They'll be removed once the
refactors are complete together with the other callbacks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 29 May 2025 08:31:46 +0000 (10:31 +0200)]
virstoragetest: Add qcow2 bitmaps to some images
Add change block tracking bitmaps to some of the qcow2 images, to ensure
that they work with our qcow2 header parser even when we don't parse
them ourselves.
There are 3 bugs in the qcow2 header extension parser:
1) Header extension padding not taken into account
Per qcow2 documentation (qemu.git/docs/interop/qcow2.txt, also now
mirrored in the comment explaining the parser) each header extension
entry is padded to a multiple of 8 bytes.
Our parser didn't take the padding into account and advanced the
current offset only by the 'length', which corresponds to the length
of the data.
This meant that in vast majority of cases only the first extension
would be parsed correctly. For any other one we'd try to fetch the
magic and length from wrong place.
Luckily this wasn't a problem for most of the almost 15 years this bug
existed as we only cared about the backing file format string header
which was always stored first by qemu.
It is now a problem in the very specific case when a qcow2 image has a
'data-file' and also a backing store with format. In such case we'd
parse the backing store format properly as it's the first header and
'data-file' being the second would be skipped.
The buffer bounds checks were correct so we didn't violate any memory
boundaries.
2) Integer underflow in calculation of end of header extension block
If the image doesn't have a backing image, the 'backing_file_offset'
qcow2 header field is 0. We use that value as 'extensions_end' which
is used in the while loop to parse the extension entries.
The check was done as "offset < (extensions_end - 8)", thus it
unreflows when there's no filename.
The design of the loop prevented anything bad from happening though.
3) Off-by-one when determining end of header extension block
The aforementioned end of extension check above also has an off-by-one
error as it allowed another loop if more than 8 bytes were available.
Now the termination entry has data length of 0 bytes so we'd not be
able to properly process that one.
This wasn't a problem either as for now there's just the terminator
having 0 byte length.
This patch improves documentation by quoting the qcow2 interoperability
document and adjusts the loop condition and length handling to comply
with the specs.
Interestingly we also had a test case for this specific scenario but the
expected test output was wrong.
Fixes: a93402d48b2996e5300754d299ef0d3f646aa098
Resolves: https://issues.redhat.com/browse/RHEL-93775 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Sun, 25 May 2025 06:12:48 +0000 (08:12 +0200)]
qemu.conf: Document options for VxHS block network protocol TLS config as ignored
qemu-5.2 dropped support for VxHS. As we now require at least qemu-6.2,
the qemu.conf option for setting up TLS for VxHS are no longer used.
Document them as such.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Peter Krempa [Sun, 25 May 2025 06:18:21 +0000 (08:18 +0200)]
qemu: block: Drop code for 'vxhs' storage protocol
qemu-5.2 dropped support for the 'vxhs' protocol. We require qemu-5.2
since commit ce48d584cc4 and thus the block code for vxhs is now dead.
Remove it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Jiri Denemark [Tue, 3 Jun 2025 08:33:03 +0000 (10:33 +0200)]
util: Avoid statfs in virFileGetExistingParent
The code was separated from virFileIsSharedFSType which is Linux-only,
but virFileGetExistingParent is also called from
virFileIsSharedFSOverride which is OS independent. Thus we can't use
statfs. Let's use virFileExists (access) instead, we were not interested
in anything but success/failure from statfs anyway.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Jiri Denemark [Wed, 28 May 2025 14:37:32 +0000 (16:37 +0200)]
util: Fix virFileIsSharedFSOverride on nonexistent paths
Commit v11.0.0-162-gf2023e8018 added path canonicalization to
virFileIsSharedFSOverride to make sure we can properly match shared
filesystem override paths which include symlinks. But
virFileCanonicalizePath only works on existing paths, while
virFileIsSharedFSOverride may be called on paths that do not exist yet.
Matching paths against overrides has always worked even for nonexistent
paths. To fix the regression we need to first get the longest existing
sub-path and canonicalize only this part and use the result for
searching in overrides. Clearly any portion of the path we dynamically
create is not going to end up on a different filesystem to what the
parent directory is stored in. So checking just the existing parent is
enough.
https://issues.redhat.com/browse/RHEL-86592
Fixes: f2023e8018fe18550ad6aec66fe72bd1376f8522 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jiri Denemark [Wed, 28 May 2025 14:36:59 +0000 (16:36 +0200)]
util: Introduce virFileGetExistingParent helper
The code from virFileIsSharedFSType which finds the longest existing
path for a given input is separated into a new helper so that it can be
reused elsewhere.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jiri Denemark [Tue, 27 May 2025 13:42:36 +0000 (15:42 +0200)]
qemu: Fix error when migration with shared TPM storage is unsupported
The VIR_ERR_NO_SUPPORT error is supposed to be used for unsupported
driver APIs. It is incorrectly used when swtpm does not support
migration with shared storage resulting in a rather strange error
message:
this function is not supported by the connection driver: the running
swtpm does not support migration with shared storage
The correct VIR_ERR_OPERATION_UNSUPPORTED error code provides a much
better message:
Operation not supported: the running swtpm does not support
migration with shared storage
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
projects / thirdparty / libvirt.git / log
