If libvirt is built in client only mode, the libvirtd/virtqemud/etc
daemons won't exist. If the client is told to connect to a local
hypervisor, it'll see the socket doesn't exist, try to spawn the
daemon and then re-try connecting to the socket for a few seconds.
Ultimately this will fail because the daemon doesn't exist and the
user gets an error message
error: Failed to connect socket to '/run/user/1000/libvirt/virtqemud-sock': No such file or directory
technically this is accurate, but it doesn't help identify the root
cause. With this change it will now report
error: binary 'virtqemud' does not exist in $PATH: No such file or directory
and will skip all the socket connect retries
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
On device-update, when user requests change of
trustGuestRxFilters we currently do nothing. Nor error out, nor
act on the request. While we can just throw an error,
implementing this is pretty trivial.
Resolves: https://issues.redhat.com/browse/RHEL-735 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuMonitorJSONQueryRxFilter: Allow @filter to be NULL
Sometimes it may be handy to just issue the query-rx-filter
monitor command without actually parsing the output. Adapt
qemuMonitorJSONQueryRxFilter() to this behavior.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Thu, 30 Nov 2023 12:26:14 +0000 (13:26 +0100)]
qemu: Relax check for memory device coldplug
When cold plugging a memory device we check whether there's
enough free memory slots to accommodate new module. Well, this
checks makes sense only for those memory devices that are plugged
into DIMM slots (DIMM and NVDIMM models). Other memory device
models, like VIRTIO_MEM, VIRTIO_PMEM or SGX_EPC are attached into
PCI bus, or no bus at all.
Resolves: https://issues.redhat.com/browse/RHEL-15480 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
qemu: Move memory device coldplug into a separate function
The code that handles coldplug of a memory device is pretty
trivial and such could continue to live in the huge switch()
where other devices are handled. But the code is about to get
more complicated. To help with code readability, move it into a
separate function.
And while at it, make the function accept a double pointer to the
memory device definition to make the ownership transfer obvious
(the device is part of the domain on successful run).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Andrea Bolognani [Thu, 30 Nov 2023 17:22:54 +0000 (18:22 +0100)]
rpm: Add Requires on scrub
We currently have a BuildDepends on it, which doesn't help
ensuring that the tool is available at runtime, when the storage
driver might need to spawn it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Andrea Bolognani [Wed, 29 Nov 2023 16:33:35 +0000 (17:33 +0100)]
rpm: Drop MinGW BuildRequires on libgcrypt/libgpg-error
They've been there since the start, but we don't seem to be using
them for anything. The native part doesn't have BuildRequires on
the native equivalents.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
The fact that we use an external search provider instead of a
built-in search functionality might come as a surprise to users,
and some of them might not be comfortable with the arrangement.
Mention it in the search interface to avoid surprises.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Ideally we'd just perform the search ourselves, but as long as
we have to rely on an external provider, at least make it a
somewhat privacy-conscious one.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Having the name of the project as part of the prefix was useful
back when we used the mailing list for all subprojects, but
these days the only patches that are sent to the list are for
the main library so it no longer makes sense to include this.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Tue, 28 Nov 2023 15:06:30 +0000 (16:06 +0100)]
qemu_command: Don't open code virPCIDeviceAddressAsString()
When building a hostdev props, its PCI address is formatted via
g_strdup_printf(VIR_PCI_DEVICE_ADDRESS_FMT, ...); Well, we have a
function that does exactly that: virPCIDeviceAddressAsString().
Use the latter.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
test_driver: Remove unused variable in testDomainAttachDeviceLiveAndConfig()
The hotplug functionality added earlier really supports only live
addition of devices, no coldplug yet rendering @devConf in
testDomainAttachDeviceLiveAndConfig() an unused variable. Remove
it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Thu, 30 Nov 2023 18:34:55 +0000 (19:34 +0100)]
rpcgen: Skip generator tests on macOS
The generator can produce different code on Linux and macOS:
specifically, on the former we want to use xdr_uint64_t while
the latter needs xdr_u_int64_t instead.
This is clearly a problem for tests that involve comparing the
output produced against some expected output that's stored in
the git repository.
In the long run, we need to find a better way to handle this,
but since 9.10.0 is going to be released very soon and we don't
want it to have a broken test suite on macOS, simply skip the
generator tests on that platform for now.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
macOS XDR library is an oddball using xdr_u_int64_t instead of
xdr_uint64_t which everyone else has.
The code generator already does the right thing, but the test
program previously generated with the Linux rpcgen program
does not compile on macOS due to this.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Andrea Bolognani [Wed, 29 Nov 2023 14:50:30 +0000 (15:50 +0100)]
rpcgen: Don't skip all tests when pytest is missing
Some of the files used by test_demo.c can only be regenerated
when pytest is present, but we have pre-generated copies in the
repository, so overall we just need the C compiler to build and
run that specific test program.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Peter Krempa [Fri, 1 Dec 2023 09:28:48 +0000 (10:28 +0100)]
qemuProcessStartWithMemoryState: Don't start qemu with '-loadvm SNAP' and '-incoming defer' together
A bug in qemuProcessStartWithMemoryState caused that we would start qemu
with '-loadvm SNAP' and '-incoming defer' together. qemu doesn't expect
that and crashes on an assertion failure [1].
[1]: https://issues.redhat.com/browse/RHEL-16782
Fixes: 8a88d3e5860881f430e528d3e5e8d6455ded4d1d
Resolves: https://issues.redhat.com/browse/RHEL-17841 Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
Michal Privoznik [Wed, 29 Nov 2023 11:30:16 +0000 (12:30 +0100)]
rpcgen: tests: Run cleanly on platforms where char is unsigned
There are some platforms where 'char' is unsigned, by default
(RPi, s390x to name a few). And because of how test_demo is
written we are experiencing some test cases failing there. For
instance: /xdr/struct-scalar is failing. This is because in the
test (test_struct_scalar()), we have a struct with two chars. One
is initialized to 0xca, the other 0xfe (note that both have the
MSB set). The XDR encoder (xdr_TestStructScalar()) then calls
xdr_char() on both of them. But XDR itself has no notion of
char type, so under the hood, it expands it to int [1] and calls
xdr_int(). And this is where the problem lies. On platforms where
char is signed, the integer expansion results in 0xffffffca, but
on platforms where char is unsigned it results in 0x000000ca. Two
distinct results.
The test then goes and compares the encoded buffer with an
expected one (memcmp(), read from the disk earlier).
This poses no problem for real life use, because when decoding
those chars back, the padding is thrown away.
To avoid tickling this issue, use values that don't have the MSB
set.
Fixes: 40cbaa8fbe rpcgen: add test case for XDR serialization Reported-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
Michal Privoznik [Wed, 29 Nov 2023 11:22:21 +0000 (12:22 +0100)]
rpcgen: tests: Allow running test_demo from anywhere
The test_demo program compares whether XDR encoded data match the
expected output as read from a file. But the file path is not
absolute and thus relative to CWD which means the program can run
only from one specific directory.
Do what we do in the rest of our test suite: define 'abs_srcdir'
macro and prefix the path with it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Michal Privoznik [Tue, 28 Nov 2023 15:35:22 +0000 (16:35 +0100)]
ch: Don't leak ch_driver->chCaps
During CH driver initialization (chStateInitialize()) the
driver's capabilities bitmap is allocated
(virCHCapsInitCHVersionCaps()), but corresponding free call is
missing in chStateCleanup().
And while at it, reorder calls to virObjectUnref() inside of
chStateCleanup() to be the reverse order of that in
chStateInitialize() so that it's easier to spot missing
free/unref call.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Jonathon Jongsma <jjongsma@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Tue, 28 Nov 2023 17:51:04 +0000 (18:51 +0100)]
rpm: Drop rpcgen Build-Requires once again
We no longer use it, so commit a62486b95fee correctly dropped
the Build-Requires; shortly afterwards, however, I accidentally
reintroduced it by mistake.
Fixes: 3df8cc658ed0ac2f84089ad0db61ba20eb8b8aa7 Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Peter Krempa [Tue, 14 Nov 2023 16:19:10 +0000 (17:19 +0100)]
qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource: Don't setup 'raw' layer for migration NBD connection
The raw driver layer is not needed in this case and can be dropped.
Removing the nodename will cause other pieces of the code to pick up and
stop adding the layer.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Tue, 14 Nov 2023 16:07:44 +0000 (17:07 +0100)]
qemuMigrationSrcNBDCopyCancel: Use qemuBlockStorageSourceAttachRollback to detach migration NBD blockdevs
Rewrite the code to use the common tooling for removing blockdevs
instead of the ad-hoc qemuBlockStorageSourceDetachOneBlockdev helper.
Use of the common infrastructure will properly handle cases when the raw
driver is ommited from the block graph.
Since the TLS data object is shared for all migration QMP commands and
objects we need to strip its alias from the definition of the storage
source before attempting to detach it.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 22 Nov 2023 13:42:26 +0000 (14:42 +0100)]
qemu: monitor: Sanitize arguments of qemuMonitorBlockdevReopen
Take the virJSONValue array object which is passed to the
'blockdev-reopen' command as the 'options' argument rather than making
the caller wrap all the properties.
The code was a leftover from the time when the blockdev-reopen command
had a different syntax, and thus can be cleaned up.
Also note that the logging of the node name never worked as the top
level object didn't ever contain a 'node-name' property.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 22 Nov 2023 13:27:19 +0000 (14:27 +0100)]
qemu: block: Extract logic from qemuBlockReopenReadWrite/ReadOnly
We want to preserve the wrappers for clarity but the inner logic can be
extracted to a common function qemuBlockReopenAccess. In further patches
the code from qemuBlockReopenFormat will be merged into the new wrapper
as well as logic for handling scenarios with missing 'format' layers
will be added.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 3 Nov 2023 13:30:35 +0000 (14:30 +0100)]
qemuDomainPrepareStorageSourceBlockdevNodename: Restructure code to allow missing 'format' layer
Similarly to other bits of code, we don't need to setup the format layer
if it will not be formatted. Add logic which uses
qemuBlockStorageSourceNeedsFormatLayer to see whether the setup of the
format node is needed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 3 Nov 2023 13:29:00 +0000 (14:29 +0100)]
qemuBlockStorageSourceDetachPrepare: Prepare for possibly missing 'format' layer
Setup the data for detaching of the 'format' layer only when it's
present.
Restructure the logic to follow the same order as
qemuBlockStorageSourceAttachPrepareBlockdev in terms of
format/slice/storage -blockdev objects, and drop the now-misleading
comment for 'slice' of raw disks.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 3 Nov 2023 13:26:35 +0000 (14:26 +0100)]
qemuBlockStorageSourceAttachPrepareBlockdev: Prepare for optionally missing format layer
Restructure the code logic so that the function is prepared for the
possibility that the 'format' blockdev layer may be missing if not
needed.
To achieve this we need to introduce logic that selects which node
(format/slice/storage) becomes the effective node and thus formats the
correct set of arguments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 24 Nov 2023 09:18:56 +0000 (10:18 +0100)]
qemu: block: Use qemuBlockStorageSourceNeedsStorageSliceLayer only for setup
Add a note stating that qemuBlockStorageSourceNeedsStorageSliceLayer
must be used only when setting up a new blockdev, any other case when
the device might been already set up must use the existence of the
nodename to do so.
Adjust qemuBlockStorageSourceAttachPrepareBlockdev to do so and refactor
qemuBlockStorageSourceDetachPrepare to use the same logic.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 23 Nov 2023 21:37:46 +0000 (22:37 +0100)]
storage_file_probe: Treat qcow2 images with protocol drivers in backing store field as raw
qemu allows and in some cases uses protocol driver names ('file',
'host_device', 'nbd', ...) in the 'backing file format' field of a qcow
to denote a image where the dummy 'raw' driver was not used on top.
Adapt our backing store parser for such cases. The examples added in
previous patch show the difference in behaviour.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 23 Nov 2023 19:42:54 +0000 (20:42 +0100)]
virstoragetest: Add test cases for QCOW2 files with a protocol name as backing file format
QEMU allows and in cases where you omit the not-strictly-needed 'raw'
driver on top of raw images automatically uses the protocol name inside
of the 'backing file format' field of the qcow2 image.
Libvirt expects only format names in that field.
Add example images showing this scenario, which will be fixed later.
The qcow2 image files in this commit were formatted as:
Peter Krempa [Fri, 24 Nov 2023 10:22:57 +0000 (11:22 +0100)]
virstoragetest: Format detected/unprocessed backing store format into output files
Compare also the detected format of the backing file
('backingStoreRawFormat' field) into the output data for comparison with
others. Since the ToString function can't convert VIR_STORAGE_FILE_AUTO
use also the numeric value.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 24 Nov 2023 08:26:39 +0000 (09:26 +0100)]
storage_file_probe: Remove BACKING_STORE_OK,BACKING_STORE_INVALID states
Replace the return values by 0 because none of the callers care and some
of the backing store parser functions return this state also in cases
the rest of the code would consider as success.
Subsequently the parsers will be refactored and proper error reporting
returned.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 23 Nov 2023 16:03:32 +0000 (17:03 +0100)]
virStorageFileProbeGetMetadata: Do not partially skip probing of the image
Since we consider the failure of parsing the backing store to be
actually success based on the value we return to the caller, we should
continue parsing also features and the 'compat' field so that we don't
have a partial definition if e.g. the backing store format is not known.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Sat, 25 Nov 2023 07:35:00 +0000 (08:35 +0100)]
bhyve: Add missing enum member to switch() in bhyveBuildSoundArgStr()
In v9.9.0-104-gc472ce024b I've introduced another value to
virDomainAudioType enum. But I forgot to add corresponding case
into switch() in bhyveBuildSoundArgStr().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Wed, 10 May 2023 10:58:44 +0000 (12:58 +0200)]
qemu: Generate cmd line for pipewire audio backend
This is mostly straightforward, except for a teensy-weensy
detail: usually, there's no system wide daemon running, no system
wide available socket that anybody could connect to. PipeWire
uses a per user daemon approach instead. But this in turn means,
that the socket location floats between various locations and is
derived from various environment variables (just like the actual
socket name) and thus we must pass the variables to QEMU.
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/560 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Wed, 10 May 2023 10:29:54 +0000 (12:29 +0200)]
conf: Introduce pipewire audio backend
QEMU gained support for PipeWire audio backend (see QEMU commit
of v8.0.0-403-gc2d3d1c294). Its configuration knobs are basically
the same as pulseaudio's, except for PA's server name. Therefore,
a lot of code is copied over from pulseadio and fixed by
s/Pulse/Pipewire/ or s/pulseaudio/pipewire/.
There's one ley difference to PA though: pipewire daemon is
usually on per user basis (just like our qemu:///session).
Therefore, introduce this 'runtimeDir' attribute, which allows
specifying path to pipewire daemon socket (useful for
qemu:///system for instance).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Fri, 24 Nov 2023 10:59:32 +0000 (11:59 +0100)]
virnuma: Avoid integer overflow in virNumaGetPages()
On systems with humongous pages (16GiB) and 32bit int it's easy
to hit integer overflow in virNumaGetPages(). What happens is,
inside of virNumaGetPages() as we process hugepages for given
NUMA node (e.g. in order to produce capabilities XML), we keep a
sum of sizes of pools in an ULL variable (huge_page_sum). In each
iteration, the variable is incremented by 1024 * page_size *
page_avail. Now, page_size is just an uint, so we have:
ULL += U * U * ULL;
and because of associativity, U * U is computed first and since
we have two operands of the same type, no type expansion happens.
But this means, for humongous pages (like 16GiB) the
multiplication overflows.
Therefore, move the multiplication out of the loop. This helps in
two ways:
1) now we have ULL += U * ULL; which expands the uint in
multiplication,
2) it saves couple of CPU cycles.
Resolves: https://issues.redhat.com/browse/RHEL-16749 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 23 Nov 2023 12:43:36 +0000 (13:43 +0100)]
qemu: block: Don't try to merge bitmaps into 'raw' images
If any of the images in a chain above a raw image have bitmaps, libvirt
would attempt to merge them when doing a block commit or block copy
operation, which would result into a error in the logs as creating
persistent bitmaps in a raw image is not supported.
Since libvirt cares only about persistent bitmaps we can simply skip the
operation if the target of a block copy or block commit is a raw image.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 23 Nov 2023 13:07:48 +0000 (14:07 +0100)]
qemu: hotplug: Detect disk backing images before setting up security access
The VM will require access also to the detected images. Unfortunately a
recent reordering of the code introduced a bug where the backing chain
was probed after setting up cgroups/selinux/namespaces, which caused
that any detected images were not allowed/added and qemu was then not
able to use them.
Fixes: 9b8bb536ff999fa61e41869bd98a026b8e23378f Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
virDomainMemoryDefCheckConflict: Check dimm & nvdimm models too
So far we check whether virtio-mem and/or virtio-pmem memory
devices do not overlap with each other. But we allow specifying
address where dimm and nvdimm memory devices are mapped too. And
there are left out from this collision check. Not anymore.
This leaves just sgx model out, but that's expected since it
can't have any address (see virDomainMemoryDefValidate()).
Resolves: https://issues.redhat.com/browse/RHEL-4452 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
domain_validate: Move memdevice address conflict check into a separate function
At the end of virDomainMemoryDefValidate() there's a code that
checks whether two virtio-mem/virtio-pmem devices don't overlap.
Separate this code into its own function
(virDomainMemoryDefCheckConflict()).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Tim Wiederhake [Thu, 9 Nov 2023 13:37:19 +0000 (14:37 +0100)]
cpu_map: Add missing vmx features from MSR 0x48C
Differences from qemu:
* "vmx-ept-uc" (bit 8) and "vmx-ept-wb" (bit 14) are not added to
qemu's list of named features yet, but used in several qemu cpu
models never the less. Add to libvirt regardless.
* "vmx-invvpid-single-context" (bit 41) is erroneously called
"vmx-invept-single-context" in qemu. This is the name of the
feature associated with bit 25 in both libvirt and qemu.
* "vmx-invvpid-single-context-noglobals" (bit 43) is erroneously
called "vmx-invept-single-context-noglobals". Use the correct name.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Tim Wiederhake [Thu, 9 Nov 2023 12:19:41 +0000 (13:19 +0100)]
cpu_map: No longer ignore vmx- features in sync_qemu_features_i386.py
Some guest OSes require cpu features from the vmx-* family,
e.g. vmx-xsaves. Up to now, libvirt ignored these features as they
were not required yet. qemu does not automatically enable e.g.
"vmx-xsaves" when requesting "xsaves":
Michal Privoznik [Wed, 22 Nov 2023 13:58:49 +0000 (14:58 +0100)]
lib: Replace qsort() with g_qsort_with_data()
While glibc provides qsort(), which usually is just a mergesort,
until sorting arrays so huge that temporary array used by
mergesort would not fit into physical memory (which in our case
is never), we are not guaranteed it'll use mergesort. The
advantage of mergesort is clear - it's stable. IOW, if we have an
array of values parsed from XML, qsort() it and produce some
output based on those values, we can then compare the output with
some expected output, line by line.
But with newer glibc this is all history. After [1], qsort() is
no longer mergesort but introsort instead, which is not stable.
This is suboptimal, because in some cases we want to preserve
order of equal items. For instance, in ebiptablesApplyNewRules(),
nwfilter rules are sorted by their priority. But if two rules
have the same priority, we want to keep them in the order they
appear in the XML. Since it's hard/needless work to identify
places where stable or unstable sorting is needed, let's just
play it safe and use stable sorting everywhere.
Fortunately, glib provides g_qsort_with_data() which indeed
implement mergesort and it's a drop in replacement for qsort(),
almost. It accepts fifth argument (pointer to opaque data), that
is passed to comparator function, which then accepts three
arguments.
We have to keep one occurance of qsort() though - in NSS module
which deliberately does not link with glib.
1: https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=03bf8357e8291857a435afcc3048e0b697b6cc04 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
projects / people / aperard / libvirt.git / log