Peter Krempa [Fri, 29 Mar 2019 12:48:22 +0000 (13:48 +0100)]
util: json: Don't bother logging output string in virJSONValueToString
We have tests that validate the XML formatter. Additionally almost every
guide tells users to disable JSON logging. Drop logging of output string
in virJSONValueToString.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Laine Stump <laine@laine.org>
Peter Krempa [Tue, 26 Mar 2019 13:56:22 +0000 (14:56 +0100)]
util: json: Use virBuffer in JSON->string conversion
The last step of the conversion involves copying of the generated JSON
into a separate string. We can use a virBuffer to do this as this will
also allow to subsequently use the buffer when we actually need to do
some other formatting of the string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Laine Stump <laine@laine.org>
Peter Krempa [Mon, 25 Mar 2019 15:54:28 +0000 (16:54 +0100)]
util: buffer: Remove struct member munging
This was meant to stop abusing the members directly, but we don't do
this for other internal structs. Additionally this did not stop the
test from touching the members. Remove the header obscurization.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Laine Stump <laine@laine.org> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 13 Mar 2019 16:42:37 +0000 (17:42 +0100)]
conf: Add 'index' attribute for <disk><mirror><source>
Similarly to the disk source we need to keep the disk index (which is in
the qemu driver used for identification of the source for block jobs)
for the <mirror> element so that when it's replaced as a disk source
after pivoting all the allocated data is present.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Wed, 13 Mar 2019 15:08:13 +0000 (16:08 +0100)]
conf: Parse and format 'backingStore' for disk <mirror>
When the block copy operation is started with a reused external file in
incremental mode libvirt will need to open and insert the backing chain
for that file into qemu (in -blockdev mode). This means that we'll need
to track the backing chain and metadata such as node names for the full
chain of <mirror>.
This patch invokes the full backing chain formatter and parser for
<mirror> so that the chain can be kept with <mirror>.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Mon, 11 Mar 2019 09:47:53 +0000 (10:47 +0100)]
conf: Replace virDomainDiskSourceParse by virDomainStorageSourceParse
virDomainDiskSourceParse was now just a thin wrapper without any extra
value. Replace all usage of it by the function it calls and remove the
function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 22 Mar 2019 15:48:54 +0000 (16:48 +0100)]
qemu: domain: Modify <migrationSource> to look like <disk>
When adding <migrationSource> I've used a slightly unusual approach. To
allow using the disk source XML parser and formatter convert
<migrationSource> to look like <disk>. This means that <source> will be
added as a subelement of <migrationSource> rather than being formatted
inline.
Conversion from the old format in the parser is very simple as it
involves only moving the XPath context current node slightly if the new
format is found.
The status XML to XML test shows that the upgrade is done correctly.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 22 Mar 2019 15:45:06 +0000 (16:45 +0100)]
tests: qemustatusxml2xml: Add separate output for migration-out-nbd-tls
Upcomming change will modify some aspects. To allow testing upgrade path
add a separate output file so that we can see the conversion from old to
new config.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Thu, 28 Mar 2019 16:06:20 +0000 (17:06 +0100)]
qemu: domain: Forbid copy_on_read option also for floppies
Using copy_on_read for removable disks is a hassle. It also does not
work for CDROMs at all as the image is supposed to be read-only and we
might ignore it for floppies when they are started as empty. Forbid it
for floppies completely rather than trying to support what probably
nobody is using.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Peter Krempa [Fri, 29 Mar 2019 08:05:45 +0000 (09:05 +0100)]
qemu: caps: Separate capabilities based on qemu version
virQEMUCapsInitQMPMonitor is massive now since it collects calls to the
various probing functions and also version based capabilities. Split
out the version based caps into a separate function.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> Reviewed-by: Erik Skultety <eskultet@redhat.com>
Ján Tomko [Fri, 29 Mar 2019 13:36:57 +0000 (14:36 +0100)]
virsh-completer: use VIR_AUTOSTRINGLIST for tmp
We've been open-coding virStringListFreeCount for cleaning up
the completion list we're building. This had the advantage of
zeoring the pointer afterwards, which is no longer needed
now that we compile the list in 'tmp' instead of 'ret'.
Since all our lists are NULL-terminated anyway, switch to using
virStringListFree via the VIR_AUTOSTRINGLIST macro.
Fixes nearly impossible NULL dereferences in
virshNWFilterBindingNameCompleter
virshNWFilterNameCompleter
virshNodeDeviceNameCompleter
virshNetworkNameCompleter
virshInterfaceNameCompleter
virshStoragePoolNameCompleter
virshDomainNameCompleter
which jumped on the error label after a failed allocation
and a possible one in
virshStorageVolNameCompleter
which jumped there when we fail to fetch the list of volumes.
Ján Tomko [Thu, 28 Mar 2019 17:26:44 +0000 (18:26 +0100)]
virsh-completer: add a cleanup label everywhere
Unify the cleanup paths for error and success.
Now that 'ret' is only set (from tmp) on the success path,
it is safe to jump right before 'return ret' after processing
the error block.
Andrea Bolognani [Tue, 12 Mar 2019 16:21:37 +0000 (17:21 +0100)]
tests: Don't use canonical paths in virstoragetest
The layout of my home directory is somewhat peculiar: I store
all git repositories in ~/src/upstream, but since I spend
almost all of my time hacking on libvirt, I also have a
convenience symlink ~/src/libvirt -> ~/src/upstream/libvirt
that I use to access that specific git repository.
The above setup has served me well for years; however, ever
since commit ca1471622dd9 dropped our own custom definitions
for abs_{,top_}{src,build}dir and started using the ones
provided by autotools, virstoragetest has started reliably
failing with errors such as
Using abolute paths instead of canonical ones in the tests makes
the problem go away.
Note that all tests that are specifically designed to test path
canonicalization via TEST_PATH_CANONICALIZE() were passing even
before this patch and are not touched by it.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Acked-by: Michal Privoznik <mprivozn@redhat.com>
This file contains the old school ChangeLog, which was manually
updated for every set of changes before the switch to git.
When libvirt was imported into git, however, *all* history was
preserved, including the changes documented in this file, and
can still be inspected using 'git log' just like more recent
changes: the format might be slightly different, but that's not
quite reason enough to treat this file any differently than the
git-generated ChangeLog we just dropped.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Our ChangeLog is generated by basically redirecting the output
of 'git log' into it so, as can be expected, it has only gotten
bigger as development has progressed. As of today, its size has
reached pretty much comical levels:
$ du -sk ChangeLog
11328 ChangeLog
All of that for information *literally nobody* cares about: end
users and distro maintainers have proper release notes lovingly
compiled for them, while developers peruse the history either by
calling 'git log' directly or through their favorite $EDITOR's
git integration.
Replacing the generated ChangeLog with a short message pointing
interested parties to the git repository does not only reduce
the size of the unpacked sources from 259904 KiB to 248576 KiB
(~4% saving): from a quick test on my laptop, doing so reduces
the size of the *compressed* release archive from 15140 KiB to
12364 KiB (~18% saving) and also takes the time needed to run
'make distcheck' down from 4:44 to 4:21 (~8% saving).
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
The VM Manager app is no longer present on the Play store and while
Google shows a couple of hits they look like the typical untrustworthy
3rd party download redistributors rather than an official site.
Reviewed-by: Andrea Bolognani <abologna@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This reverts commit 047cfb05ee949325e77726531fd381820be8dc62
Using numeric comparison on strings means we reject every update
that does include the group name, even if it's unchanged.
Signed-off-by: Ján Tomko <jtomko@redhat.com> Reviewed-by: Laine Stump <laine@laine.org>
Eric Blake [Thu, 28 Mar 2019 02:15:43 +0000 (21:15 -0500)]
virsh: Don't infloop on snapshot/storage_vol failure
Most of our completers used the pattern:
if ((nITEM = virITEMListAll()) < 0)
return NULL;
but the virDomainSnapshot and virStorageVolume completers were instead
using goto error. If the ListAll fails with -1, the cleanup label was
running a loop of 'size_t i < int nITEM', which is an extreme waste of
CPU cycles. Broken since their introduction in v4.1.
Fixes: f81f8b62 Fixes: 4cb4b649 Reported-by: Ján Tomko <jtomko@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Eric Blake [Thu, 28 Mar 2019 14:00:59 +0000 (09:00 -0500)]
snapshot: Improve logic of virDomainMomentMoveChildren
Even though Coverity can prove that 'last' is always set if the prior
loop executed, gcc 8.0.1 cannot:
CC conf/libvirt_conf_la-virdomainmomentobjlist.lo
../../src/conf/virdomainmomentobjlist.c: In function 'virDomainMomentMoveChildren':
../../src/conf/virdomainmomentobjlist.c:178:19: error: 'last' may be used uninitialized in this function [-Werror=maybe-uninitialized]
last->sibling = to->first_child;
~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
Rewrite the loop to a form that should be easier for static analysis
to work with.
Fixes: ced0898f86bf Reported-by: Bjoern Walk <bwalk@linux.ibm.com> Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com>
Laine Stump [Wed, 27 Mar 2019 18:58:45 +0000 (14:58 -0400)]
util: suppress unimportant ovs-vsctl errors when getting interface stats
commit edaf13565 modified the stats retrieval for OVS interfaces to
not fail when one of the fields was unrecognized by the ovs-vsctl
command, but ovs-vsctl was still returning an error, and libvirt was
cluttering the logs with these inconsequential error messages.
This patch modifies the GET_STAT macro to add "--if-exists" to the
ovs-vsctl command, which causes it to return an empty string (and exit
with success) if the requested statistic isn't in its database, thus
eliminating the ugly error messages from the log.
Peter Krempa [Thu, 28 Mar 2019 12:12:32 +0000 (13:12 +0100)]
qemu: Always use 'alias' in warning message when removing USB address
Avoid the extra parameter passing in the disk 'dst' parameter to be
reported instead of the device alias. Using 'dst' instead of alias does
not add much value.
Michal Privoznik [Wed, 27 Mar 2019 16:30:44 +0000 (17:30 +0100)]
qemuxml2argvtest: Drop dependency between testInfoArgName and virQEMUCapsFlags enums
Introduced in fdf6c89ee7b, this dependency looks weird. It was
needed because of the way that while() loop was written - it
fetches next argument in every iteration. Therefore, our only
option was for ARG_END to have the same value as QEMU_CAPS_LAST.
This also meant that QEMU_CAPS_* could have been only at the end
of the __VA_ARGS__.
This commit reworks the while() loop and removes the dependency.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Michal Privoznik [Wed, 27 Mar 2019 16:19:37 +0000 (17:19 +0100)]
qemu_capabilities; Drop virQEMUCapsSetVAList
There is one specific caller (testInfoSetArgs() in
qemuxml2argvtest.c) which expect the va_list argument to change
after returning from the virQEMUCapsSetVAList() function.
However, since we are passing plain va_list this is not
guaranteed. The man page of stdarg(3) says:
If ap is passed to a function that uses va_arg(ap,type), then
the value of ap is undefined after the return of that function.
(ap is a variable of type va_list)
I've seen this in action in fact: on i686 the qemuxml2argvtest
fails on the second test case because testInfoSetArgs() sees
ARG_QEMU_CAPS and calls virQEMUCapsSetVAList to process the
capabilities (in this case there's just one
QEMU_CAPS_SECCOMP_BLACKLIST). But since the changes are not
reflected in the caller, in the next iteration testInfoSetArgs()
sees the QEMU capability and not ARG_END.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
The mock fopen() function will abort if "/proc/mounts" is
requested with "r" permissions and VIR_CGROUP_MOCK_FILENAME
env var is not set.
Unfortunately this is triggering by the libselinux library
constructor when it tries to read /proc/mounts to find out
if selinuxfs is mounted in an unusual place.
This, however, only affects libselinux in Debian as that
opens with "r", while in Fedora / RHEL it opens "re" and
thus luckily never triggered the abort(), instead getting
an EACCESS.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
It turns out gcc -O2 is not happy with it, complaining:
/home/pipo/libvirt/src/qemu/qemu_driver.c: In function 'qemuDomainSnapshotCreateXML':
/home/pipo/libvirt/src/qemu/qemu_driver.c:15389:26: error: potential null pointer dereference [-Werror=null-dereference]
bool memory = snapdef->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
~~~~~~~^~~~~~~~
/home/pipo/libvirt/src/qemu/qemu_driver.c:15389:26: error: potential null pointer dereference [-Werror=null-dereference]
In file included from /home/pipo/libvirt/src/util/virbuffer.h:27,
from /home/pipo/libvirt/src/conf/capabilities.h:27,
from /home/pipo/libvirt/src/conf/domain_conf.h:32,
from /home/pipo/libvirt/src/qemu/qemu_agent.h:26,
from /home/pipo/libvirt/src/qemu/qemu_driver.c:40:
/home/pipo/libvirt/src/util/viralloc.h:125:34: error: potential null pointer dereference [-Werror=null-dereference]
# define VIR_ALLOC_N(ptr, count) virAllocN(&(ptr), sizeof(*(ptr)), (count), true, \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
VIR_FROM_THIS, __FILE__, __FUNCTION__, __LINE__)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/pipo/libvirt/src/qemu/qemu_driver.c:15103:9: note: in expansion of macro 'VIR_ALLOC_N'
if (VIR_ALLOC_N(ret, snapdef->ndisks) < 0)
^~~~~~~~~~~
/home/pipo/libvirt/src/qemu/qemu_driver.c:15798:45: error: null pointer dereference [-Werror=null-dereference]
virDomainSnapshotObjGetDef(snap)->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~
As the patch simplified one or two callers at the risk of making
many other callers now candidates to trigger aggressive compiler
warnings, it isn't worth it.
Eric Blake [Wed, 27 Mar 2019 07:12:37 +0000 (02:12 -0500)]
snapshot: Refactor qemu to utilize virDomainMoment more
Use the common base class virDomainMoment for iterator callbacks
related to snapshots from the qemu code, so that when checkpoint
operations are introduced, they can share the same callbacks.
Simplify the code for qemuDomainSnapshotCurrent by better utilizing
virDomainMoment helpers.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Eric Blake [Wed, 27 Mar 2019 08:17:46 +0000 (03:17 -0500)]
snapshot: Drop pointless function virDomainMomentIsCurrentName
The qemu driver already had a full-blown virDomainMomentObjPtr to
check against, and the test driver ought to have one since we get
better error checking that the user passed in a valid object. Removes
the need for a helper function added in commit commit 4819f54b.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Andrea Bolognani [Mon, 25 Mar 2019 09:51:05 +0000 (10:51 +0100)]
spec: Move ldconfig calls from -client to -libs
ldconfig needs to be called after installing or uninstalling
shared libraries.
For a very long time, libvirt didn't have a separate package
containing just the shared libraries, and so it shipped them
in the same one as the clients.
Since commit 70b4f0e719cd, however, shared libraries have been
moved from -client to their own -libs package; unfortunately,
the corresponding ldconfig calls were not moved at the same
time, which is what this commit takes care of.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
projects / thirdparty / libvirt.git / log
