There are only 3 places using the function. Two can use virBitmapNewCopy
directly. In case of the qemu capabilities code we need to free the old
bitmap first.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virBitmapCopy has a failure condition, which is impossible to meet when
creating a new copy. Copy the contents directly to make it obvious that
virBitmapNewCopy can't fail.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We no longer report any errors so all callers can be replaced by
virBitmapNew. Additionally virBitmapNew can't return NULL now so error
handling is not necessary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We now always return a valid pointer or crash so the return value
doesn't need to be checked.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Modify the condition which would make virBitmapNewQuiet fail to possibly
overallocate by 1 rather than failing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We now have APIs which automatically expand the bitmap and also API
which allocates a 0 size bitmap. Remove the condition from virBitmapNew.
Effectively reverts ce49cfb48a
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
virBitmapNewEmpty() can create a bitmap with 0 length. With such a
bitmap virBitmapToString will return NULL rather than an empty string.
Initialize the buffer to avoid that.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Clarify which bit is considered most significant in the bitmap and
resulting string. Also be explicit that it's a hex string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
There's only one combination used so we can remove the rest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
When VIR_EXEC_DAEMON is true and cmd->pidfile exists, the parent
will expect the pidfile to be written before exiting, sitting
tight in a saferead() call waiting.
The child then does process tuning (via virProcessSet* functions)
before writing the pidfile. Problem is that these tunings can
fail, and trigger a 'fork_error' jump, before cmd->pidfile is
written. The result is that the process was aborted in the
child, but the parent is still hang in the saferead() call.
This behavior can be reproduced by trying to create and execute
a QEMU guest in user mode (e.g. using qemu:///session as non-root).
virProcessSetMaxMemLock() will fail if the spawned libvirtd user
process does not have CAP_SYS_RESOURCE capability. setrlimit() will
fail, and a 'fork_error' jump is triggered before cmd->pidfile
is written. The parent will hung in saferead() indefinitely. From
the user perspective, 'virsh start <guest>' will hang up
indefinitely. CTRL+C can be used to retrieve the terminal, but
any subsequent 'virsh' call will also hang because the previous
libvirtd user process is still there.
We can fix this by moving all virProcessSet*() tuning functions
to be executed after cmd->pidfile is taken care of. In the case
mentioned above, this would be the result of 'virsh start'
after this patch:
error: Failed to start domain vm1
error: internal error: Process exited prior to exec: libvirt: error :
cannot limit locked memory to 79691776: Operation not permitted
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1882093
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Lack of this one function (which is called for each active tap device
every time libvirtd is started) is the one thing preventing a
"WITHOUT_LIBNL" build of libvirt from being useful. With this
alternate implementation, guests using standard tap devices will work
properly even when libvirt is built without libnl support.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
There was one stray bit of code in virnetdev.c that required libnl to
build, but wasn't qualified by defined(WITH_LIBNL). Adding that, plus
putting a similar check around a static function only used by that
aforementioned code, makes libvirt build properly without libnl3-devel
installed.
How useful it is in that state is a separate issue :-)
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This flag was originally created to indicate that either 1) the build
platform wasn't linux, 2) the build platform was linux, but the kernel
was too old to have macvtap support. Since there was already a switch
there, the ability to also disable it when 3) the kernel supports
macvtap but the user doesn't want it, was added in. I don't think that
(3) was ever an intentional goal, just something that grew naturally
out of having the flag there in the first place (unless possibly the
original author wanted a way to quickly disable their new code in case
it caused regressions elsewhere).
Now that the check for (2) has been removed, WITH_MACVTAP is just
checking (1) and (3), but (3) is pointless (because the extra code in
libvirt itself is miniscule, and the only external library needed for
it is libnl, which is also required for other unrelated features (and
itself has no subordinate dependencies and takes up < 1MB on
disk)). We can therfore eliminate the WITH_MACVTAP flag, as it is
functionally equivalent to WITH_LIBNL (which implies __linux__).
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
macvlan support was added to the Linux kernel in 2.6.33, but
MACVLAN_MODE_PASSTHRU wasn't added until 2.6.38, so a workaround had
been put in place to define that constant on those few systems where
it was missing. It's useful like was probably 6 months at most, but
it's been there for over 10 years.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
WITH_VIRTUALPORT just checks that we are building on Linux and that
IFLA_PORT_MAX is defined in linux/if_link.h. Back when 802.11Qb[gh]
support was added, the IFLA_* stuff was new (introduced in kernel
2.6.35, backported to RHEL6 2.6.32 kernel at some point), and so this
extra check was necessary, because libvirt was being built on Linux
distros that didn't yet have IFLA_* (e.g. older RHEL6, all
RHEL5). It's been in the kernel for a *very* long time now, so all
supported versions of all Linux platforms libvirt builds on have it.
Note that the above paragraph implies that the conditional compilation
should be changed to #if defined(__linux__). However, the astute
reader will notice that the code in question is sending and receiving
netlink messages, so it really should be conditional on WITH_LIBNL
(which implies __linux__) instead, so that's what this patch does.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
WITH_LIBNL will only be defined on Linux platforms (because libnl is a
library written to encapsulate parts of netlink, which is a Linux-only
API), so it's redundant to write:
#if defined(__linux__) && defined(WITH_LIBNL)
We can just check for WITH_LIBNL.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
IFLA_VF_MAX was introduced to the Linux kernel in 2.6.35, and was even
backported to the RHEL*6* 2.6.32 kernel downstream, so it is present
in all supported versions of all Linux distros that libvirt builds
on. Additionally, it can't be conditionally compiled out of a
kernel. There is no reason to conditionalize any piece of code on
presence of IFLA_VF_MAX - if the platform is Linux, it is supported.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The former has been present since
commit f43798c27684ab925adde7d8acc34c78c6e50df8
Author: Rusty Russell <rusty@rustcorp.com.au>
Date: Thu Jul 3 03:48:02 2008 -0700
tun: Allow GSO using virtio_net_hdr
and the latter since
commit bbb009941efaece3898910a862f6d23aa55d6ba8
Author: Jason Wang <jasowang@redhat.com>
Date: Wed Oct 31 19:45:59 2012 +0000
tuntap: introduce multiqueue flags
these are old enough that they can be assumed present in all Linux
platforms we support. The tap device creation code changed is specific
to Linux, with a separate impl for non-Linux platforms.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This flag was added by Linux with:
commit f43798c27684ab925adde7d8acc34c78c6e50df8
Author: Rusty Russell <rusty@rustcorp.com.au>
Date: Thu Jul 3 03:48:02 2008 -0700
tun: Allow GSO using virtio_net_hdr
so we can assume all Linux distros we support have this flag available
and thus the compile time check is sufficient.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Add an abort() on the class/object allocation failures so that
virStorageSourceNew() always returns a virStorageSource and remove
checks from all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
g_thread_join() eats a reference.
==295055== Invalid read of size 4
==295055== at 0x4DA4AE4: g_thread_unref (in /usr/lib64/libglib-2.0.so.0.6400.5)
==295055== by 0x491D5FA: vir_event_thread_finalize (vireventthread.c:47)
==295055== by 0x4E6BCFF: g_object_unref (in /usr/lib64/libgobject-2.0.so.0.6400.5)
==295055== by 0x22F35CF4: qemuProcessQMPFree (qemu_process.c:8525)
==295055== by 0x22E71B58: glib_autoptr_clear_qemuProcessQMP (qemu_process.h:237)
...
==295055== by 0x22E98A29: qemuDomainPostParseDataAlloc (qemu_domain.c:5476)
==295055== by 0x49ABF83: virDomainDefPostParse (domain_conf.c:6023)
==295055== Address 0x2acb1c68 is 24 bytes inside a block of size 88 free'd
==295055== at 0x483B9F5: free (vg_replace_malloc.c:538)
==295055== by 0x4D80A4C: g_free (in /usr/lib64/libglib-2.0.so.0.6400.5)
...
==295055== by 0x491D5F1: vir_event_thread_finalize (vireventthread.c:46)
==295055== by 0x4E6BCFF: g_object_unref (in /usr/lib64/libgobject-2.0.so.0.6400.5)
==295055== by 0x22F35CF4: qemuProcessQMPFree (qemu_process.c:8525)
==295055== by 0x22E71B58: glib_autoptr_clear_qemuProcessQMP (qemu_process.h:237)
...
==295055== Block was alloc'd at
==295055== at 0x483A809: malloc (vg_replace_malloc.c:307)
==295055== by 0x4D80958: g_malloc (in /usr/lib64/libglib-2.0.so.0.6400.5)
...
==295055== by 0x4DA4C32: g_thread_try_new (in /usr/lib64/libglib-2.0.so.0.6400.5)
==295055== by 0x491D3BC: virEventThreadStart (vireventthread.c:159)
==295055== by 0x491D3BC: virEventThreadNew (vireventthread.c:185)
...
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Fixes: f4fc3db920
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
g_variant_iter_loop() handles freeing all arguments unless we break out
of the loop, in that case we have to free them manually.
Reported-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
We used to check the format of reply data with libdbus so we should do
the same with GLib DBus as well.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
We need to pass pointer to `array`.
Reported-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Tested-by: Ján Tomko <jtomko@redhat.com>
virFileComparePaths just return 0 or 1 after commit 7b48bb8
so break while after virFileComparePaths return 1
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Yi Li <yili@winhong.com>
An extra parameter was added to virQEMUBuildQemuImgKeySecretOpts in
commit ecfc4094d8
Author: Daniel P. Berrangé <berrange@redhat.com>
Date: Tue Sep 15 16:30:37 2020 +0100
storage: add support for qcow2 LUKS encryption
but the non-null pointer annotations were not adjusted to take account.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
The storage driver was wired up to support creating raw volumes in LUKS
format, but was never adapted to support LUKS-in-qcow2. This is trivial
as it merely requires the encryption properties to be prefixed with
the "encrypt." prefix, and "encrypt.format=luks" when creating the
volume.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Crypt method number 2 indicates LUKS format.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
With libdbus our wrappers had a special syntax to create the DBus
messages by defining the DBus message signature followed by list
of arguments providing data based on the signature.
There will be no similar helper with GLib implementation as they
provide same functionality via GVariant APIs. The syntax is slightly
different mostly for how arrays, variadic types and dictionaries are
created/parsed.
Additional difference is that with GLib DBus everything is wrapped in
extra tuple (struct). For more details refer to the documentation [1].
[1] <https://developer.gnome.org/glib/stable/gvariant-format-strings.html>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
The original motivation for adding virNetDevIPCheckIPv6Forwarding
(commit 00d28a78b5) was that networking routes would disappear when
ipv6 forwarding was enabled for an interface.
This is a fairly undocumented side-effect of the "accept_ra" sysctl
for an interface. 1 means the interface will accept_ra's if not
forwarding, 2 means always accept_RAs; but it is not explained that
enabling forwarding when accept_ra==1 will also clear any kernel RA
assigned routes, very likely breaking your networking.
The check to warn about this currently uses netlink to go through all
the routes and then look at the accept_ra status of the interfaces.
However, it has been noticed that this problem does not affect systems
where IPv6 RA configuration is handled in userspace, e.g. via tools
such as NetworkManager. In this case, the error message from libvirt
is spurious, and modifying the forwarding state will not affect the RA
state or disable your networking.
If you refer to the function rt6_purge_dflt_routers() in the kernel,
we can see that the routes being purged are only those with the
kernel's RTF_ADDRCONF flag set; that is, routes added by the kernel's
RA handling. Why does it do this? I think this is a Linux
implementation decision; it has always been like that and there are
some comments suggesting that it is because a router should be
statically configured, rather than accepting external configurations.
The solution implemented here is to convert the existing check into a
walk of /proc/net/ipv6_route (because RTF_ADDRCONF is apparently not
exposed in netlink) and look for routes with this flag set. We then
check the accept_ra status for the interface, and if enabling
forwarding would break things raise an error.
This should hopefully avoid "interactive" users, who are likely to be
using NetworkManager and the like, having false warnings when enabling
IPv6, but retain the error check for users relying on kernel-based
IPv6 interface auto-configuration.
Signed-off-by: Ian Wienand <iwienand@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Reviewed-by: Cedric Bosdonnat <CBosdonnat@suse.com>
