Peter Krempa [Thu, 30 Jul 2020 15:41:43 +0000 (17:41 +0200)]
tests: qemuxml2argv: Use only modern versions of 'disk-network-tlsx509' test
We already test with real caps so there's no real need for this special
case. While it technically tested the state without TLS encryption key
secrets, it doesn't really matter that much.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Use g_autoptr to free the temporary virDomainDef object created by
qemuDomainSaveInternal() when xmlin is non-NULL. Leak was added in
commit 0ea479f8f6, first appearing in libvirt 0.9.4 in August 2011.
Signed-off-by: Zheng Chuan <zhengchuan@huawei.com> Reviewed-by: Laine Stump <laine@redhat.com>
docs: fix name of file containing max number of VFs
Signed-off-by: Paulo de Rezende Pinatti <ppinatti@linux.ibm.com> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com> Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
lib: clarify docs for hugetlb in virDomainMemoryStatTags
The term number is used for other stats and even for hugetlb
stats in virsh man page. The term number is also more clear.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
docs: virConnectGetCapabilities do not provide pool types
Remove the paragraph in the storage pool page that mentions
virConnectGetCapabilities, as virConnectGetCapabilities does not return
any information about pools.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> Reviewed-by: John Ferlan <jferlan@redhat.com>
Peter Krempa [Tue, 21 Jul 2020 10:32:10 +0000 (12:32 +0200)]
tests: commandtest: Make 'test4' checking daemonization more reliable
The 'commandhelper' checks effectively whether the parent process is
still around to report whether it was daemonized or not.
This creates a unlikely race condition in cases when we do actually
daemonize the process as the intermediate process used for the
daemonization might not have terminated yet which would report wrong
result leading to test failure.
For now there's just 'test4' which actually daemonizes the process.
Add an argument '--check-daemonize' which asks for retries of the
daemonization check in cases where we expect that the commandhelper is
going to be daemonized and use it in 'test4' to make the test more
reliable.
I've observed the test failure sporadically when my box is under load
e.g. while building two trees at once.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Michal Privoznik [Thu, 23 Jul 2020 14:02:00 +0000 (16:02 +0200)]
virdevmapper: Don't use libdevmapper to obtain dependencies
CVE-2020-14339
When building domain's private /dev in a namespace, libdevmapper
is consulted for getting full dependency tree of domain's disks.
The reason is that for a multipath devices all dependent devices
must be created in the namespace and allowed in CGroups.
However, this approach is very fragile as building of namespace
happens in the forked off child process, after mass close of FDs
and just before dropping privileges and execing QEMU. And it so
happens that when calling libdevmapper APIs, one of them opens
/dev/mapper/control and saves the FD into a global variable. The
FD is kept open until the lib is unlinked or dm_lib_release() is
called explicitly. We are doing neither.
However, the virDevMapperGetTargets() function is called also
from libvirtd (when setting up CGroups) and thus has to be thread
safe. Unfortunately, libdevmapper APIs are not thread safe (nor
async signal safe) and thus we can't use them. Reimplement what
libdevmapper would do using plain C (ioctl()-s, /proc/devices
parsing, /dev/mapper dirwalking, and so on).
conf: add control over COW for storage pool directories
The storage pool code now attempts to disable COW by default on btrfs,
but management applications may wish to override this behaviour. Thus we
introduce a concept of storage pool features:
<features>
<cow state='yes|no'/>
</features>
If the <cow> feature policy is set, it will be enforced. It will always
return an hard error if COW cannot be explicitly set or unset.
Reviewed-by: Neal Gompa <ngompa13@gmail.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This calls virFileSetCOW when building a pool with a request to attempt,
but not require, COW to be disabled. The effect is that nothing changes
on non-btrfs filesystems, but btrfs will get COW disabled on the
directory. This setting is then inherited by all newly created files in
the pool, avoiding the need for mgmt app to set "nocow" on a per-volume
basis.
Reviewed-by: Neal Gompa <ngompa13@gmail.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
When disabling COW on individual files, we now use the virFileSetCOW
method. Note that this change has a slight semantic difference to the
old implementation.
The original code reported errors but returned success when disabling
COW failed.
With this new code, we will always report an error if the user requested
disabling of COW and we could not honour it, either because btrfs
returned an error, or because the filesystem is not btrfs.
Reviewed-by: Neal Gompa <ngompa13@gmail.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
util: add a helper method for controlling the COW flag on btrfs
btrfs defaults to performing copy-on-write for files. This is often
undesirable for VM images, so we need to be able to control whether this
behaviour is used.
The virFileSetCOW() will allow for this. We use a tristate, since out of
the box, we want the default behaviour attempt to disable cow, but only
on btrfs, silently do nothing on non-btrfs. If someone explicitly asks
to disable/enable cow, then we want to raise a hard error on non-btrfs.
Reviewed-by: Neal Gompa <ngompa13@gmail.com> Reviewed-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This is only used in the ESX driver where, when set to "no", it will
ignore all the checks libvirt does about the origin of the MAC address
(whether or not it's in a VMWare OUI) and forward the original one to
the ESX server telling it not to check it either.
This allows keeping a deterministic MAC address which can be useful for
licensed software which might dislike changes.
Signed-off-by: Bastien Orivel <bastien.orivel@diateam.net>
VMX conversion parts rewritten to apply on top of previously merged
support for type='generated|static'
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
the VMX -> XML parser was not updated. As a result while we
accept the 'type' attribute on input, we never show it again
on 'output', so we loose information during the roundtrip.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
which intentionally added the 'checkMACAddress' side effect based on
the 'type' attribute.
With this change, we're reverting the handling of checkMACAddress
to match what existed historically. The 'type' attribute now directly
maps to the addressType attribute, so the above config becomes:
The mingw header define time() as a static inline function and this
causes a duplicate definition build failure. Since we're not using the
LD_PRELOAD at all on Mingw, we ideally wouldn't compile any of the
mock libraries. Rather than change the build system now though, this
just stubs out the offending function.
Reviewed-by: Laine Stump <laine@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Historically we avoided -fstack-protector* since it resulted in a broken
build on Mingw. In GCC 10 in Fedora though, we have the opposite problem,
getting a broken build if we don't enable one of the -fstack-protector*
options. This also works in GCC 9, so we don't need to worry about the
old brokeness which evidentally got fixed at some time without noticing.
Reviewed-by: Laine Stump <laine@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
gcc 10.1.0 on Debian sid has a bug where the bounds checking gets
confused beteen two branches:
In file included from /usr/include/string.h:495,
from ../../src/internal.h:28,
from ../../src/util/virsocket.h:21,
from ../../src/util/virsocketaddr.h:21,
from ../../src/util/virnetdevip.h:21,
from ../../src/util/virnetdevip.c:21:
In function 'memcpy',
inlined from 'virNetDevGetifaddrsAddress' at ../../src/util/virnetdevip.c:914:13,
inlined from 'virNetDevIPAddrGet' at ../../src/util/virnetdevip.c:962:16:
/usr/include/arm-linux-gnueabihf/bits/string_fortified.h:34:10: error: '__builtin_memcpy' offset [16, 27] from the object at 'addr' is out of the bounds of referenced subobject 'inet4' with type 'struct sockaddr_in' at offset 0 [-Werror=array-bounds]
34 | return __builtin___memcpy_chk (__dest, __src, __len, __bos0 (__dest));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../src/util/virnetdevip.h:21,
from ../../src/util/virnetdevip.c:21:
../../src/util/virnetdevip.c: In function 'virNetDevIPAddrGet':
../../src/util/virsocketaddr.h:29:28: note: subobject 'inet4' declared here
29 | struct sockaddr_in inet4;
| ^~~~~
cc1: all warnings being treated as errors
Note the source location is pointing to the "inet6" / AF_INET6 branch of
the "if", but is complaining about bounds of the "inet4" field. Changing
the code into a switch() is sufficient to avoid triggering the bug and
is arguably better code too.
Reviewed-by: Laine Stump <laine@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Andrea Bolognani [Thu, 23 Jul 2020 08:29:35 +0000 (10:29 +0200)]
ci: Run 'make distcheck' on FreeBSD
The Cirrus CI integration was modeled after the Travis CI jobs,
but those were limited to macOS where the test suite is currently
still broken. FreeBSD can run the full distcheck just fine, so
let's do that.
Fixes: 6190c14151c3e2cf5c30b9df9131697f5c3b64b9 Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
The reason why we align down the guest area (total-size - label-size) is
explained in the body of qemuDomainNVDimmAlignSizePseries(). This
behavior must also be documented in the user docs.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Andrea Bolognani <abologna@redhat.com>
qemu: pre-create the dbus directory in qemuStateInitialize
There are races condiction to make '/run/libvirt/qemu/dbus' directory in
virDirCreateNoFork() while concurrent start VMs, and get "failed to create
directory '/run/libvirt/qemu/dbus': File exists" error message. pre-create the
dbus directory in qemuStateInitialize.
Signed-off-by: Bihong Yu <yubihong@huawei.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com>
Remove the superfuous break, as there is a 'return' before it.
Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com>
Remove superfluous breaks, as there is a "return" before them.
Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com>
Remove superfluous breaks, as there is a "return" before them.
Signed-off-by: Liao Pingfang <liao.pingfang@zte.com.cn> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Ján Tomko <jtomko@redhat.com>
qemu: Properly set //cpu/@migratable default value for running domains
Since active domains which do not have the attribute already set were
not started by libvirt that probed for CPU migratable property, we need
to check this property on reconnect and update the domain definition
accordingly.
qemu: Do not set //cpu/@migratable for running domains in post-parse
Commit v6.4.0-61-g201bd5db63 started to fill the default value for
//cpu/@migratable attribute according to QEMU support. However, active
domains either have the migratable attribute already set or the
capabilities we use for checking the QEMU support were created by older
libvirt which didn't probe for this specific capability. Thus we should
leave active domains alone when parsing their XMLs.
Peter Krempa [Thu, 16 Jul 2020 12:46:43 +0000 (14:46 +0200)]
qemu: block: Remove 'active-write' bitmap even if there are no bitmaps to merge
The 'libvirt-tmp-activewrite' bitmap is added during the 'pivot'
operation of block copy and active layer block commit operations
regardless of whether there are any bitmaps to merge, but was not
removed unless a bitmap was merged. This meant that subsequent attempts
to merge into the same image would fail.
Fix it by checking whether the 'libvirt-tmp-activewrite' would be used
by the code and don't skip the code which would delete it.
This is a regression introduced when we switched to the new code for
block commit in <20a7abc2d2d> and for block copy in <7bfff40fdfe5>. The
actual bug originates from <4fa8654ece>.
Peter Krempa [Thu, 16 Jul 2020 13:54:46 +0000 (15:54 +0200)]
qemu: blockjob: Don't base bitmap handling of active-layer block commit on QEMU_CAPS_BLOCKDEV_REOPEN
The handler finalizing the active layer block commit doesn't actually
reopen the file for active layer block commit, so the comment and check
are invalid.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
The index returned by qemuDomainDiskLookupByNodename is the position in
the backing chain rather than the index we report in the XML.
Since with -blockdev they differ now and additionally the disk source
also has an index we need to fix the 'threshold' events we report:
1) If it's the top level image we must always trigger the event without
any suffix as we did until now
2) We must report the correct index
3) We must report the correct index also for the top level image, when
blockdev is used.
This means that we need to potentially emit 2 events, one for the device
without the index and then when blockdev is used and the top level image
has an index we must do it also with the index.
This will fix it for blockdev cases, while also not removing previous
semantics.
Laine Stump [Thu, 25 Jun 2020 02:20:56 +0000 (22:20 -0400)]
nwfilter: use standard label names when reasonable
Rather than having labels named exit, done, exit_snooprequnlock,
skip_rename, etc, use the standard "cleanup" label. And instead of
err_exit, malformed, tear_down_tmpebchains, use "error".
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Sun, 5 Jul 2020 02:29:23 +0000 (22:29 -0400)]
nwfilter: clear nrules when resetting virNWFilterInst
It's possible/probable the callers to virNWFilterInstReset() make it
unnecessary to set the object's nrules to 0 after freeing all its
rules, but that same function is setting nfilters to 0, so let's do
the same for the sake of consistency.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Wed, 24 Jun 2020 23:31:55 +0000 (19:31 -0400)]
nwfilter: remove unnecessary code from ebtablesGetSubChainInsts()
On failure, this function would clear out and free the list of
subchains it had been called with. This is unnecessary, because the
*only* caller of this function will also clear out and free the list
of subchains if it gets a failure from ebtablesGetSubChainInsts().
(It also makes more logical sense for the function that is creating
the entire list to be the one freeing the entire list, rather than
having a function whose purpose is only to create *one item* on the
list freeing the entire list).
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko redhat com>
Laine Stump [Sat, 4 Jul 2020 03:51:27 +0000 (23:51 -0400)]
network: eliminate unnecessary labels
All these cleanup/error labels were reduced to having just "return
ret" by a previous patch, so get rid of them and return directly.
This patch coincidentally fixes a bug in
networkFindUnusedBridgeName(), where we would log an error yet still
return success if we failed to find a single unused "virbrNNN" name
after checking all values of "N" from 0 - 256. Said bug was introduced
when that function was originally written, in commit a28d3e485f
(libvirt 1.2.15, 2015)
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Wed, 24 Jun 2020 17:04:25 +0000 (13:04 -0400)]
define g_autoptr cleanup function for virNetworkDHCPLease
virNetworkDHCPLease and virNetworkDHCPLeaseFree() are declared in the
public API file libvirt-network.h, and we can't pollute that with glib
macro invocations, so put this in src/datatypes.h next to the other
virNetwork items.
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Thu, 25 Jun 2020 02:37:33 +0000 (22:37 -0400)]
replace g_new() with g_new0() for consistency
g_new() is used in only 3 places. Switching them to g_new0() will do
no harm, reduces confusion, and helps me sleep better at night knowing
that all allocated memory is initialized to 0 :-) (Yes, I *know* that
in all three cases the associated memory is immediately assigned some
other value. Today.)
Signed-off-by: Laine Stump <laine@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Andrea Bolognani [Sat, 18 Jul 2020 21:54:37 +0000 (23:54 +0200)]
spec: Drop explicit dependency on ncurses
We don't actually use ncurses directly: readline needs it, but
that's a readline implementation detail and not something that we
should concern ourselves with.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
qemu_domainjob: introduce `privateData` for `qemuDomainJob`
To remove dependecy of `qemuDomainJob` on job specific
paramters, a `privateData` pointer is introduced.
To handle it, structure of callback functions is
also introduced.
Signed-off-by: Prathamesh Chavan <pc44800@gmail.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Andrea Bolognani [Sat, 18 Jul 2020 22:43:08 +0000 (00:43 +0200)]
spec: Don't require mdevctl on RHEL 7
mdevctl is a relatively new tool that's packaged for Fedora and
RHEL 8, but not for RHEL 7. Make the dependency conditional to
avoid the libvirt-daemon-driver-nodedev package becoming
uninstallable on that platform.
Fixes: 9691440ecbc7d9383a1410f1067a4f9221f2de2c Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
docs: fix compilation instructions to use separate build dir
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Pavel Hrdina <phrdina@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Fri, 17 Jul 2020 14:14:23 +0000 (16:14 +0200)]
virNetSocketCheckProtocols: Actually check bool value
In 9536379da4c8ed61 and 8b0cb0e666f I've tried to call
virNetSocketCheckProtocolByLookup() only if we are suspecting the
host is IPv4 or IPv6 capable (because we've found an interface
with such address). However, the code was missing dereference of
the boolean variables and thus was comparing pointers against
NULL.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>