Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Remove the wrapper and fix callers.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

All callers including transitive callers through
virDomainDiskSourceFormatInternal always pass true. Remove the argument.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

We parse the seclabels and use them internally so omitting them when
formatting would be misleading. Additionally our schema actually allows
them.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Until the block job completes we can't change the disk chain. Removal
would fail as the block job still has reference to the chain.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Unref the config pointer automatically in code paths which get a local
copy.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

qemuDomainChangeGraphicsPasswords and qemuDomainRemoveHostDevice
don't use 'cfg' any more since commits 4327df7e and 802c59d4
respectively.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

There's nothing to clean up.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>

It's actually used.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>

Failure of qemuMonitorGetVersion is fatal now that we only support QMP
based qemus. Remove the debug message since we report an error already.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

If the detected qemu version is below our required version 'package'
would be leaked.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Move the check out of virQEMUCapsInitQMPMonitor similarly to other
functions.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Move the code out of virQEMUCapsInitQMPMonitor similarly to other
functions.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Move the check out of virQEMUCapsInitQMPMonitor similarly to other
functions.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Some caps are cleared according to some more advanced logic after
detection. Split all that logic out into virQEMUCapsInitProcessCaps.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

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: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

Convert the other string comparisons to use the macro.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NLaine Stump <laine@laine.org>

Add hppa, nios2, or1k, riscv32 and riscv64 to the profile.

Fixes: https://bugs.debian.org/914940Signed-off-by: Nintrigeri <intrigeri@boum.org>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

Check that the attribute is the same in qemuDomainDiskChangeSupported
in case somebody tries to change it using the UpdateDevice API.

https://bugzilla.redhat.com/show_bug.cgi?id=1601677Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NLaine Stump <laine@laine.org>

A macro for comparing string fields of the disk.

https://bugzilla.redhat.com/show_bug.cgi?id=1601677Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NLaine Stump <laine@laine.org>

https://bugzilla.redhat.com/show_bug.cgi?id=1601677

This reverts commit 047cfb05
Using numeric comparison on strings means we reject every update
that does include the group name, even if it's unchanged.
Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NLaine Stump <laine@laine.org>

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: ced0898fReported-by: NBjoern Walk <bwalk@linux.ibm.com>
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

commit edaf1356 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.

Resolves: https://bugzilla.redhat.com/1683175Signed-off-by: NLaine Stump <laine@laine.org>

The warning is reported at a code path which already reports a proper
error so it's pointless to add yet another line into logs.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>

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.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>

qemuDomainRemoveDiskDevice calls qemuDomainReleaseDeviceAddress which
already calls virDomainUSBAddressRelease so we don't need to call it
again.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>

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: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

This reverts commit 6b90a847.

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.
Signed-off-by: NEric Blake <eblake@redhat.com>

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: NEric Blake <eblake@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Doing so can simplify some callers.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

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: NEric Blake <eblake@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

The VIR_MIGRATE_PARALLEL flag is implemented using QEMU's multifd
migration capability and the corresponding multifd-channels migration
parameter.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>

Prepare for introducing a bunch of new public APIs related to
backup checkpoints by first introducing a new internal type
and errors associated with that type.  Checkpoints are modeled
heavily after virDomainSnapshotPtr (both represent a point in
time of the guest), although a snapshot exists with the intent
of rolling back to that state, while a checkpoint exists to
make it possible to create an incremental backup at a later
time.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJohn Ferlan <jferlan@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Since I was copying this text to form checkpoint XML and API
documentation, I might as well make improvements along the way. Most
of these changes are based on reviews of the checkpoint docs.

Among other things: grammar tweaks, point to a single source of
documentation rather than repeating verbosity, reword things for
easier legibility.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

This reverts commit 86c0ed6f, and
subsequent refactorings of the function into new files.  There are no
callers of this function - I had originally proposed it for
implementing a new bulk snapshot API, but that proved to be too
invasive given RPC limits. I also tried using it for streamlining how
the qemu driver stores snapshot state across libvirtd restarts
internally, but in the end, the risks of a new internal format
outweighed the benefits of one file per snapshot.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

This reverts commit 1b57269c, and
subsequent refactorings of the function into new files.  There are no
callers of this function - I had originally proposed it for
implementing a new bulk snapshot API, but that proved to be too
invasive given RPC limits. I also tried using it for streamlining how
the qemu driver stores snapshot state across libvirtd restarts
internally, but in the end, the risks of a new internal format
outweighed the benefits of one file per snapshot.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

For [some unknown reason, possibly/probably pure chance], Net devices
have been taken offline and their bandwidth tc rules cleared as the
very first operation when detaching the device. This is contrary to
every other type of device, where all hostside teardown is delayed
until we receive the DEVICE_DELETED event back from qemu, indicating
that the guest has finished with the device.

This patch delays these two operations until receipt of
DEVICE_DELETED, which removes an ugly wart from
qemuDomainDetachDeviceLive(), and also seems to be a more correct
sequence of events.
Signed-off-by: NLaine Stump <laine@laine.org>
ACKed-by: NPeter Krempa <pkrempa@redhat.com>