54 min agoCVE-2014-3633: qemu: blkiotune: Use correct definition when looking up disk master CVE-2014-3633
Peter Krempa [Thu, 11 Sep 2014 14:35:53 +0000]
CVE-2014-3633: qemu: blkiotune: Use correct definition when looking up disk

Live definition was used to look up the disk index while persistent one
was indexed leading to a crash in qemuDomainGetBlockIoTune. Use the
correct def and report a nice error.

Unfortunately it's accessible via read-only connection, though it can
only crash libvirtd in the cases where the guest is hot-plugging disks
without reflecting those changes to the persistent definition.  So
avoiding hotplug, or doing hotplug where persistent is always modified
alongside live definition, will avoid the out-of-bounds access.

Introduced in: eca96694a7f992be633d48d5ca03cedc9bbc3c9aa (v0.9.8)
Reported-by: Luyao Huang <>
Signed-off-by: Peter Krempa <>

60 min agoqemu: Honor hugepages for UMA domains
Michal Privoznik [Tue, 2 Sep 2014 14:53:10 +0000]
qemu: Honor hugepages for UMA domains

There are two ways how to tell qemu to use huge pages. The first one
is suitable for domains with NUMA nodes: the path to hugetlbfs mount
is appended to NUMA node definition on the command line. The second
one is suitable for UMA domains: here there's this global '-mem-path'
argument that accepts path to the hugetlbfs mount point. However, the
latter case was not used for all the cases that it should be. For

      <page size='2048' unit='KiB' nodeset='0'/>

didn't trigger the '-mem-path' so the huge pages - despite being
configured - were not used at all.

Signed-off-by: Michal Privoznik <>

60 min agoconf: Disallow nonexistent NUMA nodes for hugepages
Michal Privoznik [Mon, 15 Sep 2014 09:59:09 +0000]
conf: Disallow nonexistent NUMA nodes for hugepages

As of 136ad4974 it is possible to specify different huge pages per
guest NUMA node. However, there's no check if nodeset specified in
./hugepages/page contains only those guest NUMA nodes that exist.
In other words with current code it is possible to define meaningless

      <page size='1048576' unit='KiB' nodeset='0,2-3'/>
      <page size='2048' unit='KiB' nodeset='1,4'/>
  <vcpu placement='static'>4</vcpu>
      <cell id='0' cpus='0' memory='1048576'/>
      <cell id='1' cpus='1' memory='1048576'/>
      <cell id='2' cpus='2' memory='1048576'/>
      <cell id='3' cpus='3' memory='1048576'/>

Notice the node 4 in <hugepages/>?

Signed-off-by: Michal Privoznik <>

77 min agoman: virsh: Add docs for supported stats groups
Peter Krempa [Mon, 15 Sep 2014 15:32:42 +0000]
man: virsh: Add docs for supported stats groups

Document the fields returned.

77 min agolib: Document that virConnectGetAllDomainStats may omit some stats fields
Peter Krempa [Mon, 15 Sep 2014 15:17:17 +0000]
lib: Document that virConnectGetAllDomainStats may omit some stats fields

Add a note to make the users aware that some stats groups or fields may
be missing in certain cases.

77 min agolib: De-duplicate stats group documentation for all stats functions
Peter Krempa [Mon, 15 Sep 2014 15:13:24 +0000]
lib: De-duplicate stats group documentation for all stats functions

State that full stats for the stats groups are available in the
virConnectGetAllDomainStats documentation section rather than
duplicating the docs.

78 min agovirsh: add options to query bulk stats group
Francesco Romani [Mon, 15 Sep 2014 08:48:10 +0000]
virsh: add options to query bulk stats group

Add new bulk stats groups to the domstats command.

Signed-off-by: Francesco Romani <>

78 min agoqemu: bulk stats: implement block group
Francesco Romani [Mon, 15 Sep 2014 08:48:09 +0000]
qemu: bulk stats: implement block group

This patch implements the VIR_DOMAIN_STATS_BLOCK group of statistics.

To do so, a helper function to get the block stats of all the disks of
a domain is added.

Signed-off-by: Francesco Romani <>
Signed-off-by: Peter Krempa <>

78 min agoqemu: bulk stats: implement interface group
Francesco Romani [Mon, 15 Sep 2014 08:48:08 +0000]
qemu: bulk stats: implement interface group

This patch implements the VIR_DOMAIN_STATS_INTERFACE group of

Signed-off-by: Francesco Romani <>
Signed-off-by: Peter Krempa <>

78 min agoqemu: bulk stats: implement VCPU group
Francesco Romani [Mon, 15 Sep 2014 08:48:07 +0000]
qemu: bulk stats: implement VCPU group

This patch implements the VIR_DOMAIN_STATS_VCPU group of statistics. To
do so, this patch also extracts a helper to gather the vCPU information.

Signed-off-by: Francesco Romani <>
Signed-off-by: Peter Krempa <>

78 min agoqemu: bulk stats: implement balloon group
Francesco Romani [Mon, 15 Sep 2014 08:48:06 +0000]
qemu: bulk stats: implement balloon group

This patch implements the VIR_DOMAIN_STATS_BALLOON group of statistics.

Signed-off-by: Francesco Romani <>

78 min agoqemu: bulk stats: implement CPU stats group
Francesco Romani [Mon, 15 Sep 2014 08:48:05 +0000]
qemu: bulk stats: implement CPU stats group

This patch implements the VIR_DOMAIN_STATS_CPU_TOTAL group of

Signed-off-by: Francesco Romani <>
Signed-off-by: Peter Krempa <>

78 min agoqemu: bulk stats: extend internal collection API
Francesco Romani [Mon, 15 Sep 2014 08:48:04 +0000]
qemu: bulk stats: extend internal collection API

Future patches which will implement more bulk stats groups for QEMU will
need to access the connection object.

To accommodate that, a few changes are needed:

* enrich internal prototype to pass qemu driver object

* add per-group flag to mark if one collector needs monitor access or not

* If at least one collector of the requested stats needs monitor access
  we must start a query job for each domain.  The specific collectors
  will run nested monitor jobs inside that.

* If the job can't be acquired we pass flags to the collector so
  specific collectors that need monitor access can be skipped in order
  to gather as much data as is possible.

Signed-off-by: Francesco Romani <>
Signed-off-by: Peter Krempa <>

110 min agodomaincapstest: Run cleanly on systems missing OVMF firmware
Michal Privoznik [Wed, 17 Sep 2014 15:17:03 +0000]
domaincapstest: Run cleanly on systems missing OVMF firmware

As of f05b6a918e28 the test produces the list of paths that can
be passed to <loader/> and libvirt knows about them. However,
during the process of generating the list the paths are checked
for their presence. This may produce different results on
different systems.  Therefore, the path - if missing - is
added to pretend it's there.

Signed-off-by: Michal Privoznik <>

2 hours agorpc: make daemon spawning a bit more intelligent
Martin Kletzander [Sun, 7 Sep 2014 18:41:11 +0000]
rpc: make daemon spawning a bit more intelligent

This way it behaves more like the daemon itself does (acquiring a
pidfile, deleting the socket before binding, etc.).


Signed-off-by: Martin Kletzander <>

3 hours agodomaincaps: Expose UEFI binary path, if it exists
Michal Privoznik [Tue, 16 Sep 2014 23:52:54 +0000]
domaincaps: Expose UEFI binary path, if it exists

Check to see if the UEFI binary mentioned in qemu.conf actually
exists, and if so expose it in domcapabilities like

<loader ...>

We introduce some generic domcaps infrastructure for handling
a dynamic list of string values, it may be of use for future bits.

Signed-off-by: Michal Privoznik <>

3 hours agoqemu_capabilities: Change virQEMUCapsFillDomainCaps signature
Michal Privoznik [Wed, 17 Sep 2014 09:33:35 +0000]
qemu_capabilities: Change virQEMUCapsFillDomainCaps signature

Up till now the virQEMUCapsFillDomainCaps() was type of void as
there was no way for it to fail. This is, however, going to
change in the next commit.

Signed-off-by: Michal Privoznik <>

3 hours agoqemu: add support for shared memory mapping
Martin Kletzander [Mon, 8 Sep 2014 09:36:09 +0000]
qemu: add support for shared memory mapping

Signed-off-by: Martin Kletzander <>

3 hours agodocs, conf, schema: add support for shared memory mapping
Martin Kletzander [Mon, 8 Sep 2014 09:34:22 +0000]
docs, conf, schema: add support for shared memory mapping

Signed-off-by: Martin Kletzander <>

3 hours agoschemas: finish virTristate{Bool,Switch} transition
Martin Kletzander [Mon, 8 Sep 2014 06:06:35 +0000]
schemas: finish virTristate{Bool,Switch} transition

Signed-off-by: Martin Kletzander <>

7 hours agoqemu: Add support for multiple versions of 'pseries' machine type
Pradipta Kr. Banerjee [Sat, 13 Sep 2014 15:28:58 +0000]
qemu: Add support for multiple versions of 'pseries' machine type

qemu for IBM Power processor architecture is adding functionality for
supporting multiple 'pseries' machine type versions, each with different
capabilities. This patch is for supporting the same

Signed-off-by: Pradipta Kr. Banerjee <>

9 hours agodomaincaps: Expose UEFI capability
Michal Privoznik [Tue, 16 Sep 2014 12:47:47 +0000]
domaincaps: Expose UEFI capability

As of 542899168c38 we learned libvirt to use UEFI for domains.
However, management applications may firstly query if libvirt
supports it. And this is where virConnectGetDomainCapabilities()
API comes handy.

Signed-off-by: Michal Privoznik <>

25 hours agoutil: storage: Copy driver type when initializing chain element
Peter Krempa [Tue, 16 Sep 2014 10:55:32 +0000]
util: storage: Copy driver type when initializing chain element

virStorageSourceInitChainElement initializes a new storage chain element
for use as a new disk source. If the new element doesn't contain the
driver name, copy it from the old source.

This fixes issue where a disk would forget the driver after a snapshot.


25 hours agoqemu: time: Report errors if agent command fails
Peter Krempa [Tue, 16 Sep 2014 13:37:08 +0000]
qemu: time: Report errors if agent command fails

Commit b606bbb4 broke reporting of errors when setting of guest time
fails via the guest agent as the return value is not checked and later
overwritten by the return value qemuMonitorRTCResetReinjection();

Fix this by checking the return value before resetting the RTC


27 hours agoWire up the interface backend options
Ján Tomko [Thu, 11 Sep 2014 15:15:24 +0000]
Wire up the interface backend options

Pass the user-specified tun path down when creating tap device
when called from the qemu driver.

Also honor the vhost device path specified by user.

27 hours agoconf: add backend element to interfaces
Ján Tomko [Thu, 11 Sep 2014 15:11:28 +0000]
conf: add backend element to interfaces

For tuning the network, alternative devices
for creating tap and vhost devices can be specified via:
<backend tap='/dev/net/tun' vhost='/dev/net-vhost'/>

29 hours agoconf: remove redundant local variable
Ján Tomko [Thu, 11 Sep 2014 10:54:20 +0000]
conf: remove redundant local variable

Use just one int variable for all the FromString calls.

29 hours agoconf: split out virtio net driver formatting
Ján Tomko [Wed, 10 Sep 2014 17:12:54 +0000]
conf: split out virtio net driver formatting

Instead of checking upfront if the <driver> element will be needed
in a big condition, just format all the attributes into a string
and output the <driver> element if the string is not empty.

31 hours agoqemu: Need to check for capability before query
John Ferlan [Tue, 16 Sep 2014 09:57:28 +0000]
qemu: Need to check for capability before query

Prior to trying the query-iothreads call - check if the qemu has
the capability

Signed-off-by: John Ferlan <>

32 hours agonetwork: check negative values in bridge queues
Erik Skultety [Tue, 16 Sep 2014 08:06:50 +0000]
network: check negative values in bridge queues

We already are checking for negative value, reporting an error, but
using wrong function and the check only succeeds when a value that
cannot be converted to number successfully is encountered. This patch
provides just a minor change in call of the right version
of function virStrToLong.


33 hours agoopenvz: fixed two memory leaks on migration code
Hongbin Lu [Tue, 16 Sep 2014 02:22:48 +0000]
openvz: fixed two memory leaks on migration code

The first one occurs in openvzDomainMigratePrepare3Params() where in
case no remote uri is given, the distant hostname is used. The name is
obtained via virGetHostname() which require callers to free the
returned value.
The second leak lies in openvzDomainMigratePerform3Params(). There's a
virCommand used later. However, at the beginning of the function
virCheckFlags() is called which returns. So the command created was

Signed-off-by: Michal Privoznik <>

33 hours agovirprocess: Extend list of platforms for setns wrapper
Michal Privoznik [Mon, 15 Sep 2014 13:31:40 +0000]
virprocess: Extend list of platforms for setns wrapper

Currently, the setns() wrapper is supported only for x86_64 and i686
which leaves us failing to build on other platforms like arm, aarch64
and so on. This means, that the wrapper needs to be extended to those
platforms and make to fail on runtime not compile time.

The syscall numbers for other platforms was fetched using this

kernel.git $ git grep "define.*__NR_setns" | grep -e arm -e powerpc -e s390
arch/arm/include/uapi/asm/unistd.h:#define __NR_setns                   (__NR_SYSCALL_BASE+375)
arch/arm64/include/asm/unistd32.h:#define __NR_setns 375
arch/powerpc/include/uapi/asm/unistd.h:#define __NR_setns               350
arch/s390/include/uapi/asm/unistd.h:#define __NR_setns          339

Signed-off-by: Michal Privoznik <>

34 hours agoutil: storage: Fix qcow(2) header parser according to docs
Peter Krempa [Mon, 15 Sep 2014 14:16:25 +0000]
util: storage: Fix qcow(2) header parser according to docs

The backing store string location offset 0 determines that the file
isn't present. The string size shouldn't be then checked:

from qemu.git/docs/specs/qcow2.txt

== Header ==

The first cluster of a qcow2 image contains the file header:

Byte  0 -  3:   magic
                QCOW magic string ("QFI\xfb")

      4 -  7:   version
                Version number (valid values are 2 and 3)

      8 - 15:   backing_file_offset
                Offset into the image file at which the backing file name
                is stored (NB: The string is not null terminated). 0 if the
                image doesn't have a backing file.

     16 - 19:   backing_file_size
                Length of the backing file name in bytes. Must not be
                longer than 1023 bytes. Undefined if the image doesn't have
                a backing file.         ^^^^^^^^^

This patch intentionally leaves the backing file string size check in
place in case a malformatted file would be presented to libvirt. Also
according to the docs the string size is maximum 1023 bytes, thus this
patch adds a check to verify that.

I was also able to verify that the check was done the same way in the
legacy qcow fromat (in qemu's code).

40 hours agoqemu: Fix call in qemuDomainSetNumaParamsLive for virCgroupNewIOThread
John Ferlan [Tue, 16 Sep 2014 00:01:18 +0000]
qemu: Fix call in qemuDomainSetNumaParamsLive for virCgroupNewIOThread

Found by inspection of the "i+1" change.  IOThreads are numbered 1..n
thus the virCgroupNewIOThread needs to create a 1..n value not 0 based.

40 hours agoqemu_cgroup: Adjust spacing around incrementor
John Ferlan [Tue, 16 Sep 2014 00:00:21 +0000]
qemu_cgroup: Adjust spacing around incrementor

Change "i+1" to "i + 1"

40 hours agoqemu: Fix iothreads issue
John Ferlan [Mon, 15 Sep 2014 23:57:22 +0000]
qemu: Fix iothreads issue

If there are no iothreads, then return from qemuProcessDetectIOThreadPIDs
without error; otherwise, the following occurs:

error: Failed to start domain $dom
error: An error occurred, but the cause is unknown

40 hours agocputune: allow interleaved xml
Eric Blake [Mon, 15 Sep 2014 23:33:16 +0000]
cputune: allow interleaved xml

I noticed this with the recent iothread pinning code, but the
problem existed longer than that. The XML validation required
users to supply <cputune> children in a strict order, even though
there was no conceptual reason why they can't occur in any order.

docs/ changes best viewed with -w

* docs/schemas/domaincommon.rng (cputune): Add interleave.
* tests/qemuxml2argvdata/qemuxml2argv-cputune-iothreads.xml: Swap
up order, copying canonical form...
* tests/qemuxml2xmloutdata/qemuxml2xmlout-cputune-iothreads.xml:
* tests/qemuxml2xmltest.c (mymain): Mark the difference.

Signed-off-by: Eric Blake <>

46 hours agovircgroup: Fix broken builds without cgroups
John Ferlan [Mon, 15 Sep 2014 18:45:23 +0000]
vircgroup: Fix broken builds without cgroups

I missed adding virCgroupNewIOThread to the !VIR_CGROUP_SUPPORTED

Pushing as build breaker

46 hours agonetwork: detect conflicting route even if it is the final entry
Laine Stump [Mon, 15 Sep 2014 17:30:08 +0000]
network: detect conflicting route even if it is the final entry

This is a folloup to commit 5f719596, which checks for a route
conflicting with the standard libvirt default network subnet
( It turns out that $() strips the trailing newline
from the output of "ip route show", so there would be no match if the
route we were looking for was the final line of output. This can be
solved by adding ${nl} to the end of the output (just as we were
already adding it at the beginning of the output).

2 days agodomain_conf: Add iothreadpin to cputune
John Ferlan [Wed, 3 Sep 2014 13:21:39 +0000]
domain_conf: Add iothreadpin to cputune

Add an option 'iothreadpin' to the <cpuset> to allow for setting the
CPU affinity for each IOThread.

The iothreadspin will mimic the vcpupin with respect to being able to
assign each iothread to a specific CPU, although iothreads ids start
at 1 while vcpu ids start at 0. This matches the iothread naming scheme.

2 days agoqemu: Allow pinning specific IOThreads to a CPU
John Ferlan [Wed, 3 Sep 2014 13:07:38 +0000]
qemu: Allow pinning specific IOThreads to a CPU

Modify qemuProcessStart() in order to allowing setting affinity to
specific CPU's for IOThreads. The process followed is similar to
that for the vCPU's.

This involves adding a function to fetch the IOThread id's via
qemuMonitorGetIOThreads() and adding them to iothreadpids[] list.
Then making sure all the cgroup data has been properly set up and
finally assigning affinity.

2 days agoqemu_cgroup: Introduce cgroup functions for IOThreads
John Ferlan [Wed, 3 Sep 2014 13:05:02 +0000]
qemu_cgroup: Introduce cgroup functions for IOThreads

In order to support cpuset setting, introduce qemuSetupCgroupIOThreadsPin
and qemuSetupCgroupForIOThreads to mimic the existing Vcpu API's.

These will support having an 'iotrhreadpin' element in the 'cpuset' in
order to pin named IOThreads to specific CPU's. The IOThread pin names
will follow the IOThread naming scheme starting at 1 (eg "iothread1")
up through an including the def->iothreads value.

2 days agoqemu_domain: Add niothreadpids and iothreadpids
John Ferlan [Wed, 3 Sep 2014 13:06:52 +0000]
qemu_domain: Add niothreadpids and iothreadpids

Add new 'niothreadpids' and 'iothreadpids' to mimic the 'ncpupids' and
'vcpupids' that already exist.

2 days agovircgroup: Introduce virCgroupNewIOThread
John Ferlan [Wed, 3 Sep 2014 13:04:07 +0000]
vircgroup: Introduce virCgroupNewIOThread

Add virCgroupNewIOThread() to mimic virCgroupNewVcpu() except the naming
scheme with use "iothread" rather than "vcpu".

2 days agoqemu: Issue query-iothreads and to get list of active IOThreads
John Ferlan [Fri, 29 Aug 2014 20:23:11 +0000]
qemu: Issue query-iothreads and to get list of active IOThreads

Generate infrastructure and test to handle fetching the QMP
IOThreads data.

2 days agovirsh: Add iothread to 'attach-disk'
John Ferlan [Tue, 2 Sep 2014 15:20:41 +0000]
virsh: Add iothread to 'attach-disk'

Add an iothread parameter to allow attaching to an IOThread, such as:

virsh attach-disk $dom $source $target --live --config --iothread 2 \
     --targetbus virtio --driver qemu --subdriver raw  --type disk

2 days agoutil: Fix copy-paste error in virXPathLongLong description
Martin Kletzander [Mon, 15 Sep 2014 15:26:50 +0000]
util: Fix copy-paste error in virXPathLongLong description

Signed-off-by: Erik Skultety <>
Signed-off-by: Martin Kletzander <>

2 days agonetwork: check for invalid forward delay time
Erik Skultety [Mon, 15 Sep 2014 08:42:15 +0000]
network: check for invalid forward delay time

When spanning tree protocol is allowed in bridge settings, forward delay
value is set as well (default is 0 if omitted). Until now, there was no
check for delay value validity. Delay makes sense only as a positive
numerical value.

Note: However, even if you provide positive  numerical value, brctl
utility only uses values from range <2,30>, so the number provided can
be modified (kernel most likely) to fall within this range.


2 days agoqemu: Fix build breaker on printf directive
John Ferlan [Mon, 15 Sep 2014 15:37:20 +0000]
qemu: Fix build breaker on printf directive

%zu for size_t not %lu

2 days agodaemon: Resolve Coverity FORWARD_NULL
John Ferlan [Fri, 12 Sep 2014 12:40:07 +0000]
daemon: Resolve Coverity FORWARD_NULL

Coverity complains that the comparison:

  if (nfds && nfds > ((int)!!sock_path + (int)!!sock_path_ro))

could mean 'sock_path' is NULL. Later in virNetSocketNewListenUNIX
there's a direct dereference of path in the error path:

  if (path[0] != '@')

A bit of sleuthing proves that upon entry to daemonSetupNetworking
there is no way for 'sock_path' to be NULL since daemonUnixSocketPaths
will set up 'sock_file' (although it may not set up 'sock_file_ro')
in all 3 paths.

Adjusted code to add ATTRIBUTE_NONNULL(3) on incoming path parameter and
then fixup the comparison of nfds to be a comparison against 2 or 1
depending on whether sock_path_ro is NULL or not.

2 days agoqemu: Resolve Coverity BAD_SIZEOF
John Ferlan [Fri, 12 Sep 2014 12:22:58 +0000]
qemu: Resolve Coverity BAD_SIZEOF

Coverity complains about the calculation of the buf & len within
the PROBE macro.  So to quiet things down, do the calculation prior
to usage in either write() or qemuMonitorIOWriteWithFD() calls and
then have the PROBE use the calculated values - which works.

2 days agoResolve Coverity CHECKED_RETURN
John Ferlan [Fri, 12 Sep 2014 12:16:07 +0000]
Resolve Coverity CHECKED_RETURN

Coverity complained that checking the return of virDomainCreate()
was not consistent amongst the callers - so added the return check
to the objecteventtest.c and adjust the virt-login-shell to compare
< 0 rather than just non zero for the failure condition.

2 days agovirsh: Resolve Coverity DEADCODE
John Ferlan [Fri, 12 Sep 2014 12:04:28 +0000]
virsh: Resolve Coverity DEADCODE

Coverity complains that on the first pass through the for loop that
'params' cannot be true, thus the ternary setting to "&" cannot be
done. Since we can only ever get to this point once, drop the ternary

2 days agodomain_conf: Resolve Coverity COPY_PASTE_ERROR
John Ferlan [Fri, 12 Sep 2014 11:52:39 +0000]
domain_conf: Resolve Coverity COPY_PASTE_ERROR

Seems when commit id 'ea130e3b' added the checks to ensure each of
the hard_limit, soft_limit, and swap_hard_limit wasn't set at
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED - a copy/paste error of using
the 'hard_limit' for each comparison was done. Adjust the code.

2 days agovirtime: Resolve Coverity DEADCODE
John Ferlan [Thu, 11 Sep 2014 21:29:18 +0000]
virtime: Resolve Coverity DEADCODE

Coverity complains that because of how 'offset' is initialized to
0 (zero), the resulting math and comparison on rem is pointless.

According to the origin commit id '3ec128989', the code is a
replacement for gmtime(), but without the localtime() or GMT
calculations - so just remove this code and add a comment
indicating the removal

2 days agoremote_driver: Resolve Coverity RESOURCE_LEAK
John Ferlan [Thu, 4 Sep 2014 12:49:32 +0000]
remote_driver: Resolve Coverity RESOURCE_LEAK

Since 98b9acf5aa02551dd37d0209339aba2e22e4004a

This was a false positive where Coverity was complaining that the
remoteDeserializeTypedParameters() could allocate 'params', but
none of the callers could return the allocated memory back to their
caller since on input the param was passed by value. Additionally,
the flow of the code was that if params was NULL on entry, then each
function would return 'nparams' as the number of params entries the
caller would need to allocate in order to call the function again
with 'nparams' and 'params' being set.  By the time the deserialize
routine was called params would have something.  For other callers
where the 'params' was passed by reference as NULL since it's expected
that the deserialize allocates the memory and then have that passed
back to the original caller to dispose there was no Coverity issue.

As it turns out Coverity didn't quite seem to understand the
relationship between 'nparams' and 'params'; however, if the
!userAllocated path of the deserialize code compared against
limit in any manner, then the Coverity error went away which
was quite strange, but useful.

As it turns out one code path remoteDomainGetJobStats had a
comparison against 'limit' while another remoteConnectGetAllDomainStats
did not assuming that limit would be checked.  So I refactored the
code a bit to cause the limit check to occur in deserialize for
both conditions and then only made the check of current returned
size against the incoming *nparams fail the non allocation case.
This means the job code doesn't need to check the limit any more,
while the stats code now does check the limit.

Additionally, to help perhaps decipher which of the various
callers to the deserialize code caused the failure - I used
a #define to pass the __FUNCNAME__ of the caller along so that
error messages could have something like:

error: remoteConnectGetAllDomainStats: too many parameters '2' for nparams '0'
error: Reconnected to the hypervisor

(it's a contrived error just to show the funcname in the error)

2 days agonode_device_udev: Try harder to get human readable vendor:product
Lubomir Rintel [Tue, 9 Sep 2014 12:20:43 +0000]
node_device_udev: Try harder to get human readable vendor:product

The manufacurer and product from USB device itself are usually not particularly
useful -- they tend to be missing, or ugly (all-uppercase, padded with spaces,
etc.). Prefer what's in the usb id database and fall back to descriptors only
if the device is too new to be in database.

2 days agoadd migration support for OpenVZ driver
Hongbin Lu [Fri, 5 Sep 2014 02:25:06 +0000]
add migration support for OpenVZ driver

This patch adds initial migration support to the OpenVZ driver,
using the VIR_DRV_FEATURE_MIGRATION_PARAMS family of migration

Signed-off-by: Michal Privoznik <>

2 days agoutil: get rid of unnecessary umask() call
Martin Kletzander [Sun, 7 Sep 2014 18:09:36 +0000]
util: get rid of unnecessary umask() call

Signed-off-by: Martin Kletzander <>

2 days agoutil: fix potential leak in error codepath
Martin Kletzander [Sun, 7 Sep 2014 18:07:49 +0000]
util: fix potential leak in error codepath

Signed-off-by: Martin Kletzander <>

2 days agoremove redundant pidfile path constructions
Martin Kletzander [Sun, 7 Sep 2014 17:52:34 +0000]
remove redundant pidfile path constructions

Signed-off-by: Martin Kletzander <>

2 days agorpc: reformat the flow to make a bit more sense
Martin Kletzander [Sun, 7 Sep 2014 15:08:57 +0000]
rpc: reformat the flow to make a bit more sense

Just remove useless "else".  Best viewed with '-w'.

Signed-off-by: Martin Kletzander <>

2 days agonetwork: try to eliminate default network conflict during package install
Laine Stump [Wed, 10 Sep 2014 17:10:45 +0000]
network: try to eliminate default network conflict during package install

Sometimes libvirt is installed on a host that is already using the
network If the libvirt-daemon-config-network package
is installed, this creates a conflict, since that package has been
hard-coded to create a virtual network that also uses In the past libvirt has attempted to warn of /
remediate this situation by checking for conflicting routes when the
network is started, but it turns out that isn't always useful (for
example in the case that the *other* interface/network creating the
conflict hasn't yet been started at the time libvirtd start its own

This patch attempts to catch the problem earlier - at install
time. During the %post install script for
libvirt-daemon-config-network, we use a case statement to look through
the output of "ip route show" for a route that exactly matches, and if found we search for a similar route that
*doesn't* match (e.g. (note that the search starts
with "124" instead of 123 because of reports of people already
modifying their L1 host's network to in an attempt to
solve exactly the problem we are also trying to solve).  When we find
an available route, we just replace all occurrences of "122" in the
default.xml that is being created with the newly found 192.168
subnet. This could obviously be made more complicated - examine the
template defaul.xml to automatically determine the existing network
address and mask rather than hard coding it in the specfile, etc, but
this scripting is simpler and gets the job done as long as we continue
to use in the template. (If anyone with mad bash
skillz wants to suggest something to do that, by all means please do).

This is intended to at least "further reduce" occurrence of the
problems detailed in:

5 days agoblockjob: allow finer bandwidth tuning for set speed
Eric Blake [Sun, 31 Aug 2014 03:56:19 +0000]
blockjob: allow finer bandwidth tuning for set speed

We stupidly modeled block job bandwidth after migration
bandwidth, which in turn was an 'unsigned long' and therefore
subject to 32-bit vs. 64-bit interpretations.  To work around
the fact that 10-gigabit interfaces are possible but don't fit
within 32 bits, the original interface took the number scaled
as MiB/sec.  But this scaling is rather coarse, and it might
be nice to tune bandwidth finer than in megabyte chunks.

Several of the block job calls that can set speed are fed
through a common interface, so it was easier to adjust them all
at once.  Note that there is intentionally no flag for the new
virDomainBlockCopy; there, since the API already uses a 64-bit
type always, instead of a possible 32-bit type, and is brand
new, it was easier to just avoid scaling issues.  As with the
previous patch that adjusted the query side (commit db33cc24),
omitting the new flag preserves old behavior, and the
documentation now mentions limits of what happens when a 32-bit
machine is on either client or server side.

* include/libvirt/ (virDomainBlockJobSetSpeedFlags)
* src/libvirt.c (virDomainBlockJobSetSpeed, virDomainBlockPull)
(virDomainBlockRebase, virDomainBlockCommit): Document them.
* src/qemu/qemu_driver.c (qemuDomainBlockJobSetSpeed)
(qemuDomainBlockPull, qemuDomainBlockRebase)
(qemuDomainBlockCommit, qemuDomainBlockJobImpl): Support new flag.

Signed-off-by: Eric Blake <>

5 days agoblockcopy: add qemu implementation of new tunables
Eric Blake [Mon, 8 Sep 2014 20:53:12 +0000]
blockcopy: add qemu implementation of new tunables

Upstream qemu 1.4 added some drive-mirror tunables not present
when it was first introduced in 1.3.  Management apps may want
to set these in some cases (for example, without tuning
granularity down to sector size, a copy may end up occupying
more bytes than the original because an entire cluster is
copied even when only a sector within the cluster is dirty,
although tuning it down results in more CPU time to do the
copy).  I haven't personally needed to use the parameters, but
since they exist, and since the new API supports virTypedParams,
we might as well expose them.

Since the tuning parameters aren't often used, and omitted from
the QMP command when unspecified, I think it is safe to rely on
qemu 1.3 to issue an error about them being unsupported, rather
than trying to create a new capability bit in libvirt.

Meanwhile, all versions of qemu from 1.4 to 2.1 have a bug where
a bad granularity (such as non-power-of-2) gives a poor message:
error: internal error: unable to execute QEMU command 'drive-mirror': Invalid parameter 'drive-virtio-disk0'

because of abuse of QERR_INVALID_PARAMETER (which is supposed to
name the parameter that was given a bad value, rather than the
value passed to some other parameter).  I don't see that a
capability check will help, so we'll just live with it (and it
has since been improved in upstream qemu).

* src/qemu/qemu_monitor.h (qemuMonitorDriveMirror): Add
* src/qemu/qemu_monitor.c (qemuMonitorDriveMirror): Likewise.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONDriveMirror):
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONDriveMirror):
* src/qemu/qemu_driver.c (qemuDomainBlockCopyCommon): Likewise.
(qemuDomainBlockRebase, qemuDomainBlockCopy): Adjust callers.
* src/qemu/qemu_migration.c (qemuMigrationDriveMirror): Likewise.
* tests/qemumonitorjsontest.c (qemuMonitorJSONDriveMirror): Likewise.

Signed-off-by: Eric Blake <>

5 days agoblockcopy: add qemu implementation of new API
Eric Blake [Mon, 25 Aug 2014 21:11:05 +0000]
blockcopy: add qemu implementation of new API

The hard part of managing the disk copy is already coded; all
this had to do was convert the XML and virTypedParameters into
the internal representation.

With this patch, all blockcopy operations that used the old
API should also work via the new API.  Additional extensions,
such as supporting the granularity tunable or a network rather
than file destination, will be added as later patches.

* src/qemu/qemu_driver.c (qemuDomainBlockCopy): New function.

Signed-off-by: Eric Blake <>

5 days agoblockcopy: tweak how rebase calls into copy
Eric Blake [Fri, 29 Aug 2014 22:30:46 +0000]
blockcopy: tweak how rebase calls into copy

In order to implement the new virDomainBlockCopy, the existing
block copy internal implementation needs to be adjusted.  The
new function will parse XML into a storage source, and parse
typed parameters into integers, then call into the same common
backend.  For now, it's easier to keep the same implementation
limits that only local file destinations are suported, but now
the check needs to be explicit.  Similar to qemuDomainBlockJobImpl
consuming 'vm', this code also consumes the caller's 'mirror'
description of the destination.

* src/qemu/qemu_driver.c (qemuDomainBlockCopy): Rename...
(qemuDomainBlockCopyCommon): ...and adjust parameters.
(qemuDomainBlockRebase): Adjust caller.

Signed-off-by: Eric Blake <>

5 days agoformatdomain: Update <loader/> example to match the rest
Michal Privoznik [Fri, 12 Sep 2014 11:18:32 +0000]
formatdomain: Update <loader/> example to match the rest

At the beginning when I was inventing <loader/> attributes and
<nvram/> I've introduced this @readonly attribute to the loader
element. It accepted values 'on' and 'off'. However, later, during the
review process, that has changed to 'yes' and 'no', but the example
XML snippet wasn't updated, so while the description is correct, the
example isn't.

Reported-by: Laszlo Ersek <>
Signed-off-by: Michal Privoznik <>

5 days agovirDomainUndefineFlags: Allow NVRAM unlinking
Michal Privoznik [Thu, 11 Sep 2014 11:17:11 +0000]
virDomainUndefineFlags: Allow NVRAM unlinking

When a domain is undefined, there are options to remove it's
managed save state or snapshots. However, there's another file
that libvirt creates per domain: the NVRAM variable store file.
Make sure that the file is not left behind if the domain is

Signed-off-by: Michal Privoznik <>

5 days agolibxl: Resolve Coverity CHECKED_RETURN
John Ferlan [Thu, 11 Sep 2014 21:52:58 +0000]
libxl: Resolve Coverity CHECKED_RETURN

Add a check of the return for virDomainHostdevInsert() like every
other call.

Signed-off-by: John Ferlan <>

5 days agoqemu: Resolve Coverity FORWARD_NULL
John Ferlan [Thu, 11 Sep 2014 21:45:04 +0000]
qemu: Resolve Coverity FORWARD_NULL

If we end up at the cleanup lable before we've VIR_EXPAND_N the list,
then calling virQEMUCapsFreeStringList() with a NULL proplist could
theoretically deref proplist if nproplist was set. Coverity doesn't
seem to acknowledge the relationship between proplist and nproplist
assuming in virQEMUCapsFreeStringList that nproplist could be at
least 1 and thus have a null deref.  It only seems to follow the
NULL proplist.

Signed-off-by: John Ferlan <>

5 days agovirfile: Resolve Coverity RESOURCE_LEAK
John Ferlan [Thu, 11 Sep 2014 21:05:34 +0000]
virfile: Resolve Coverity RESOURCE_LEAK

With the virGetGroupList() change in place - Coverity further complains
that if we fail to virFork(), the groups will be leaked - which aha seems
to be the case. Adjust the logic to save off the -errno, free the groups,
and then return the value we saved

Signed-off-by: John Ferlan <>

5 days agovirutil: Resolve Coverity RESOURCE_LEAK
John Ferlan [Thu, 11 Sep 2014 21:01:12 +0000]
virutil: Resolve Coverity RESOURCE_LEAK

This ends up being a very bizarre false positive. With an assist from
eblake, the claim is that mgetgroups() could return a -1 value, but yet
still have a groups buffer allocated, yet the example shown doesn't
seem to prove that.

Rather than fret about it, by adding a well placed sa_assert() on the
returned *list value we can "assure" ourselves that the mgetgroups()
failure path won't signal this condition.

Signed-off-by: John Ferlan <>

5 days agodaemon: Resolve Coverity RESOURCE_LEAK
John Ferlan [Thu, 11 Sep 2014 20:58:40 +0000]
daemon: Resolve Coverity RESOURCE_LEAK

With eblake's help - adjust the checks for stdinfd/stdoutfd to ensure the
values are within the range we expect; otherwise the dup2()'s and subsequent
VIR_CLOSE() calls cause Coverity to believe there's a resource leak.

Signed-off-by: John Ferlan <>

5 days agovirsh: Resolve Coverity NEGATIVE_RETURNS
John Ferlan [Thu, 4 Sep 2014 21:08:48 +0000]
virsh: Resolve Coverity NEGATIVE_RETURNS

Coverity notes that after we VIR_ALLOC_N(params, nparams) a failed call to
virDomainGetCPUStats could result in nparams being set to -1. In that case,
the subsequent virTypedParamsFree in cleanup will pass -1 which isn't good.

Use the returned value as the number of stats to display in the loop as
it will be the value reported from the hypervisor and may be less than
nparams which is OK

Signed-off-by: John Ferlan <>

5 days agovirsh: Move --completed from resume to domjobinfo
Jiri Denemark [Fri, 12 Sep 2014 08:05:32 +0000]
virsh: Move --completed from resume to domjobinfo

Because of similar contexts, git rebase I did just before pushing the
series which added --completed option patched the wrong command.

5 days agoconf: snapshot: Don't default-snapshot empty drives
Peter Krempa [Thu, 11 Sep 2014 15:45:06 +0000]
conf: snapshot: Don't default-snapshot empty drives

If a (floppy) drive isn't selected for snapshot explicitly and is empty
don't try to snapshot it. For external snapshots this would fail as we
can't generate a name for the snapshot from an empty drive.

Reported-by: Pavel Hrdina <>

5 days agoutil: Add function to check if a virStorageSource is "empty"
Peter Krempa [Thu, 11 Sep 2014 17:43:53 +0000]
util:  Add function to check if a virStorageSource is "empty"

To express empty drive we historically use storage source with empty
path. Unfortunately NBD disks may be declared without a path.

Add a helper to wrap this logic.

5 days agolibvirt.spec: Fix permission even for libvirt-driver-qemu
Michal Privoznik [Fri, 12 Sep 2014 07:01:46 +0000]
libvirt.spec: Fix permission even for libvirt-driver-qemu

In my previous patch (37d8c75fad) I've tried to fix permissions
for nvram store path. The aim was to give the nvram directory
execute permission so that domain running under other users
than qemu:qemu can access their nvram file. However, my fix
was incomplete as the path belongs to libvirt-driver-qemu
package too and I've fixed it only for the libvirt-daemon

Reported-by: Laszlo Ersek <>
Signed-off-by: Michal Privoznik <>

5 days agolibxl: fix mapping of libvirt and libxl lifecycle actions
Jim Fehlig [Wed, 3 Sep 2014 20:14:50 +0000]
libxl: fix mapping of libvirt and libxl lifecycle actions

The libxl driver was blindly assigning libvirt's
virDomainLifecycleAction to libxl's libxl_action_on_shutdown, when
in fact the various actions take on different values in these enums.

Introduce helpers to properly map the enum values.

Signed-off-by: Jim Fehlig <>

5 days agotests: Add more test suite mock helpers
Daniel P. Berrange [Tue, 3 Jun 2014 11:02:52 +0000]
tests: Add more test suite mock helpers

Rename the VIR_MOCK_IMPL* macros to VIR_MOCK_WRAP*
and add new VIR_MOCK_IMPL macros which let you directly
implement overrides in the preloaded source.

Signed-off-by: Daniel P. Berrange <>
Signed-off-by: Jim Fehlig <>

5 days agoutil: Allow port allocator to skip bind() check
Daniel P. Berrange [Tue, 3 Jun 2014 11:02:51 +0000]
util: Allow port allocator to skip bind() check

Test suites using the port allocator don't want to have different
behaviour depending on whether a port is in use on the host. Add
a VIR_PORT_ALLOCATOR_SKIP_BIND_CHECK which test suites can use
to skip the bind() test. The port allocator will thus only track
ports in use by the test suite process itself. This is fine when
using the port allocator to generate guest configs which won't
actually be launched

Signed-off-by: Daniel P. Berrange <>
Signed-off-by: Jim Fehlig <>

6 days agonvram: Fix permissions
Michal Privoznik [Thu, 11 Sep 2014 10:09:04 +0000]
nvram: Fix permissions

I've noticed two problem with the automatically created NVRAM varstore
file. The first, even though I run qemu as root:root for some reason I
get Permission denied when trying to open the _VARS.fd file. The
problem is, the upper directory misses execute permissions, which in
combination with us dropping some capabilities result in EPERM.

The next thing is, that if I switch SELinux to enforcing mode, I get
another EPERM because the vars file is not labeled correctly. It is
passed to qemu as disk and hence should be labelled as disk. QEMU may
write to it eventually, so this is different to kernel or initrd.

Signed-off-by: Michal Privoznik <>

6 days agoutil/virprocess.c: fix MinGW build
Pavel Hrdina [Thu, 11 Sep 2014 12:51:48 +0000]
util/virprocess.c: fix MinGW build

The build failed because of missing "sys/syscall.h".

Signed-off-by: Pavel Hrdina <>

6 days agolibxl: Resolve Coverity NULL_RETURNS
John Ferlan [Thu, 4 Sep 2014 21:21:37 +0000]
libxl: Resolve Coverity NULL_RETURNS

With all the changes in my previous foray into this code, I forgot to
remove the libxlDomainEventQueue(driver, event); call inside the
dom == NULL condition.

Signed-off-by: John Ferlan <>

6 days agoqemu: Resolve Coverity NEGATIVE_RETURNS
John Ferlan [Thu, 4 Sep 2014 21:18:48 +0000]
qemu: Resolve Coverity NEGATIVE_RETURNS

Coverity notes that if the virConnectListAllDomains returns a negative
value then the loop at the cleanup label that ends on numDomains will
have issues.

Signed-off-by: John Ferlan <>

6 days agoqemu: Resolve Coverity NEGATIVE_RETURNS
John Ferlan [Thu, 4 Sep 2014 21:15:51 +0000]
qemu: Resolve Coverity NEGATIVE_RETURNS

Coverity notes that if qemuMonitorGetMachines() returns a negative
nmachines value, then the code at the cleanup label will have issues.

Signed-off-by: John Ferlan <>

6 days agoxen: Resolve Coverity NEGATIVE_RETURNS
John Ferlan [Thu, 4 Sep 2014 21:11:57 +0000]
xen: Resolve Coverity NEGATIVE_RETURNS

Coverity notes that if the call to virBitmapParse() returns a negative
value, then when we jump to the error label, the call to
virCapabilitiesClearHostNUMACellCPUTopology() will have issues
with the negative nb_cpus

Signed-off-by: John Ferlan <>

6 days agonodeinfo: Resolve Coverity NEGATIVE_RETURNS
John Ferlan [Thu, 4 Sep 2014 21:00:30 +0000]
nodeinfo: Resolve Coverity NEGATIVE_RETURNS

If the virNumaGetNodeCPUs() call fails with -1, then jumping to cleanup
with 'cpus == NULL' and calling virCapabilitiesClearHostNUMACellCPUTopology
will cause issues.

Signed-off-by: John Ferlan <>

6 days agoqemu: Resolve Coverity NEGATIVE_RETURNS
John Ferlan [Thu, 4 Sep 2014 20:50:15 +0000]
qemu: Resolve Coverity NEGATIVE_RETURNS

In qemuProcessInitPCIAddresses() if qemuMonitorGetAllPCIAddresses()
returns a negative (or zero) value, then no need to call the

Signed-off-by: John Ferlan <>

6 days agonetwork_conf: Resolve Coverity FORWARD_NULL
John Ferlan [Thu, 4 Sep 2014 21:59:20 +0000]
network_conf: Resolve Coverity FORWARD_NULL

The code compares def->forwarders when deciding to return 0 at a
couple of points, then uses "def->nfwds" as a way to index into
the def->forwarders array.  That reference results in Coverity
complaining that def->forwarders being NULL was checked as part
of an arithmetic OR operation where failure could be any one 5
conditions, but that is not checked when entering the loop to
dereference the array.  Changing the comparisons to use nfwds
will clear the warnings

Signed-off-by: John Ferlan <>

6 days agoqemu: Resolve Coverity FORWARD_NULL
John Ferlan [Thu, 4 Sep 2014 20:40:34 +0000]
qemu: Resolve Coverity FORWARD_NULL

If the qemuMigrationEatCookie() fails to set mig, we jump to cleanup:
which will call qemuMigrationCancelDriveMirror() without first checking
if mig == NULL

Signed-off-by: John Ferlan <>

6 days agovirstring: Resolve Coverity FORWARD_NULL
John Ferlan [Thu, 4 Sep 2014 20:37:11 +0000]
virstring: Resolve Coverity FORWARD_NULL

Perhaps a false positive, but since Coverity doesn't understand the
relationship between the 'count' and the 'strings', rather than leave
the chance the on input 'strings' is NULL and causes a deref - just
check for it and return

Signed-off-by: John Ferlan <>

6 days agonetwork: Resolve Coverity FORWARD_NULL
John Ferlan [Thu, 4 Sep 2014 20:27:58 +0000]
network: Resolve Coverity FORWARD_NULL

If the VIR_STRDUP(exptime,...) fails, then we will jump to cleanup,
no need to check if exptime is set which causes Coverity to issue
a complaint in the virStrToLong_ll call because there wasn't a check
for a NULL value while there was one for the reference right after

Signed-off-by: John Ferlan <>

6 days agoqemu: Resolve Coverity FORWARD_NULL
John Ferlan [Thu, 4 Sep 2014 20:24:37 +0000]
qemu: Resolve Coverity FORWARD_NULL

If we jump to cleanup before allocating the 'result', then the call
to virBlkioDeviceArrayClear will deref result causing a problem.

Signed-off-by: John Ferlan <>

6 days agolxc: Resolve Coverity FORWARD_NULL
John Ferlan [Thu, 4 Sep 2014 20:22:07 +0000]
lxc: Resolve Coverity FORWARD_NULL

If we jump to cleanup before allocating 'result', then the call to
virBlkioDeviceArrayClear() could dereference result

Signed-off-by: John Ferlan <>

6 days agoqemu: Resolve Coverity FORWARD_NULL
John Ferlan [Thu, 4 Sep 2014 20:12:44 +0000]
qemu: Resolve Coverity FORWARD_NULL

If the virJSONValueNewObject() fails, then rather than going to error
and getting a Coverity false positive since it doesn't seem to understand
the relationship between nkeywords, keywords, and values and seems to
believe calling qemuFreeKeywords will cause a NULL deref - just return NULL

Signed-off-by: John Ferlan <>

6 days agovirsh: Resolve Coverity DEADCODE
John Ferlan [Thu, 4 Sep 2014 19:42:08 +0000]
virsh: Resolve Coverity DEADCODE

Coverity points out that if 'dom' isn't returned from virDomainQemuAttach,
then the code already jumps to cleanup, so there was no need for the
subsequent if (dom != NULL) check.

I moved the error message about failure into the goto cleanup on failure
and then removed the if (dom != NULL)

Signed-off-by: John Ferlan <>

6 days agotests: Resolve Coverity DEADCODE
John Ferlan [Thu, 4 Sep 2014 19:30:33 +0000]
tests: Resolve Coverity DEADCODE

Coverity complains that the various checks for autoincrement and changed
variables are DEADCODE - seems to me to be a false positive - so mark it.

Signed-off-by: John Ferlan <>

6 days agoqemu: Resolve Coverity DEADCODE
John Ferlan [Thu, 4 Sep 2014 19:19:47 +0000]
qemu: Resolve Coverity DEADCODE

Add another 'dead_code_begin' - victims of our own coding practices

Signed-off-by: John Ferlan <>