If there was a failure processing 'authdef' and the code went to cleanup
before the setting to source->auth, then it'd be leaked.

Coverity found that the 'buf' wasn't VIR_FREE'd at exit.

Coverity determines that when jumping to the connected: label, the
addressinfo (ai) is not free'd.

In libxlDomainMigrationPrepare() if the uri_in is false, then
'hostname' is allocated and used "generically" in the routine,
but not freed.  Conversely, if uri_in is true, then a uri is
allocated and hostname is set to the uri->hostname value and
likewise generically used.

At function exit, hostname wasn't free'd in the !uri_in path,
so that was added.  To just make it clearer on usage the else
path became the call to virURIFree() although I suppose technically
it didn't have to since it would be a call using (NULL)

In the error path the 'ipaddr' wasn't VIR_FREE'd before jumping to cleanup

Need to free 'xmlFromFile' on/for the error path when current was
returning false only

Need to VIR_FREE the startip/endip we allocated for the error message

Coverity determined that on error path that 'mach' wouldn't be free'd
Since virCapabilitiesFreeGuestMachine() isn't globally available, we'll
insert first and then if the VIR_STRDUP's fail they it will eventually
cause the 'mach' to be freed in the error path

On the error path need to free the chrdef

Coverity found that on error paths, the 'arg' value wasn't be cleaned
up. Followed the example in qemuAgentSetVCPUs() where upon successful call
to qemuAgentCommand() the 'cpus' is set to NULL; otherwise, when cleanup
occurs the free the memory for 'arg'

In qemuParseISCSIString() if an error was returned, then the call
to qemuParseDriveURIString() where the uri is free'd wouldn't be run

Coverity determined that the copied 'oldguest' would be leaked for
both error and success paths.

Resolve a few RESOURCE_LEAK's identified by Coverity

In each of these cases, Coverity complains that the result count returned
on error paths would be -1 disregarding that the count and the corresponding
are "linked" together (it doesn't know that).  Simple enough to check and
remove the warning

Add "domstats" command that excercises both of the new APIs depending if
you specify a domain list or not. The output is printed as a key=value
list of the returned parameters.

In function virQEMUCapsParseMachineTypesStr, VIR_STRNDUP allocates
memory for 'name' in {do,while} loop. If 'name' isn't freed before
'continue', its memory will be allocated again in the next loop.
In this case the memory allocated for 'name' in privious loop is
useless and not freed. Free it before continue this loop to fix that.
Signed-off-by: NWang Rui <moon.wangrui@huawei.com>

The 'lib' handle will be leaked if 'dlsym' condition fails.
So close the handle before return.
Signed-off-by: NWang Rui <moon.wangrui@huawei.com>

Coverity determined that 'conflict' would be leaked.
Signed-off-by: NWang Rui <moon.wangrui@huawei.com>

Implement the remote driver support for shuffling the domain stats
around.

Add domain list filtering functions and a flag to enforce checking
whether the remote daemon supports the requested stats groups.

Add helper to free a list of virDomainPtrs without raising or clearing
errors. Use it in one place and prepare it for reuse.

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

The error message for an out-of-range argument was confusing:

virsh -k 9999999999
error: option --k requires a positive numeric argument

After this patch, it is:

error: Invalid value for option -k
Signed-off-by: NEric Blake <eblake@redhat.com>

Adjust the initialization of qemuCaps() to check for a NULL before
attempting to dereference like other callers/users do.

The call to virDomainSnapshotRedefinePrep() had a spurrious ! in front of
it which caused Coverity to complan that the expression is always false.

Coverity complains that checking for domain->def being non NULL in the
if (live) path of virDomainObjAssignDef() would be unnecessary or a
NULL deref since the call to virDomainObjIsActive() would already
dereference domain->def when checking if the def->id field was != -1.

Checked all callers to virDomainObjAssignDef() and each at some point
dereferences (vm)->def->{field} prior to calling when live is true.

In qemuNetworkIfaceConnect() a call to virNetDevBandwidthSet() is
made where the function prototype requires the first parameter
(net->ifname) to be non NULL.  Coverity complains that the subsequent
non NULL check for net->ifname prior to the next call gets flagged as
an unnecessary check.  Resolve by removing the extra check

In virDomainActualNetDefFormat() a call to virDomainNetGetActualType(def)
was made before a check for (!def) a few lines later. This triggered
Coverity to note the possible NULL deref.  Just moving the initialization
to after the !def checks resolves the issue

There were two occurrances of attempting to initialize actualType by
calling virStorageSourceGetActualType(src) prior to a check if (!src)
resulting in Coverity complaining about the possible NULL dereference
in virStorageSourceGetActualType() of src.

Resolve by moving the actualType setting until after checking !src

If virDomainDiskDefFree(disk) is called in 'skipdisk:', then it's possible
to either return to skipdisk without reallocating a new disk (via the if
condition just prior) or to end the loop having deleted the disk. Since
virDomainDiskDefFree() does not pass by reference, disk isn't changed in
this context, thus the possible issue.

There were two warnings in this module

  If the VIR_ALLOC_N(def->serials, 1) fails, then a virDomainChrDefFree(chr)
  is called and we jump to cleanup which makes the same call. Just remove
  the one after VIR_ALLOC_N()

  In the label "skipnic:" a virDomainNetDefFree(net) is made; however, if
  in going back to the top of the loop we jump back down to skipnic for any
  reason, the call will attempt to free an already freed structure since
  "net" was not passed by reference to virDomainNetDefFree().  Just set
  net = NULL in skipnic: to resolve the issue.

Coverity complains that calling virNetworkDefFree(def), then jumping
to the cleanup: label which calls virNetworkDefFree(def) could result
in a double_free.  Just remove the call from the if statement.

Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Since times when vbox moved to the daemon (due to some licensing
issue) the subdrivers that vbox implements were registered, but not
opened since our generic subdrivers took priority. I've tried to fix
this in 65b7d553 but it was not correct. Apparently moving
vbox driver registration upfront changes the default connection URI
which makes some users sad. So, this commit breaks vbox into pieces
and register vbox's network and storage drivers first, and vbox driver
then at the end. This way, the vbox driver is registered in the order
it always was, but its subdrivers are registered prior the generic
ones.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

There's this unwritten rule in libvirt that vir_function is translated
into virFunction when needed (e.g. in remote protocol definition,
python, ...). Up till now we ignored such translation in driver module
loading and did fine. Well, we didn't have any module with an
underscore in its name. But this will change in next commit. The
problem is, once an a module is dlopen()-ed, we derive register
function name from its name. So instead of "driver_subdriverRegister"
do some magic to turn that into "driverSubdriverRegister".
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Even though we kept adding new and new modules (e.g. vbox or bhyve)
the test wasn't updated. Do that now. Moreover, while it's not
crucial, it's nice to reorder test cases to match the order in which
the daemon loads the modules.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

While working on virDomainBlockCopy, I noticed we had a verify()
concerning internal XML flags that was incomplete after several
recent flag additions; move that up higher in the code to make it
harder to forget to modify on the next flag addition.  Adjust
some formatting while at it.

* src/conf/domain_conf.c (verify): Move closer to internal flag
definitions.  Cover missing flags ALLOW_ROM and ALLOW_BOOT.
Signed-off-by: NEric Blake <eblake@redhat.com>

While prepping for virDomainBlockJob patches, I found some dead code.

* tools/virsh-domain.c (blockJobImpl): Kill unused 'name'.
Signed-off-by: NEric Blake <eblake@redhat.com>

In qemuDomainRevertToSnapshot(), it will check snap->def->state.
But when the state is PMSUSPENDED/NOSTATE/BLOCKED, it forgets to
call qemuDomainObjEndJob.

https://bugzilla.redhat.com/show_bug.cgi?id=1134154
Bug introduced in commit 1e833899.
Signed-off-by: NJincheng Miao <jmiao@redhat.com>
Signed-off-by: NEric Blake <eblake@redhat.com>

Let's fix this before we bake in a painful API.  Since we know
that we have exactly one non-negative fd on success, we might
as well return the fd directly instead of forcing the user to
pass in a pointer.  Furthermore, I found some memory and fd
leaks while reviewing the code - the idea is that on success,
libvirtd will have handed two fds in two different directions:
one to qemu, and one to the RPC client.

* include/libvirt/libvirt.h.in (virDomainOpenGraphicsFD): Drop
unneeded parameter.
* src/driver.h (virDrvDomainOpenGraphicsFD): Likewise.
* src/libvirt.c (virDomainOpenGraphicsFD): Adjust interface to
return fd directly.
* daemon/remote.c (remoteDispatchDomainOpenGraphicsFd): Adjust
semantics.
* src/qemu/qemu_driver.c (qemuDomainOpenGraphicsFD): Likewise,
and plug fd leak.
* src/remote/remote_driver.c (remoteDomainOpenGraphicsFD):
Likewise, and plug memory and fd leak.
Signed-off-by: NEric Blake <eblake@redhat.com>

This commit (finally) adds the virDomainBlockCopy API, with the
intent that it will provide more power to the existing 'virsh
blockcopy' command.

'virsh blockcopy' was first added in Apr 2012 (v0.9.12), which
corresponds to the upstream qemu 1.2 timeframe.  It was done as
a hack on top of the existing virDomainBlockRebase() API call,
for two reasons: 1) it was targetting a feature that landed first
in downstream RHEL qemu, but had not stabilized in upstream qemu
at the time (and indeed, 'drive-mirror' only landed upstream in
qemu 1.3 with slight differences to the first RHEL attempt,
and later gained further parameters like granularity and buf-size
that are also worth exposing), and 2) extending an existing API
allowed it to be backported without worrying about bumping .so
versions.  A virDomainBlockCopy() API was proposed at that time
[1], but we decided not to accept it into libvirt until after
upstream qemu stabilized, and it ended up getting scrapped.
Whether or not RHEL should have attempted adding a new feature
without getting it upstream first is a debate that can be held
another day; but enough time has now elapsed that we are ready to
do the interface cleanly.

[1] https://www.redhat.com/archives/libvir-list/2012-April/msg00768.html

Delaying the creation of a clean API until now has also had a
benefit: we've only recently learned of a few shortcomings in the
original design: 1) it is unable to target a network destination
(such as a gluster volume) because it hard-coded the assumption
that the destination is a local file name.  Because of all the
refactoring we've done to add virStorageSourcePtr, we are in a
better position to declare an API that parses XML describing a
host storage source as the copy destination, which was not
possible had we implemented virDomainBlockCopy as it had been
originally envisioned (although a network target will have to wait
until a later libvirt release compared to the API addition to
actually be implemented).  2) the design of using MiB/sec as the
bandwidth throttle is rather coarse; qemu is actually tuned to
bytes/second, and libvirt is preventing access to that level of
detail.  A later patch will add flags to existing block job API
that can request bytes/second instead of back-compat MiB/s, but as
this is a new API, we can get it right to begin with.

At least I had the foresight to create 'virsh blockcopy' as a
separate command at the UI level (commit 1f06c007) rather than
leaking the underlying API overload of virDomainBlockRebase onto
shell users.

A further note on the bandwidth option: virTypedParameters
intentionally lacks unsigned long (since variable-width
interaction between mixed 32- vs. 64-bit client/server setups is
nasty), but we have to deal with the fact that we are interacting
with existing older code that mistakenly chose unsigned long
bandwidth at a point before we decided to prohibit it in all new
API.  The typed parameter is therefore unsigned long long, but
the implementation (in a later patch) will have to do overflow
detection on 32-bit platforms, as well as capping the value to
match the LLONG_MAX>>20 cap of the existing MiB/s interfaces.

* include/libvirt/libvirt.h.in (virDomainBlockCopy): New API.
(virDomainBlockJobType, virConnectDomainEventBlockJobStatus):
Update related documentation.
* src/libvirt.c (virDomainBlockCopy): Implement it.
* src/libvirt_public.syms (LIBVIRT_1.2.8): Export it.
* src/driver.h (_virDriver): New driver callback.
Signed-off-by: NEric Blake <eblake@redhat.com>