qemu: return -1 on error paths in qemuDomainSaveImageStartVM

Error paths after sending the event that domain is started written as if ret = -1
which is set at the beginning of the function. It's common idioma to keep 'ret'
equal to -1 until the end of function where it is set to 0. But here we use ret
to keep result of restore operation too and thus breaks the idioma and its users :)

Let's use different variable to hold restore result.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>

logical: Clean up allocation when building regex on the fly

Rather than a loop reallocating space to build the regex, just allocate
it once up front, then if there's more than 1 nextent, append a comma and
another regex_unit string.

Signed-off-by: John Ferlan <jferlan@redhat.com>

logical: Use 'stripes' value for mirror/raid segtype

The 'stripes' value is described as the "Number of stripes or mirrors in
a logical volume". So add "mirror" and anything that starts with "raid"
to the list of segtypes that can have an 'nextents' value greater than one.
Use of raid segtypes (raid1, raid4, raid5*, raid6*, and raid10) is favored
over mirror in more recent lvm code.

Signed-off-by: John Ferlan <jferlan@redhat.com>

logical: Use VIR_APPEND_ELEMENT instead of VIR_REALLOC_N

Rather than preallocating a set number of elements, then walking through
the extents and adjusting the specific element in place, use the APPEND
macros to handle that chore.

Signed-off-by: John Ferlan <jferlan@redhat.com>

include: Handle case when builddir == srcdir

In my previous commit a70f3b1c779120129 I've tried to fix case
when building from VPATH and a file wasn't being installed.
However, my fix broke non-VPATH build.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

tests: avoid realpath in test-lib.sh

Ever since commit ace4aecd, running 'make check' on RHEL 6 produces:

./test-lib.sh: line 21: realpath: command not found

for every shell script test, because 'realpath' was not part of
coreutils back then.

* tests/test-lib.sh (_scriptdir): Compute with only portable shell.

Signed-off-by: Eric Blake <eblake@redhat.com>

virsh: improve waiting for block job readiness

After a block job hits 100%, we only need to apply a timeout waiting for
a block job event if exactly one of the BLOCK_JOB or BLOCK_JOB_2
callbacks were able to be registered.

If neither callback could be registered, there's clearly no need for a
timeout.

If both callbacks were registered, then we're guaranteed to eventually
get one of the events. The path being used by virsh must be exactly the
source path or target device in the domain's disk definition, and these
are the respective strings sent back in these two events.

Signed-off-by: Michael Chapman <mike@very.puzzling.org>

virsh: ensure SIGINT action is reset on all errors

If virTimeMillisNow() fails, the SIGINT action must be reset back to
its previous state.

Signed-off-by: Michael Chapman <mike@very.puzzling.org>

virsh: be consistent with style of loop exit

When waiting for a block job, the various statuses (COMPLETED, READY,
CANCELED, etc.) should all be treated consistently by having the loop be
exited with "break". Use "goto cleanup" for the error cases only, when
no block job status is available.

Signed-off-by: Michael Chapman <mike@very.puzzling.org>

virsh: avoid unnecessary progress updates

There is no need to call virshPrintJobProgress() unless the block job's
cur or end cursors have changed since the last iteration.

Signed-off-by: Michael Chapman <mike@very.puzzling.org>

qemu: Mark some functions as static

fdstream: Realign

Some lines in this file are misaligned which fires up my OCD.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

qemu: Align dump options for watchdog and on_crash events

Having on_crash set to either coredump-destroy or coredump-restart
creates core dumps with option memory-only in the directory specified
by auto_dump_path. When a watchdog is triggered with the action dump
the core dump is also placed into the directory specified by auto_dump_path
but is created without the option memory-only.

This patch sets the option memory-only also for core dumps created by the
watchdog event.

Signed-off-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com>

includes: Install libvirt-common.h

The libvirt-common.h is build time generated file from .in.
Obviously, it's generated into builddir and not srcdir. Problem
is, the list of header files to install, virinc_HEADERS contains
only $(srcdir)/*.h and this misses libvirt-common.h. This problem
is pretty obvious when doing a VPATH build.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

logical: Create helper virStorageBackendLogicalParseVolExtents

Create a helper routine in order to parse any extents information
including the extent size, length, and the device string contained
within the generated 'lvs' output string.

A future patch would then be able to avoid the code more cleanly

Signed-off-by: John Ferlan <jferlan@redhat.com>

rbd: Open in Read-Only mode when refreshing a volume

By opening a RBD volume in Read-Only we do not register a
watcher on the header object inside the Ceph cluster.

Refreshing a volume only calls rbd_stat() which is a operation
which does not write to a RBD image.

This allows us to use a cephx user which has no write
permissions if we would want to use the libvirt storage pool
for informational purposes only.

It also saves us a write into the Ceph cluster which should
speed up refreshing a RBD pool.

rbd_open_read_only() is available in all librbd versions which
also support rbd_open().

Signed-off-by: Wido den Hollander <wido@widodh.nl>

rbd: Implement buildVolFrom using RBD cloning

RBD supports cloning by creating a snapshot, protecting it and create
a child image based on that snapshot afterwards.

The RBD storage driver will try to find a snapshot with zero deltas between
the current state of the original volume and the snapshot.

If such a snapshot is found a clone/child image will be created using
the rbd_clone2() function from librbd.

rbd_clone2() is available in librbd since Ceph version Dumpling (0.67) which
dates back to August 2013.

It will use the same features, strip size and stripe count as the parent image.

This implementation will only create a single snapshot on the parent image if
never changes. This reduces the amount of snapshots created for that RBD image
which benefits the performance of the Ceph cluster.

During build the decision will be made to use either rbd_diff_iterate() or
rbd_diff_iterate2().

The latter is faster, but only available on Ceph versions after 0.94 (Hammer).

Cloning is only supported if RBD format 2 is used. All images created by libvirt
are already format 2.

If a RBD format 1 image is used as the original volume the backend will report
a VIR_ERR_OPERATION_UNSUPPORTED error.

Signed-off-by: Wido den Hollander <wido@widodh.nl>

rbd: Add support for wiping RBD volumes using TRIM.

Using VIR_STORAGE_VOL_WIPE_ALG_TRIM a RBD volume can be trimmed down
to 0 bytes using rbd_discard()

Effectively all the data on the volume will be lost/gone, but the volume
remains available for use afterwards.

Starting at offset 0 the storage pool will call rbd_discard() in stripe
size * count increments which is usually 4MB. Stripe size being 4MB and
count 1.

rbd_discard() is available since Ceph version Dumpling (0.67) which dates
back to August 2013.

Signed-off-by: Wido den Hollander <wido@widodh.nl>

storage: Add TRIM algorithm to storage volume API

This new algorithm adds support for wiping volumes using TRIM.

It does not overwrite all the data in a volume, but it tells the
backing storage pool/driver that all bytes in a volume can be
discarded.

It depends on the backing storage pool how this is handled.

A SCSI backend might send UNMAP commands to remove all data present
on a LUN.

A Ceph backend might use rbd_discard() to instruct the Ceph cluster
that all data on that RBD volume can be discarded.

Signed-off-by: Wido den Hollander <wido@widodh.nl>

rbd: Add support for wiping RBD volumes

When wiping the RBD image will be filled with zeros started
at offset 0 and until the end of the volume.

This will result in the RBD volume growing to it's full allocation
on the Ceph cluster. All data on the volume will be overwritten
however, making it unavailable.

It does NOT take any RBD snapshots into account. The original data
might still be in a snapshot of that RBD volume.

Signed-off-by: Wido den Hollander <wido@widodh.nl>

storage: Adjust fix virStorageBackendVolWipeLocal switch

Use the cast of (virStorageVolWipeAlgorithm) adding the missing case:'s
(VIR_STORAGE_VOL_WIPE_ALG_ZERO and VIR_STORAGE_VOL_WIPE_ALG_LAST).

Additionally, the old code would also still run the SCRUB command on
default since it didn't go to cleanup when a invalid flag was supplied.
We now go to cleanup and exit if a invalid flag would be provided.

Signed-off-by: Wido den Hollander <wido@widodh.nl>

logical: Fix comment examples for virStorageBackendLogicalFindLVs

When commit id '82c1740a' made changes to the output format (changing from
using a ',' separator to '#'), the examples in the lvs output from the
comments weren't changed.

Additionally, the two new fields added ('segtype' and 'stripes') were
not included in the output, leaving it well confusing.

This patch fixes the sample output, adds a 'striped' example, and makes
other comment related adjustments for long line and spacing between followup
'NB' remarks (while I'm there).

Signed-off-by: John Ferlan <jferlan@redhat.com>

pci: Use bool return type for some virPCIDeviceGet*() functions

The affected functions are:

  virPCIDeviceGetManaged()
  virPCIDeviceGetUnbindFromStub()
  virPCIDeviceGetRemoveSlot()
  virPCIDeviceGetReprobe()

Change their return type from unsigned int to bool: the corresponding
members in struct _virPCIDevice are defined as bool, and even the
corresponding virPCIDeviceSet*() functions take a bool value as input
so there's no point in these functions having unsigned int as return
type.

Suggested-by: John Ferlan <jferlan@redhat.com>

gendispatch: Don't output spaces on empty line

In our generator for some code we put empty lines in the output
to separate blocks of code. However, in some cases we put couple
of spaces on the empty line too. It's not bug, it just isn't
nice.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

pci: Add debug messages when unbinding from stub driver

Unbinding a PCI device from the stub driver can require several steps,
and it can be useful for debugging to be able to trace which of these
steps are performed and which are skipped for each device.

pci: Phase out virPCIDeviceReattachInit()

The name is confusing, and there are just two uses: one is a test case,
and the other will be removed as part of an upcoming refactoring of
the hostdev code.

qemu: don't iterate vcpus using priv->nvcpupids in qemuProcessSetSchedParams

This should be the last offender.

conf: disallow empty cpuset for emulatorpin

It's disallowed in the API.

conf: disallow empty cpusets for vcpu pinning when parsing XML

They are disallowed in the pinning API and as default cpuset.

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

qemu: Don't use priv->ncpus to iterate cgroup setting

Iterate over all cpus skipping inactive ones.

virnetdevopenvswitch: Don't call strlen() twice on the same string

Commit 871e10f fixed a memory corruption error, but called strlen()
twice on the same string to do so. Even though the compiler is
probably smart enough to optimize the second call away, having a
single invocation makes the code slightly cleaner.

Suggested-by: Michal Privoznik <mprivozn@redhat.com>

virnetdevmacvlan: Provide stubs for build without macvtap

In 370608b4c76f we have introduced two new internal APIs.
However, there are no stubs for build without macvtap. Therefore
build on systems lacking macvtap support (e.g. mingw or freebds)
fails when trying to link.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Fix libvirtd free() segfault when migrating guest with deleted open vswitch port

libvirtd crashes on free()ing portData for an open vswitch port if that port
was deleted.  To reproduce:

ovs-vsctl del-port vnet0
virsh migrate --live kvm1 qemu+ssh://dstHost/system

Error message:
libvirtd: *** Error in `/usr/sbin/libvirtd': free(): invalid pointer: 0x000003ff90001e20 ***

The problem is that virCommandRun can return an empty string in the event that
the port being queried does not exist. When this happens then we are
unconditionally overwriting a newline character at position strlen()-1. When
strlen is 0, we overwrite memory that does not belong to the string.

The fix: Only overwrite the newline if the string is not empty.

Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com>
Signed-off-by: Jason J. Herne <jjherne@linux.vnet.ibm.com>

util: keep/use a bitmap of in-use macvtap devices

This patch creates two bitmaps, one for macvlan device names and one
for macvtap. The bitmap position is used to indicate that libvirt is
currently using a device with the name macvtap%d/macvlan%d, where %d
is the position in the bitmap. When requested to create a new
macvtap/macvlan device, libvirt will now look for the first clear bit
in the appropriate bitmap and derive the device name from that rather
than just starting at 0 and counting up until one works.

When libvirtd is restarted, the qemu driver code that reattaches to
active domains calls the appropriate function to "re-reserve" the
device names as it is scanning the status of running domains.

Note that it may seem strange that the retry counter now starts at
8191 instead of 5. This is because we now don't do a "pre-check" for
the existence of a device once we've reserved it in the bitmap - we
move straight to creating it; although very unlikely, it's possible
that someone has a running system where they have a large number of
network devices *created outside libvirt* named "macvtap%d" or
"macvlan%d" - such a setup would still allow creating more devices
with the old code, while a low retry max in the new code would cause a
failure. Since the objective of the retry max is just to prevent an
infinite loop, and it's highly unlikely to do more than 1 iteration
anyway, having a high max is a reasonable concession in order to
prevent lots of new failures.

util: increase libnl buffer size

In the following cases nl_recv() was returning the error "No buffer
space available":

* When switching CPUs to offline/online in a system more than 128 cpus
* When using virsh to destroy domain in a system with many interfaces

This patch sets the buffer size for all netlink sockets created by
libnl to 128K and turns on message peeking for nl_recv(). This
eliminates the "No buffer space available" errors seen in the cases
above, and also preempts other future errors the smaller buffers could
have caused.

Signed-off-by: Leno Hou <houqy@linux.vnet.ibm.com>
Signed-off-by: Laine Stump <laine@laine.org>

device: cleanup input device code

The current code was a little bit odd.  At first we've removed all
possible implicit input devices from domain definition to add them later
back if there was any graphics device defined while parsing XML
description.  That's not all, while formating domain definition to XML
description we at first ignore any input devices with bus different to
USB and VIRTIO and few lines later we add implicit input devices to XML.

This seems to me as a lot of code for nothing.  This patch may look
to be more complicated than original approach, but this is a preferred
way to modify/add driver specific stuff only in those drivers and not
deal with them in common parsing/formating functions.

The update is to add those implicit input devices into config XML to
follow the real HW configuration visible by guest OS.

There was also inconsistence between our behavior and QEMU's in the way,
that in QEMU there is no way how to disable those implicit input devices
for x86 architecture and they are available always, even without graphics
device.  This applies also to XEN hypervisor.  VZ driver already does its
part by putting correct implicit devices into live XML.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

tests: add some missing tests to qemuxml2xmltest

Those tests are in qemuargv2xmltest and it makes sense to include them
also in qemuxml2xmltest and qemuxml2argvtest.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

tests: use virtTestDifferenceFull in tests where we have output file

This will enable regenerate functionality for those tests to make
developer lives easier while updating tests.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

vircgroup: Finish renaming of virCgroupIsolateMount

In dc576025c360 we renamed virCgroupIsolateMount function to
virCgroupBindMount. However, we forgot about one occurrence in
section of the code which provides stubs for platforms without
support for CGroups like *BSD for instance.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

lxc: don't try to hide parent cgroups inside container

On the host when we start a container, it will be
placed in a cgroup path of

   /machine.slice/machine-lxc\x2ddemo.scope

under /sys/fs/cgroup/*

Inside the containers' namespace we need to setup
/sys/fs/cgroup mounts, and currently will bind
mount /machine.slice/machine-lxc\x2ddemo.scope on
the host to appear as / in the container.

While this may sound nice, it confuses applications
dealing with cgroups, because /proc/$PID/cgroup
now does not match the directory in /sys/fs/cgroup

This particularly causes problems for systems and
will make it create repeated path components in
the cgroup for apps run in the container eg

  /machine.slice/machine-lxc\x2ddemo.scope/machine.slice/machine-lxc\x2ddemo.scope/user.slice/user-0.slice/session-61.scope

This also causes any systemd service that uses
sd-notify to fail to start, because when systemd
receives the notification it won't be able to
identify the corresponding unit it came from.
In particular this break rabbitmq-server startup

Future kernels will provide proper cgroup namespacing
which will handle this problem, but until that time
we should not try to play games with hiding parent
cgroups.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

qemu: add reporting of vCPU wait time

The VIR_DOMAIN_STATS_VCPU flag to virDomainListGetStats
enables reporting of stats about vCPUs. Currently we
only report the cumulative CPU running time and the
execution state.

This adds reporting of the wait time - time the vCPU
wants to run, but the host scheduler has something else
running ahead of it.

The data is reported per-vCPU eg

$ virsh domstats --vcpu demo
 Domain: 'demo'
   vcpu.current=4
   vcpu.maximum=4
   vcpu.0.state=1
   vcpu.0.time=1420000000
   vcpu.0.wait=18403928
   vcpu.1.state=1
   vcpu.1.time=130000000
   vcpu.1.wait=10612111
   vcpu.2.state=1
   vcpu.2.time=110000000
   vcpu.2.wait=12759501
   vcpu.3.state=1
   vcpu.3.time=90000000
   vcpu.3.wait=21825087

In implementing this I notice our reporting of CPU execute
time has very poor granularity, since we are getting it
from /proc/$PID/stat. As a future enhancement we should
prefer to get CPU execute time from /proc/$PID/schedstat
or /proc/$PID/sched (if either exist on the running kernel)

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

virsh: fix cpu-stats command output format issue

After commit 57177f1, the cpu-stats command format change to:

CPU0:
    cpu_time         14401.507878990 seconds
    vcpu_time    14378732785511

vcpu_time is not user friendly. After this patch, it will
change back:
CPU0:
    cpu_time         14401.507878990 seconds
    vcpu_time        14378.732785511 seconds

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

Signed-off-by: Luyao Huang <lhuang@redhat.com>

util: buffer: Sanitize comment for virBufferAddBuffer

Idioms are usually weird and obscure when translated literally.

test: Touch up error message when attempting to pin invalid vCPU

Report
error: invalid argument: requested vcpu '100' is not present in the domain
instead of
error: invalid argument: requested vcpu is higher than allocated vcpus

tests: qemuxml2xml: Order pinning information numerically

A future patch will refactor the storage of the pinning information in a
way where the ordering will be lost. Order them numerically to avoid
changing the tests later.

virsh: cpu-stats: Remove unneeded flags

virDomainGetCPUStats doesn't support flags so there's no need to carry
the 'flags' variable around. Additionally since the API is poorly
designed I doubt that it will be extended.

virsh: cpu-stats: Extract common printing code into a function

Simplify the code by extracting a common code path.

(qemu|lxc)DomainGetCPUStats: Clean up

Remove unnecessary condition and variable.

vz: Fix invalid iteration of def->cputune.vcpupin

The array doesn't necessarily have the same cardinality as the count of
vCPUs for a domain. Iterating it can cause access beyond the end of the
array.

qemu: process: Disallow VMs with 0 vcpus

Counterintuitively the user would end up with a VM with maximum number
of vCPUs available.

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

qemu: process: refactor and rename qemuValidateCpuMax to qemuValidateCpuCount

Next patch will add minimum checking, so use a more generic name.
Refactor return values to the commonly used semantics.

virt-host-validate: Fix error level for user namespace check

From the code it seems to me that we need user namespace if
configured in domain XML. Otherwise we don't use it at all.
However our tool is more strict about that. Fix this discrepancy.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

virt-host-validate: Check those CGroups that we actually use

Since the introduction of virt-host-validate tool the set of
cgroup controllers we use has changed so the tool is checking for
some cgroups that we don't need (e.g. net_cls, although I doubt
we have ever used that one) and is not checking for those we
actually use (e.g. cpuset).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

virsh: Correctly detect inserted media in change-media command

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

It all works like this. The change-media command dumps domain
XML, finds the corresponding cdrom device we want to change media
in and returns it in the xmlNodePtr form. This way we don't have
to bother with keeping all the subelements or attributes that we
don't care about in the XML that is fed back to libvirt for the
update API.

Now, the problem is we try to be clever here and detect if disk
already has a source (indicated by <source/> subelement).
However, bare fact that the element is there does not mean disk
has source. Make our clever check better.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

vmx: Adapt to emptyBackingString for cdrom-image

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

We are missing this value for cdrom-image device. It seems like
there's no added value to extend this to other types of disk
devices [1].

1: https://www.redhat.com/archives/libvir-list/2016-January/msg01038.html

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

qemu: snapshot: Correctly report qemu error on 'savevm'

Since 'savevm' was not converted to QMP libvirt has to parse for error
strings in the text monitor output. One of the unhandled errors is
produced when qemu treats a device as unmigratable.

As current qemu actually does support AHCI migration this bug is
applicable only to older versions of qemu.

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

qemu: monitor: Refactor error handling for 'savevm'

Unify few error conditions into a single error reporting case.

bhyve: bhyveload: respect boot dev and boot order

Make bhyveload respect boot order as specified by os.boot section of the
domain XML or by "boot order" for specific devices. As bhyve does not
support a real boot order specification right now, it's just about
choosing a single device to boot from.

conf: expose virDomainBootType(From|To)String

These functions are going to be used by the Bhyve driver.

util: reset MAC address of macvtap passthrough physdev after disassociate

libvirt always resets the MAC address of the physdev used for macvtap
passthrough when the guest is finished with it. This was happening
prior to the 802.1Qb[gh] DISASSOCIATE command, and was quite often
failing, presumably because the driver wouldn't allow the MAC address
to be reset while the association was still active, with a log message
like this:

virNetDevSetMAC:168 : Cannot set interface MAC to 00:00:00:00:00:00 on 'eth13': Cannot assign requested address

This patch changes the order - we now do the 802.1Qb[gh] disassociate
and delete the macvtap interface first, then and reset the MAC
address.

lxc: fuse: Stub out Slab bits in /proc/meminfo

'free' on fedora23 wants to use the Slab field for calculated used
memory. The equation is:

used = MemTotal - MemFree - (Cached + Slab) - Buffers

We already set Cached and Buffers to 0, do the same for Slab and its
related values

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

lxc: fuse: Fill in MemAvailable for /proc/meminfo

'free' on Fedora 23 will use MemAvailable to calculate its 'available'
field, but we are passing through the host's value. Set it to match
MemFree, which is what 'free' will do for older linux that don't have
MemAvailable

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

lxc: fuse: Fix /proc/meminfo size calculation

We virtualize bits of /proc/meminfo by replacing host values with
values specific to the container.

However for calculating the final size of the returned data, we are
using the size of the original file and not the altered copy, which
could give garbelled output.

lxc: fuse: Unindent meminfo logic

Reverse the conditional at the start so we aren't stuffing all the logic
in an 'if' block

libxl: Support cmdline= in xl config files

... and consolidate the cmdline/extra/root parsing to facilitate doing
so.

The logic is the same as xl's parse_cmdline from the current xen.git master
branch (e6f0e099d2c17de47fd86e817b1998db903cab61).

On the formatting side switch to producing cmdline= instead of extra=.

Update a few tests and add serveral more.
  - test-cmdline is added to test the exclusive use of cmdline.
  - test-fullvirt-direct-kernel-boot.cfg is updated due to the switch
    on the formatting side and now tests the exclusive use of cmdline=.
  - Tests are added for both paravirt and fullvirt where the .cfg uses
    extra= and (paravirt only) root=. These are format (xl->xml) only
    since the inverse will generate cmdline= hence is not a round trip
    (which was already true if using root=, which used to generate
    extra= on the way back).
  - Tests are added for both paravirt and fullvirt where the .cfg
    declares cmdline= as well as bogus extra= and (paravirt only) root=
    entries which should be ignored. Again these are format only tests
    since the inverse won't include the bogus lines.

The last two bullets here required splitting the DO_TEST macro into
two halves, as is done in the xmconfigtest.c case.

In order to introduce a use of VIR_WARN for logging I had to add
virerror.h and VIR_LOG_INIT.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

libxl: dispose libxl_dominfo after libxl_domain_info()

As suggested in a previous thread [0] this patch adds some missing calls
to libxl_dominfo_{init,dispose} when doing some of the libxl_domain_info
operations which would otherwise lead to memory leaks.

[0]
https://www.redhat.com/archives/libvir-list/2015-September/msg00519.html

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>

Xen: add XENXL to virErrorDomain enum

Add "Xen XL Config" to the virErrorDomain enum and use it in
src/xenconfig/xen_xl.c.

Xen: VIR_FROM_THIS cleanup

The virErrorDomain enum has VIR_FROM_XEN, VIR_FROM_XEND,
VIR_FROM_XENSTORE, VIR_FROM_SEXPR, and VIR_FROM_XENXM. Use
these elements in the corresponding .c files. While at it,
remove the VIR_FROM_THIS define in src/xenconfig/xenxs_private.h.

qemu: Add support for migration iteration event

The corresponding event in QEMU is called MIGRATION_PASS.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

Introduce migration iteration event

The VIR_DOMAIN_EVENT_ID_MIGRATION_ITERATION event will be triggered
whenever VIR_DOMAIN_JOB_MEMORY_ITERATION changes its value, i.e.,
whenever a new iteration over guest memory pages is started during
migration.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

qemuDomainReboot: use fakeReboot=true only for acpi mode

When acpi is used to reboot/shutdown qemu domain, qemu emits
SHUTDOWN event. Libvirt uses fakeReboot variable in order to
differentiate reboot or shutdown. fakeReboot value is reseted
to false after domain restart/reset.

When mode=agent is used to reboot qemu domain, qemu doesn't emit
SHUTDOWN event and libvirt doesn't reset fakeReboot value to false.
In this case next 'shutdown -h now' performs reboot. That's why
we don't need to set fakeReboot=true for mode=agent.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

virsh: Don't fetch status for all domains in cmdList

We are getting the list of domains and after that we iterate over
the list and try to get status for each domain hoping it will
skip over domains that disappeared meanwhile. However, this
solution to race is bogus - domain may disappear right after we
have checked its state and before we exec another API over it
(e.g. virDomainHasManagedSaveImage()). Also, when printing just
names or uuids (list --name / --uuid) we issue APIs to obtain the
values, however these require no RPC call as all requested info
is in virDomain object that client already has.
Therefore move the status obtaining only to the place that really
needs it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

docs: fix syntax-check long line error

build: predictably generate systemtap tapsets (bz 1173641)

The generated output is dependent on perl hashtable ordering, which
gives different results for i686 and x86_64. Fix this by sorting
the hash keys before iterating over them

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

docs: fix generation of docs from VPATH build

When generating docs in a VPATH build we get a failure to
create a file due to the 'internals' subdir not existing:

  Generating internals/locking.html.tmp
  /bin/sh: line 3: internals/locking.html.tmp: No such file or directory
  rm: cannot remove ‘internals/locking.html.tmp’: No such file or directory
  Makefile:2229: recipe for target 'internals/locking.html.tmp' failed
  make: *** [internals/locking.html.tmp] Error 1

For some reason, make has decided to run the target

  %.html.tmp: %.html.in site.xsl page.xsl sitemap.html.in $(acl_generated)

instead of the target

  internals/%.html.tmp: internals/%.html.in subsite.xsl page.xsl sitemap.html.in

Removing '$(acl_generated)' from the first target, inexplicably
causes make to now run the correct target for the internals/
files.

Rather than figure this out, lets just combine the two targets
into one.

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

leaseshelper: reduce indentation level in virLeaseReadCustomLeaseFile

Instead of nested ifs, jump out early.

Mostly whitespace changes.

leaseshelper: remove useless comparison

We do not care if the mac was specified in the delete section,
we are going to delete the record anyway.

leaseshelper: move comment about adding IPv6 leases

The comment is relevant to the ADD action, not DEL.

leaseshelper: split out virLeasePrintLeases

Introduce a function for printing the leases on the 'init' operation.

leaseshelper: split out custom leases file read

Introduce virLeaseReadCustomLeaseFile which will populate
the new leases array with all the leases, except for expired
ones and the ones matching 'ip_to_delete'.

This removes five variables from main().

leaseshelper: store server_duid as an allocated string

We either use the value from the environment variable, or learn it from
the existing lease file.

In the second case, the pointer would be pointing into the JSON object
of the first lease with a DUID, owned by leases_array, then
leases_array_new.

Always allocate the string instead, making obvious who should free the
string.

leaseshelper: fix crash when no mac is specified

If dnsmasq specified DNSMASQ_IAID (so we're dealing with an IPv6
lease) but no DNSMASQ_MAC, we skip creation of the new lease object.

Also skip adding it to the leases array.

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

storage: Add new flag for libvirt_parthelper

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

In order to be able to process disk storage pool's using a multipath
device to handle the partitions, libvirt_parthelper will need a way to
not automatically add a partition separator "p" to the generated device
name for each partition found. This is designed to mimic the multipath
features known as 'user_friendly_names' and custom 'alias' name.

If the part_separator attribute is set to "no", then generation of the
multipath partition name will not include the "p" partition separator
unless the source device path name ends with a number. The generated
partition names that get passed back to libvirt are processed in order
to find the device mapper multipath (dm-#) path device.

For example, device path "/dev/mapper/mpatha" would create partitions
"/dev/mapper/mpatha1", "/dev/mapper/mpatha2", etc. instead of
"/dev/mapper/mpathap1", "/dev/mapper/mpathap2", etc. If the device
path ends with a number "/dev/mapper/mpatha1", then the algorithm
to generate names "/dev/mapper/mpatha1p1", "/dev/mapper/mpatha1p2", etc.
would be utilized.

Signed-off-by: John Ferlan <jferlan@redhat.com>

conf: Add storage pool device attribute part_separator

Add a new storage pool source device attribute 'part_separator=[yes|no]'
in order to allow a 'disk' storage pool using a device mapper multipath
device to not add the "p" partition separator to the generated device
name when libvirt_parthelper is run.

This will allow libvirt to find device mapper multipath devices which were
configured in /etc/multipath.conf to use 'user_friendly_names' or custom
'alias' names for the LUN.

virLogManagerDomainReadLogFile: Don't do dummy allocs

Since we pass dummy variables @fdout and @fdoutlen into
virNetClientProgramCall() we make it alloc @fdout array (even
though it's an array of 0 elements since vitlogd can hardly pass
us some FDs at this stage). Nevertheless, it's an allocation not
followed by free():

==29385== 0 bytes in 60 blocks are definitely lost in loss record 2 of 1,009
==29385==    at 0x4C2C070: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==29385==    by 0x54B99EF: virAllocN (viralloc.c:191)
==29385==    by 0x56821B1: virNetClientProgramCall (virnetclientprogram.c:359)
==29385==    by 0x563B304: virLogManagerDomainReadLogFile (log_manager.c:272)
==29385==    by 0x217CD613: qemuDomainLogContextRead (qemu_domain.c:2485)
==29385==    by 0x217EDC76: qemuProcessReadLog (qemu_process.c:1660)
==29385==    by 0x217EDE1D: qemuProcessReportLogError (qemu_process.c:1696)
==29385==    by 0x217EE8C1: qemuProcessWaitForMonitor (qemu_process.c:1957)
==29385==    by 0x217F6636: qemuProcessLaunch (qemu_process.c:4955)
==29385==    by 0x217F71A4: qemuProcessStart (qemu_process.c:5152)
==29385==    by 0x21846582: qemuDomainObjStart (qemu_driver.c:7396)
==29385==    by 0x218467DE: qemuDomainCreateWithFlags (qemu_driver.c:7450)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

qemuProcessReadLog: Fix memmove arguments

So I can observe this crasher that with freshly started daemon
(and virtlogd enabled) I am trying to startup a domain that
immediately dies (because it's said to use huge pages but I
haven't allocated a single one in the pool). Hardly reproducible
with -O0 or under valgrind. But I just got lucky:

==20469== Invalid write of size 8
==20469==    at 0x4C2E99B: memcpy@GLIBC_2.2.5 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==20469==    by 0x217EDD07: qemuProcessReadLog (qemu_process.c:1670)
==20469==    by 0x217EDE1D: qemuProcessReportLogError (qemu_process.c:1696)
==20469==    by 0x217EE8C1: qemuProcessWaitForMonitor (qemu_process.c:1957)
==20469==    by 0x217F6636: qemuProcessLaunch (qemu_process.c:4955)
==20469==    by 0x217F71A4: qemuProcessStart (qemu_process.c:5152)
==20469==    by 0x21846582: qemuDomainObjStart (qemu_driver.c:7396)
==20469==    by 0x218467DE: qemuDomainCreateWithFlags (qemu_driver.c:7450)
==20469==    by 0x21846845: qemuDomainCreate (qemu_driver.c:7468)
==20469==    by 0x5611CD0: virDomainCreate (libvirt-domain.c:6753)
==20469==    by 0x125D9A: remoteDispatchDomainCreate (remote_dispatch.h:3613)
==20469==    by 0x125CB7: remoteDispatchDomainCreateHelper (remote_dispatch.h:3589)
==20469==  Address 0x27a52ad0 is 0 bytes after a block of size 5,584 alloc'd
==20469==    at 0x4C29F80: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==20469==    by 0x9B8D1DB: xdr_string (in /lib64/libc-2.21.so)
==20469==    by 0x563B39C: xdr_virLogManagerProtocolNonNullString (log_protocol.c:24)
==20469==    by 0x563B6B7: xdr_virLogManagerProtocolDomainReadLogFileRet (log_protocol.c:123)
==20469==    by 0x164B34: virNetMessageDecodePayload (virnetmessage.c:407)
==20469==    by 0x5682360: virNetClientProgramCall (virnetclientprogram.c:379)
==20469==    by 0x563B30E: virLogManagerDomainReadLogFile (log_manager.c:272)
==20469==    by 0x217CD613: qemuDomainLogContextRead (qemu_domain.c:2485)
==20469==    by 0x217EDC76: qemuProcessReadLog (qemu_process.c:1660)
==20469==    by 0x217EDE1D: qemuProcessReportLogError (qemu_process.c:1696)
==20469==    by 0x217EE8C1: qemuProcessWaitForMonitor (qemu_process.c:1957)
==20469==    by 0x217F6636: qemuProcessLaunch (qemu_process.c:4955)

This points to memmove() in qemuProcessReadLog(). Imagine we just
read the following string from qemu:

"abc\n2016-01-18T09:40:44.022744Z qemu-system-x86_64: Error\n"

After the first pass of the while() loop in the
qemuProcessReadLog() (in which we have taken the false branch in
the if) @buf still points to the beginning of the string,
@filter_next points to the beginning of the second line.  So we
start second iteration because there is yet another newline
character at the end. In this iteration @eol points to it
actually. Now, the control gets inside true branch of if(). Just
to remind you:

got = 58
filter_next = buf + 5,
eol = buf + 58.

Therefore skip = 54 which is correct. The message we want to skip
is 54 bytes long. However:

memmove(filter_next, eol + 1, (got - skip) +1);

which is

memmove(filter_next, eol + 1, 5)

is obviously wrong as there is only one byte we can access, not 5!

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Fix make check with gcc version 5

When building with gcc-5 (particularly gcc-5.3.0 now) and having pdwtags
installed (package dwarves) make check fails with the following error:

  $ make lock_protocol-struct
  GEN      lock_protocol-struct
  --- lock_protocol-structs 2016-01-13 15:04:59.318809607 +0100
  +++ lock_protocol-struct-t3 2016-01-13 15:05:17.703501234 +0100
  @@ -26,10 +26,6 @@
           virLockSpaceProtocolNonNullString name;
           u_int                      flags;
   };
  -enum virLockSpaceProtocolAcquireResourceFlags {
  -        VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED = 1,
  -        VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE = 2,
  -};
   struct virLockSpaceProtocolAcquireResourceArgs {
           virLockSpaceProtocolNonNullString path;
           virLockSpaceProtocolNonNullString name;
  Makefile:10415: recipe for target 'lock_protocol-struct' failed
  make: *** [lock_protocol-struct] Error 1

That happens because without any specific options gcc doesn't keep enum
information in the resulting binary object.  I managed to isolate the
parameters of gcc that caused this issue to disappear, however I
remember that they influenced the resulting binaries quite a bit and
were definitely not something we would want to add as mandatory to the
build process.

So to deal with this cleanly, let's take that enum and separate it out
to its own header file.  Since it is only used in the lockd driver and
the protocol, lock_driver_lockd.h feels like a suitable name.

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

rbd: Set r variable so it can be returned should an error occur

This was reported in bug #1298024 where r would be filled with the
return code of rbd_open().

Should rbd_snap_unprotect() fail for any reason the virReportSystemError
call would return 'Success' since rbd_open() succeeded.

https://bugzilla.redhat.com/show_bug.cgi?id=1298024
Signed-off-by: Wido den Hollander <wido@widodh.nl>

qemuTestDriverInit: fill driver with zeroes

In the commit aea47e48c473a we have fixed a single pointer within
driver structure. Since all callers pass statically allocated
driver on stack other pointers within driver may contain random
values too. Before touching it lets overwrite it with zeroes and
thus fix all dangling pointers.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Post-release version bump to 1.3.2

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

NEWS: Don't prefix version numbers with 'v'

It was only used in two instances, so get rid of it for
consistency's sake.

NEWS: Fix whitespace issues

NEWS: Move 2015 entries to a separate file

Now that the first release made in 2016 is out, we can move all
entries for 2015 to their own file, just like we did for all previous
years.

Release of libvirt-1.3.1

* docs/news.html.in libvirt.spec.in: update for release
* po/*.po*: regenerated

security: Do not restore labels on device tree binary

A device tree binary file specified by /domain/os/dtb element is a
read-only resource similar to kernel and initrd files. We shouldn't
restore its label when destroying a domain to avoid breaking other
domains configure with the same device tree.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

security: Do not restore kernel and initrd labels

Kernel/initrd files are essentially read-only shareable images and thus
should be handled in the same way. We already use the appropriate label
for kernel/initrd files when starting a domain, but when a domain gets
destroyed we would remove the labels which would make other running
domains using the same files very unhappy.

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

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

qemu: Print better warning in qemuAgentNotifyEvent

We have this function qemuAgentNotifyEvent() which is supposed to
be called from thread pool responsible for processing qemu
monitor events. The function then should wake up other thread
that is waiting for a guest to shutdown or reboot. However, if we
have received a different error a warning is printed out. This
warning lacks info on which event is expected.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

cgroup: Fix possible bug as a result of code motion for vcpu cgroup setup

Commit id '90b721e43' moved where the virCgroupAddTask was made until
after the check for the vcpupin checks. However, in doing so it missed
an option where if the cpumap didn't exist, then the code would continue
back to the top of the current vcpu loop. The results was that the
virCgroupAddTask wouldn't be called.

Signed-off-by: John Ferlan <jferlan@redhat.com>

Revert "lxc_cgroup: Add check for NULL cgroup before AddTask call"

This reverts commit ae09988eb787df63d3bb298f713a3bbd77275901.

Since commit id '71ce4759' has been reverted, this one is no
longer necessary.

Revert "util: cgroups do not implicitly add task to new machine cgroup"

This reverts commit 71ce475967b3523882b0e1a0f649fdbea5abf9d5.

Since commit id 'a41c00b47' has been reverted, this no longer is
necessary