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>
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>
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>
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>
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>
The virDomainMemoryDefCheckConflict() already does the same set
of checks. There's no need to duplicate them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Since we're iterating over def->mems array, might as well check
for dimm slot duplicates.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
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>
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>
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>
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":
qmp="qemu-kvm -machine accel=kvm -nodefaults -nographic -qmp stdio"
$(qmp) <<-EOF | jq | grep "xsaves"
{ "execute": "qmp_capabilities" }
{
"execute": "query-cpu-model-expansion",
"arguments": {
"type": "full",
"model": {
"name": "Skylake-Client-v1",
"props": { "xsaves": true } `# set to "true" or "false"`
}
}
}
{ "execute": "quit" }
EOF
with xsaves "false":
"xsaves": false,
"vmx-xsaves": false,
with xsaves "true":
"xsaves": true,
"vmx-xsaves": false,
Stop ignoring vmx-* features and begin adding them to libvirt's
database.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
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>
Originally the disk hotplug code didn't know how to attach a CD-ROM
drive, thus didn't have the necessary logic to handle empty cdroms.
Other disks can't be empty which is enforced by the parser validation
logic.
When support for hotplugging cdroms was added the code was not adjusted
to deal with empty drives thus attempted to setup the blockdev backend
for it.
Fixes: 3078799fef82d45ac10624e3bacded7a285d8a4f
Resolves: https://issues.redhat.com/browse/RHEL-16870
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Commit allowing hotplug of CDROMs moved the logic forbidding the hotplug
to the appropriate blocks based on the disk frontend but forgot to
actually bail out on such error.
Fixes: 3078799fef82d45ac10624e3bacded7a285d8a4f
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When I've originally refactored the function in commit 0d981bcefcb5defa2
the logic was still correct, but then later in commit 52f865543920b0
I've moved most of the image setup logic into the function neglecting to
add the 'goto cleanup;' needed to skip over the setup of the disk
images.
Fixes: 52f865543920b0cc5ba93f4407c1b2efdffb8ddc
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reword the error message to clearly state that the machine type doesn't
support the address type. It doesn't matter which device it's for.
Additionally the alias may be still NULL at the point when the error is
being reported misleading users that they have something wrong with a
specific device.
Resolves: https://issues.redhat.com/browse/RHEL-16878
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
A domain name is expected to be non-empty, and we validate this when
parsing XML, or accepting a new name during renames. We fail to
enforce this property, however, when performing a migration. This
was discovered when a user complained about inaccessible VMs after
migrating with the Rust APIs which mistakenly hardcoded 'dname' to
the empty string.
Fixes: https://gitlab.com/libvirt/libvirt-rust/-/issues/11
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
As a guard against programming errors, one part of the condition
only dereferences srcpool if it exists, other one does not.
Move the check up one level so that it actually has a chance to do
something useful.
Fixes: 19b1c0d31988a3a10c4694c10c27eb15c018f450
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Now that we have virXMLParseWithIndent() and
virXMLParseStringCtxtWithIndent(), we can use them directly and
drop calls to xmlKeepBlanksDefault().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When parsing an XML it may be important to keep indentation to
produce a better looking result when formatting the XML back.
Just look at all those xmlKeepBlanksDefault() calls just before
virXMLParse() is called.
Anyway, as of libxml2 commit v2.12.0~108 xmlKeepBlanksDefault()
is deprecated. Therefore, introduce virXMLParse...WithIndent()
variants which would do exactly xmlKeepBlanksDefault() did but
with non-deprecated APIs.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The virXMLParseHelper() can work in two modes: either it parses a
file or a string. Either way, the same set of flags is specified
in call of corresponding function. Save flags in a local variable
instead.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After libxml2's commit of v2.12.0~101 we no longer get
xmlIndentTreeOutput declaration by us including just
libxml/xpathInternals.h and libxml2's header files leakage.
Resolves: https://bugs.gentoo.org/917516
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
As mentioned in previous commit, VirtualBox has its own snapshot
XML which we parse, change and then format back. During this, we
ought to keep the indentation to produce better looking result
(especially when we want to compare the output in tests later on,
like we do in vboxsnapshotxmltest).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When working with VirtualBox's snapshots, the snapshot XML is
firstly parsed, stored in memory (with some parts being stored as
verbatim XML snippets, strings), requested changes are made and
then this modified XML is formatted via
virVBoxSnapshotConfSaveVboxFile() which calls
xmlParseInNodeContext() to format those previously stored XML
snippets.
The first parse of whole VirtualBox snapshot file is done using
virXMLParse() (in virVBoxSnapshotConfLoadVboxFile()) and thus
with XML_PARSE_NONET specified.
But those ad-hoc parsings when formatting the XML back pass zero
flags mask: xmlParseInNodeContext(..., options = 0, ...);
This is potentially dangerous.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
gpg-agent can be used instead of ssh-agent to authenticate
against an SSH server, but in order to do so the GPG_TTY and
TERM environment variables need to be passed through.
For obvious reasons, we avoid doing that when no_tty=1 is found
in the connection URI.
https://bugs.debian.org/843863https://gitlab.com/libvirt/libvirt/-/merge_requests/290
Thanks: Guilhem Moulin <guilhem@guilhem.org>
Thanks: Kunwu Chan <chentao@kylinos.cn>
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
When reverting to inactive snapshot updating the domain definition needs
to happen after the new overlays are created otherwise qemu-img will
correctly fail with error:
Trying to create an image with the same filename as the backing file
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
When we revert to non-leaf snapshot and create new branch or branches
the overlay in snapshot metadata is no longer usable as a disk source
for deletion of that snapshot. We need to use other places to figure out
the correct storage source.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/534
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
CCW addresses need to be also checked for ABI stability.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
We recently unified all services and sockets, except a couple
were missed. Finish the job.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Problem with HW_PHYSMEM sysctl on 64-bit macOS is that it
returns a 32-bit signed value. Thus it overflows. Switching to
HW_MEMSIZE is recommended as it's of an uint_64 type [1].
1: https://github.com/apple-oss-distributions/xnu/blob/xnu-10002.1.13/bsd/sys/sysctl.h
Reported-by: Jaroslav Suchanek <jsuchane@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Xcode 15, which provides the compiler toolchain for building libvirt
on macOS has switched to a new linker that warns about duplicated
"-lblah" options on the ld commandline. In practice this is impossible
to prevent in a large project, and also harmless.
Fortunately the new ld command also has an option,
-no_warn_duplicate_libraries, that supresses this harmless/pointless
warning, meson has a simple way to check if that option is supported,
and libvirt's meson.build files already have examples of adding an
option to the ld commandline if it's available.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Currently some, but not all, methods have a call to the
xdr_free function, for the 'ret' variable. This is done
on methods where there are complex structs containing
allocated memory. In other cases the structs contain
allocated memory, but the pointer is stolen, so xdr_free
is not called. In other cases no allocated memory is
present, so xdr_free.
This is hard to reason about, because the definition of
the struct is not visible in the client stubs.
Switch to use g_auto() for the 'ret' variable, which
means 'xdr_free' is always going to be called. Some
places now need to use g_steal_pointer as a result.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently some, but not all, methods have a call to the
xdr_free function, for the 'ret' variable. This is done
on methods where there are complex structs containing
allocated memory. In other cases the structs contain
allocated memory, but the pointer is stolen, so xdr_free
is not called. In other cases no allocated memory is
present, so xdr_free.
This is hard to reason about, because the definition of
the struct is not visible in the client stubs.
Switch to use g_auto() for the 'ret' variable, which
means 'xdr_free' is always going to be called. Some
places now need to use g_steal_pointer as a result.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Currently some, but not all, methods have a call to the
xdr_free function, for the 'ret' variable. This is done
on methods where there are complex structs containing
allocated memory. In other cases the structs contain
allocated memory, but the pointer is stolen, so xdr_free
is not called. In other cases no allocated memory is
present, so xdr_free.
This is hard to reason about, because the definition of
the struct is not visible in the client stubs.
Switch to use g_auto() for the 'ret' variable, which
means 'xdr_free' is always going to be called. Some
places now need to use g_steal_pointer as a result.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This replaces use of 'rpcgen' with our new python impl of
the RPC code generator. Since the new impl generates code
that matches our style/coding rules, and does not contain
long standing bugs, we no longer need to post-process the
output.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The current RPC code is post-processed to introduce an
intermediate variable, rather than casting directly
to char ** at time of use. This is said to be a workaround
for type-puning warnings that the compiler emitted.
Neither GCC or CLang emit any warnings for the code in
question today, across any of the architectures we
test in CI. Thus it is presumed that somewhere in the
15 years since the workaround was done, the compilers
have got smarter and do the right thing.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Commit changing the code to allow passing NULL as @data into
qemuSaveImageDecompressionStart() was not correct as it left the
original call into the function as well.
Introduced-by: 2f3e582a1ac1008eba8d43c751cdba8712dd1614
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2247754
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
This is the default for the version of rpcgen shipped with
Linux distributions, but the one in macOS and possibly others
default to K&R C, which modern compilers don't appreciate.
Luckily, all versions of rpcgen shipped with our target
platforms seem to support the -C option.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
QEMU_BLOCK_STORAGE_SOURCE_BACKEND_PROPS_SKIP_UNMAP is no longer
referenced inside the code.
QEMU_BLOCK_STORAGE_SOURCE_BACKEND_PROPS_AUTO_READONLY is passed from
various code paths to the qemuBlockStorageSourceGetBackendProps helper,
but it's no longer used.
Both thus can be removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
The code was refactored to format the 'read-only' and 'auto-read-only'
flags via the common helper, so the logic determining their values can
be removed.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
