Jiri Denemark [Tue, 28 Jun 2016 12:39:58 +0000 (14:39 +0200)]
qemu: Let empty default VNC password work as documented
CVE-2016-5008
Setting an empty graphics password is documented as a way to disable
VNC/SPICE access, but QEMU does not always behaves like that. VNC would
happily accept the empty password. Let's enforce the behavior by setting
password expiration to "now".
Eric Blake [Wed, 9 Dec 2015 00:46:31 +0000 (17:46 -0700)]
CVE-2015-5313: storage: don't allow '/' in filesystem volume names
The libvirt file system storage driver determines what file to
act on by concatenating the pool location with the volume name.
If a user is able to pick names like "../../../etc/passwd", then
they can escape the bounds of the pool. For that matter,
virStoragePoolListVolumes() doesn't descend into subdirectories,
so a user really shouldn't use a name with a slash.
Normally, only privileged users can coerce libvirt into creating
or opening existing files using the virStorageVol APIs; and such
users already have full privilege to create any domain XML (so it
is not an escalation of privilege). But in the case of
fine-grained ACLs, it is feasible that a user can be granted
storage_vol:create but not domain:write, and it violates
assumptions if such a user can abuse libvirt to access files
outside of the storage pool.
Therefore, prevent all use of volume names that contain "/",
whether or not such a name is actually attempting to escape the
pool.
Since commit 8eb55d782a2b9afacc7938694891cc6fad7b42a5 libxml2 removes
two slashes from the URI when there is no server part. This is fixed
with beb7281055dbf0ed4d041022a67c6c5cfd126f25, but only if the calling
application calls xmlSaveUri() on URI that xmlURIParse() parsed. And
that is not the case in virURIFormat(). virURIFormat() accepts
virURIPtr that can be created without parsing it and we do that when we
format network storage paths for gluster for example. Even though
virStorageSourceParseBackingURI() uses virURIParse(), it throws that data
structure right away.
Since we want to format URIs as URIs and not absolute URIs or opaque
URIs (see RFC 3986), we can specify that with a special hack thanks to
commit beb7281055dbf0ed4d041022a67c6c5cfd126f25, by setting port to -1.
This fixes qemuxml2argvtest test where the disk-drive-network-gluster
case was failing.
In systemd >= 218, the udev_set_log_fn method has been marked
deprecated and turned into a no-op. Nothing in the udev client
library will print to stderr by default anymore, so we can
just stop installing a logging hook for new enough udev.
Well, in 8ad126e6 we tried to fix a memory corruption problem.
However, the fix was not as good as it could be. I mean, the
commit has one line more than it should. I've noticed this output
just recently:
# ./run valgrind --leak-check=full --show-reachable=yes ./tools/virsh domblklist gentoo
==17019== Memcheck, a memory error detector
==17019== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==17019== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==17019== Command: /home/zippy/work/libvirt/libvirt.git/tools/.libs/virsh domblklist gentoo
==17019==
Target Source
------------------------------------------------
fda /var/lib/libvirt/images/fd.img
vda /var/lib/libvirt/images/gentoo.qcow2
hdc /home/zippy/tmp/install-amd64-minimal-20150402.iso
==17019== Thread 2:
==17019== Invalid read of size 4
==17019== at 0x4EFF5B4: virObjectUnref (virobject.c:258)
==17019== by 0x5038CFF: remoteClientCloseFunc (remote_driver.c:552)
==17019== by 0x5069D57: virNetClientCloseLocked (virnetclient.c:685)
==17019== by 0x506C848: virNetClientIncomingEvent (virnetclient.c:1852)
==17019== by 0x5082136: virNetSocketEventHandle (virnetsocket.c:1913)
==17019== by 0x4ECD64E: virEventPollDispatchHandles (vireventpoll.c:509)
==17019== by 0x4ECDE02: virEventPollRunOnce (vireventpoll.c:658)
==17019== by 0x4ECBF00: virEventRunDefaultImpl (virevent.c:308)
==17019== by 0x130386: vshEventLoop (vsh.c:1864)
==17019== by 0x4F1EB07: virThreadHelper (virthread.c:206)
==17019== by 0xA8462D3: start_thread (in /lib64/libpthread-2.20.so)
==17019== by 0xAB441FC: clone (in /lib64/libc-2.20.so)
==17019== Address 0x139023f4 is 4 bytes inside a block of size 240 free'd
==17019== at 0x4C2B1F0: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==17019== by 0x4EA8949: virFree (viralloc.c:582)
==17019== by 0x4EFF6D0: virObjectUnref (virobject.c:273)
==17019== by 0x4FE74D6: virConnectClose (libvirt.c:1390)
==17019== by 0x13342A: virshDeinit (virsh.c:406)
==17019== by 0x134A37: main (virsh.c:950)
The problem is, when registering remoteClientCloseFunc(), it's
conn->closeCallback which is ref'd. But in the function itself
it's conn->closeCallback->conn what is unref'd. This is causing
imbalance in reference counting. Moreover, there's no need for
the remote driver to increase/decrease conn refcount since it's
not used anywhere. It's just merely passed to client registered
callback. And for that purpose it's correctly ref'd in
virConnectRegisterCloseCallback() and then unref'd in
virConnectUnregisterCloseCallback().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit e68930077034f786e219bdb015f8880dbc5a246f) Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Jim Fehlig [Mon, 10 Aug 2015 18:49:55 +0000 (12:49 -0600)]
Revert "LXC: show used memory as 0 when domain is not active"
This reverts commit 1ce7c1d20cfd5afb26d2dbc88201085d52415d0e,
which introduced a significant semantic change to the
virDomainGetInfo() API. Additionally, the change was only
made to 2 of the 15 virt drivers.
lxc: set nosuid+nodev+noexec flags on /proc/sys mount
Future kernels will mandate the use of nosuid+nodev+noexec
flags when mounting the /proc/sys filesystem. Unconditionally
add them now since they don't harm things regardless and could
mitigate future security attacks.
Eric Blake [Fri, 13 Mar 2015 23:01:43 +0000 (17:01 -0600)]
daemon: avoid memleak when ListAll returns nothing
Commit 4f25146 (v1.2.8) managed to silence Coverity, but at the
cost of a memory leak detected by valgrind:
==24129== 40 bytes in 5 blocks are definitely lost in loss record 355 of 637
==24129== at 0x4A08B1C: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==24129== by 0x5084B8E: virReallocN (viralloc.c:245)
==24129== by 0x514D5AA: virDomainObjListExport (domain_conf.c:22200)
==24129== by 0x201227DB: qemuConnectListAllDomains (qemu_driver.c:18042)
==24129== by 0x51CC1B6: virConnectListAllDomains (libvirt-domain.c:6797)
==24129== by 0x14173D: remoteDispatchConnectListAllDomains (remote.c:1580)
==24129== by 0x121BE1: remoteDispatchConnectListAllDomainsHelper (remote_dispatch.h:1072)
In short, every time a client calls a ListAll variant and asks
for the resulting list, but there are 0 elements to return, we
end up leaking the 1-entry array that holds the NULL terminator.
What's worse, a read-only client can access these functions in a
tight loop to cause libvirtd to eventually run out of memory; and
this can be considered a denial of service attack against more
privileged clients. Thankfully, the leak is so small (8 bytes per
call) that you would already have some other denial of service with
any guest calling the API that frequently, so an out-of-memory
crash is unlikely enough that this did not warrant a CVE.
Michal Privoznik [Fri, 30 Jan 2015 09:37:10 +0000 (10:37 +0100)]
xend: Don't crash in virDomainXMLDevID
The function is called from all {Attach,Update,Detach}Device APIs to
create config strings that are later passed to the xend to perform the
desired action. The function is intended to handle all supported
devices. However, as of 5b05358abacb1029fa0d61f72decacf0d4fd8ffb we
are trying to get disk driver of the device without checking if the
device really is a disk. This leads to an segmentation fault:
#0 0x00007ffff7571815 in virDomainDiskGetDriver () from /usr/lib/libvirt.so.0
#1 0x00007fffeb9ad471 in ?? () from /usr/lib/libvirt/connection-driver/libvirt_driver_xen.so
#2 0x00007fffeb9b1062 in xenDaemonAttachDeviceFlags () from /usr/lib/libvirt/connection-driver/libvirt_driver_xen.so
#3 0x00007fffeb9a8a86 in ?? () from /usr/lib/libvirt/connection-driver/libvirt_driver_xen.so
#4 0x00007ffff7609266 in virDomainAttachDevice () from /usr/lib/libvirt.so.0
#5 0x0000555555593c9d in ?? ()
#6 0x00007ffff76743c9 in virNetServerProgramDispatch () from /usr/lib/libvirt.so.0
#7 0x00005555555a678d in ?? ()
#8 0x00007ffff755460e in ?? () from /usr/lib/libvirt.so.0
#9 0x00007ffff7553b06 in ?? () from /usr/lib/libvirt.so.0
#10 0x00007ffff4998b50 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
#11 0x00007ffff46e30ed in clone () from /lib/x86_64-linux-gnu/libc.so.6
#12 0x0000000000000000 in ?? ()
Reported-by: Xiaolin Su <linxxnil@126.com> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit cd7702d4561bc100f291be7a1f6fa8f358440558)
Peter Krempa [Tue, 20 Jan 2015 16:01:01 +0000 (17:01 +0100)]
CVE-2015-0236: qemu: Check ACLs when dumping security info from snapshots
The ACL check didn't check the VIR_DOMAIN_XML_SECURE flag and the
appropriate permission for it. Found via code inspection while fixing
permissions for save images.
When trying to use an invalid offset to virStorageVolUpload(), libvirt
fails in virFDStreamOpenFileInternal(), although it seems libvirt does
not check the return in storageVolUpload(), and calls
virFDStreamSetInternalCloseCb() right after. But stream doesn't have a
privateData (is NULL) yet, and the daemon crashes then.
0 0x00007f09429a9c10 in pthread_mutex_lock () from /lib64/libpthread.so.0
1 0x00007f094514dbf5 in virMutexLock (m=<optimized out>) at util/virthread.c:88
2 0x00007f09451cb211 in virFDStreamSetInternalCloseCb at fdstream.c:795
3 0x00007f092ff2c9eb in storageVolUpload at storage/storage_driver.c:2098
4 0x00007f09451f46e0 in virStorageVolUpload at libvirt.c:14000
5 0x00007f0945c78fa1 in remoteDispatchStorageVolUpload at remote_dispatch.h:14339
6 remoteDispatchStorageVolUploadHelper at remote_dispatch.h:14309
7 0x00007f094524a192 in virNetServerProgramDispatchCall at rpc/virnetserverprogram.c:437
CVE-2014-8131: Fix possible deadlock and segfault in qemuConnectGetAllDomainStats()
When user doesn't have read access on one of the domains he requested,
the for loop could exit abruptly or continue and override pointer which
pointed to locked object.
This patch fixed two issues at once. One is that domflags might have
had QEMU_DOMAIN_STATS_HAVE_JOB even when there was no job started (this
is fixed by doing domflags |= QEMU_DOMAIN_STATS_HAVE_JOB only when the
job was acquired and cleaning domflags on every start of the loop.
Second one is that the domain is kept locked when
virConnectGetAllDomainStatsCheckACL() fails and continues the loop when
it didn't end. Adding a simple virObjectUnlock() and clearing the
pointer ought to do.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 57023c0a3af4af1c547189c1f6712ed5edeb0c0b) Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Conflicts:
src/qemu/qemu_driver.c -- GetAllDomainStats did not use jobs
before 1f4831ee
Laine Stump [Wed, 15 Oct 2014 22:49:01 +0000 (00:49 +0200)]
util: eliminate "use after free" in callers of virNetDevLinkDump
virNetDevLinkDump() gets a message from netlink into "resp", then
calls nlmsg_parse() to fill the table "tb" with pointers into resp. It
then returns tb to its caller, but not before freeing the buffer at
resp. That means that all the callers of virNetDevLinkDump() are
examining memory that has already been freed. This can be verified by
filling the buffer at resp with garbage prior to freeing it (or, I
suppose, just running libvirtd under valgrind) then performing some
operation that calls virNetDevLinkDump().
The upstream commit log incorrectly states that the code has been like
this ever since virNetDevLinkDump() was written. In reality, the
problem was introduced with commit e95de74d, first in libvirt-1.0.5,
which was attempting to eliminate a typecast that caused compiler
warnings. It has only been pure luck (or maybe a lack of heavy load,
and/or maybe an allocation algorithm in malloc() that delays re-use of
just-freed memory) that has kept this from causing errors, for example
when configuring a PCI passthrough or macvtap passthrough network
interface.
The solution taken in this patch is the simplest - just return resp to
the caller along with tb, then have the caller free it after they are
finished using the data (pointers) in tb. I alternately could have
made a cleaner interface by creating a new struct that put tb and resp
together along with a vir*Free() function for it, but this function is
only used in a couple places, and I'm not sure there will be
additional new uses of virNetDevLinkDump(), so the value of adding a
new type, extra APIs, etc. is dubious.
Eric Blake [Thu, 6 Nov 2014 08:42:24 +0000 (09:42 +0100)]
CVE-2014-7823: dumpxml: security hole with migratable flag
Commit 28f8dfd (v1.0.0) introduced a security hole: in at least
the qemu implementation of virDomainGetXMLDesc, the use of the
flag VIR_DOMAIN_XML_MIGRATABLE (which is usable from a read-only
connection) triggers the implicit use of VIR_DOMAIN_XML_SECURE
prior to calling qemuDomainFormatXML. However, the use of
VIR_DOMAIN_XML_SECURE is supposed to be restricted to read-write
clients only. This patch treats the migratable flag as requiring
the same permissions, rather than analyzing what might break if
migratable xml no longer includes secret information.
Fortunately, the information leak is low-risk: all that is gated
by the VIR_DOMAIN_XML_SECURE flag is the VNC connection password;
but VNC passwords are already weak (FIPS forbids their use, and
on a non-FIPS machine, anyone stupid enough to trust a max-8-byte
password sent in plaintext over the network deserves what they
get). SPICE offers better security than VNC, and all other
secrets are properly protected by use of virSecret associations
rather than direct output in domain XML.
* src/remote/remote_protocol.x (REMOTE_PROC_DOMAIN_GET_XML_DESC):
Tighten rules on use of migratable flag.
* src/libvirt-domain.c (virDomainGetXMLDesc): Likewise.
Pavel Hrdina [Mon, 22 Sep 2014 16:19:07 +0000 (18:19 +0200)]
domain_conf: fix domain deadlock
If you use public api virConnectListAllDomains() with second parameter
set to NULL to get only the number of domains you will lock out all
other operations with domains.
Laine Stump [Mon, 15 Sep 2014 17:30:08 +0000 (13:30 -0400)]
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
(192.168.122.0/24). 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).
Laine Stump [Wed, 10 Sep 2014 17:10:45 +0000 (13:10 -0400)]
network: try to eliminate default network conflict during package install
Sometimes libvirt is installed on a host that is already using the
network 192.168.122.0/24. 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
192.168.122.0/24. 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
networks).
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
192.168.122.0/24, and if found we search for a similar route that
*doesn't* match (e.g. 192.168.124.0/24) (note that the search starts
with "124" instead of 123 because of reports of people already
modifying their L1 host's network to 192.168.123.0/24 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 192.168.122.0/24 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:
Peter Krempa [Thu, 11 Sep 2014 14:35:53 +0000 (16:35 +0200)]
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.
Eric Blake [Sun, 31 Aug 2014 04:02:19 +0000 (22:02 -0600)]
blockcopy: allow larger buf-size
While qemu definitely caps granularity to 64 MiB, it places no
limits on buf-size. On a machine beefy enough for lots of
memory, a buf-size larger than 2 GiB is feasible, so we should
pass a 64-bit parameter.
The cleanup in commit cf976d9d used secdef->label to label the tap
FDs, but that is not possible since it's process-only label (svirt_t)
and not a object label (e.g. svirt_image_t). Starting a domain failed
with EPERM, but simply using secdef->imagelabel instead of
secdef->label fixes it.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Fix connection to already running session libvirtd
Since 1b807f92, connecting with virsh to an already running session
libvirtd fails with:
$ virsh list --all
error: failed to connect to the hypervisor
error: no valid connection
error: Failed to connect socket to
'/run/user/1000/libvirt/libvirt-sock': Transport endpoint is already
connected
This is caused by a logic error in virNetSocketNewConnectUnix: even if
the connection to the daemon socket succeeded, we still try to spawn the
daemon and then connect to it.
This commit changes the logic to not try to spawn libvirtd if we
successfully connected to its socket.
Most of this commit is whitespace changes, use of -w is recommended to
look at it.
Currently, after calling commands to create a new volumes,
virStorageBackendZFSCreateVol calls virStorageBackendZFSFindVols that
calls virStorageBackendZFSParseVol.
virStorageBackendZFSParseVol checks if a volume already exists by
trying to get it using virStorageVolDefFindByName.
For a just created volume it returns NULL, so volume is reported as
new and appended to pool->volumes. This causes a volume to be listed
twice as storageVolCreateXML appends this new volume to the list as
well.
Fix that by passing a new volume definition to
virStorageBackendZFSParseVol so it could determine if it needs to add
this volume to the list.
(3) Event ptr_arith:
Performing pointer arithmetic on "cur_fd" in expression "cur_fd++".
130 return virNetServerServiceNewFD(*cur_fd++,
The complaint is that pointer arithmetic taking place instead of the
expected auto increment of the variable... Adding some well placed
parentheses ensures our order of operation.
John Ferlan [Mon, 25 Aug 2014 19:59:32 +0000 (15:59 -0400)]
qemu: Allow use of iothreads for disk definitions
For virtio-blk-pci disks with the disk iothread attribute that are
running the correct emulator, add the "iothread=iothread#" to the
-device command line in order to enable iothreads for the disk as
long as the command is available, the disk iothread value provided is
valid, and is supported for the disk device being added
John Ferlan [Mon, 25 Aug 2014 12:43:17 +0000 (08:43 -0400)]
domain_conf: Add support for iothreads in disk definition
Add a new disk "driver" attribute "iothread" to be parsed as the thread
number for the disk to use. In order to more easily facilitate the usage
and configuration of the iothread, a "zero" for the attribute indicates
iothreads are not supported for the device and a positive value indicates
the specific thread to try and use.
John Ferlan [Fri, 22 Aug 2014 22:15:30 +0000 (18:15 -0400)]
qemu: Add support for iothreads
Add a new capability to ensure the iothreads feature exists for the qemu
emulator being run - requires the "query-iothreads" QMP command. Using the
domain XML add correspoding command argument in order to generate the
threads. The iothreads will use a name space "iothread#" where, the
future patch to add support for using an iothread to a disk definition to
merely define which of the available threads to use.
Add tests to ensure the xml/argv processing is correct. Note that no
change was made to qemuargv2xmltest.c as processing the -object element
would require knowing more than just iothreads.
John Ferlan [Fri, 22 Aug 2014 14:15:51 +0000 (10:15 -0400)]
domain_conf: Introduce iothreads XML
Introduce XML to allowing adding iothreads to the domain. These can be
used by virtio-blk-pci devices in order to assign a specific thread to
handle the workload for the device. The iothreads are the official
implementation of the virtio-blk Data Plane that's been in tech preview
for QEMU.
John Ferlan [Thu, 28 Aug 2014 18:56:33 +0000 (14:56 -0400)]
libxl_migration: Resolve Coverity NULL_RETURNS
Coverity noted that all callers to libxlDomainEventQueue() could ensure
the second parameter (event) was true before calling except this case.
As I look at the code and how events are used - it seems that prior to
generating an event for the dom == NULL condition, the resume/suspend
event should be queue'd after the virDomainSaveStatus() call which will
goto cleanup and queue the saved event anyway.
Peter Krempa [Mon, 25 Aug 2014 16:54:49 +0000 (18:54 +0200)]
qemu: Implement bulk stats API and one of the stats groups to return
Implement the API function for virDomainListGetStats and
virConnectGetAllDomainStats in a modular way and implement the
VIR_DOMAIN_STATS_STATE group of statistics.
Although it may look like the function looks universal I'd rather not
expose it to other drivers as the coming stats groups are likely to do
qemu specific stuff to obtain the stats.
John Ferlan [Wed, 27 Aug 2014 20:33:12 +0000 (16:33 -0400)]
qemu_command: Resolve Coverity DEADCODE
One useless warning, but the other one rather pertinent. On entry
the 'trans' variable is initialized to VIR_DOMAIN_DISK_TRANS_DEFAULT.
When the "trans" was found in the parsing loop it def->geometry.trans
was assigned to the return from virDomainDiskGeometryTransTypeFromString
and then 'trans' was used to do the comparison to see if it was valid.
So remove 'trans' and use def->geometry.trans properly
John Ferlan [Wed, 27 Aug 2014 19:22:33 +0000 (15:22 -0400)]
libxl_migration: Resolve Coverity RESOURCE_LEAK
In libxlDomainMigrationPrepare() if the uri_in is false, then
'hostname' is allocated and used "generically" in the routine,
but not freed. Conversely, if uri_in is true, then a uri is
allocated and hostname is set to the uri->hostname value and
likewise generically used.
At function exit, hostname wasn't free'd in the !uri_in path,
so that was added. To just make it clearer on usage the else
path became the call to virURIFree() although I suppose technically
it didn't have to since it would be a call using (NULL)
John Ferlan [Wed, 27 Aug 2014 18:57:08 +0000 (14:57 -0400)]
qemu_capabilities: Resolve Coverity RESOURCE_LEAK
Coverity determined that on error path that 'mach' wouldn't be free'd
Since virCapabilitiesFreeGuestMachine() isn't globally available, we'll
insert first and then if the VIR_STRDUP's fail they it will eventually
cause the 'mach' to be freed in the error path
John Ferlan [Wed, 27 Aug 2014 18:42:41 +0000 (14:42 -0400)]
qemu_agent: Resolve Coverity RESOURCE_LEAK
Coverity found that on error paths, the 'arg' value wasn't be cleaned
up. Followed the example in qemuAgentSetVCPUs() where upon successful call
to qemuAgentCommand() the 'cpus' is set to NULL; otherwise, when cleanup
occurs the free the memory for 'arg'
John Ferlan [Wed, 27 Aug 2014 17:38:29 +0000 (13:38 -0400)]
daemon: Resolve Coverity NEGATIVE_RETURNS
In each of these cases, Coverity complains that the result count returned
on error paths would be -1 disregarding that the count and the corresponding
are "linked" together (it doesn't know that). Simple enough to check and
remove the warning
Peter Krempa [Tue, 26 Aug 2014 13:55:14 +0000 (15:55 +0200)]
virsh: Implement command to excercise the bulk stats APIs
Add "domstats" command that excercises both of the new APIs depending if
you specify a domain list or not. The output is printed as a key=value
list of the returned parameters.
Wang Rui [Thu, 28 Aug 2014 10:20:58 +0000 (18:20 +0800)]
qemu_capabilities: Resolve Coverity RESOURCE_LEAK
In function virQEMUCapsParseMachineTypesStr, VIR_STRNDUP allocates
memory for 'name' in {do,while} loop. If 'name' isn't freed before
'continue', its memory will be allocated again in the next loop.
In this case the memory allocated for 'name' in privious loop is
useless and not freed. Free it before continue this loop to fix that.
John Ferlan [Wed, 27 Aug 2014 12:51:15 +0000 (08:51 -0400)]
domain_conf: Resolve Coverity REVERSE_INULL
Coverity complains that checking for domain->def being non NULL in the
if (live) path of virDomainObjAssignDef() would be unnecessary or a
NULL deref since the call to virDomainObjIsActive() would already
dereference domain->def when checking if the def->id field was != -1.
Checked all callers to virDomainObjAssignDef() and each at some point
dereferences (vm)->def->{field} prior to calling when live is true.
John Ferlan [Wed, 27 Aug 2014 12:35:08 +0000 (08:35 -0400)]
qemu_command: Resolve Coverity REVERSE_INULL
In qemuNetworkIfaceConnect() a call to virNetDevBandwidthSet() is
made where the function prototype requires the first parameter
(net->ifname) to be non NULL. Coverity complains that the subsequent
non NULL check for net->ifname prior to the next call gets flagged as
an unnecessary check. Resolve by removing the extra check
John Ferlan [Wed, 27 Aug 2014 12:01:44 +0000 (08:01 -0400)]
domain_conf: Resolve Coverity REVERSE_INULL
In virDomainActualNetDefFormat() a call to virDomainNetGetActualType(def)
was made before a check for (!def) a few lines later. This triggered
Coverity to note the possible NULL deref. Just moving the initialization
to after the !def checks resolves the issue
John Ferlan [Wed, 27 Aug 2014 11:56:00 +0000 (07:56 -0400)]
storage_driver: Resolve Coverity REVERSE_INULL
There were two occurrances of attempting to initialize actualType by
calling virStorageSourceGetActualType(src) prior to a check if (!src)
resulting in Coverity complaining about the possible NULL dereference
in virStorageSourceGetActualType() of src.
Resolve by moving the actualType setting until after checking !src
John Ferlan [Wed, 27 Aug 2014 11:48:37 +0000 (07:48 -0400)]
xen_xm: Resolve Coverity USE_AFTER_FREE
If virDomainDiskDefFree(disk) is called in 'skipdisk:', then it's possible
to either return to skipdisk without reallocating a new disk (via the if
condition just prior) or to end the loop having deleted the disk. Since
virDomainDiskDefFree() does not pass by reference, disk isn't changed in
this context, thus the possible issue.
John Ferlan [Wed, 27 Aug 2014 11:40:02 +0000 (07:40 -0400)]
xen_common: Resolve Coverity USE_AFTER_FREE
There were two warnings in this module
If the VIR_ALLOC_N(def->serials, 1) fails, then a virDomainChrDefFree(chr)
is called and we jump to cleanup which makes the same call. Just remove
the one after VIR_ALLOC_N()
In the label "skipnic:" a virDomainNetDefFree(net) is made; however, if
in going back to the top of the loop we jump back down to skipnic for any
reason, the call will attempt to free an already freed structure since
"net" was not passed by reference to virDomainNetDefFree(). Just set
net = NULL in skipnic: to resolve the issue.
John Ferlan [Wed, 27 Aug 2014 11:32:11 +0000 (07:32 -0400)]
parallels: Resolve Coverity USE_AFTER_FREE
Coverity complains that calling virNetworkDefFree(def), then jumping
to the cleanup: label which calls virNetworkDefFree(def) could result
in a double_free. Just remove the call from the if statement.
Michal Privoznik [Fri, 22 Aug 2014 09:37:52 +0000 (11:37 +0200)]
vbox: Register per partes
Since times when vbox moved to the daemon (due to some licensing
issue) the subdrivers that vbox implements were registered, but not
opened since our generic subdrivers took priority. I've tried to fix
this in 65b7d553f39ff9 but it was not correct. Apparently moving
vbox driver registration upfront changes the default connection URI
which makes some users sad. So, this commit breaks vbox into pieces
and register vbox's network and storage drivers first, and vbox driver
then at the end. This way, the vbox driver is registered in the order
it always was, but its subdrivers are registered prior the generic
ones.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Michal Privoznik [Fri, 22 Aug 2014 09:37:51 +0000 (11:37 +0200)]
virDriverLoadModule: Honor libvirt func name tranlsation
There's this unwritten rule in libvirt that vir_function is translated
into virFunction when needed (e.g. in remote protocol definition,
python, ...). Up till now we ignored such translation in driver module
loading and did fine. Well, we didn't have any module with an
underscore in its name. But this will change in next commit. The
problem is, once an a module is dlopen()-ed, we derive register
function name from its name. So instead of "driver_subdriverRegister"
do some magic to turn that into "driverSubdriverRegister".
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Fri, 22 Aug 2014 09:37:50 +0000 (11:37 +0200)]
virdrivermoduletest: Test all the modules
Even though we kept adding new and new modules (e.g. vbox or bhyve)
the test wasn't updated. Do that now. Moreover, while it's not
crucial, it's nice to reorder test cases to match the order in which
the daemon loads the modules.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Eric Blake [Mon, 25 Aug 2014 18:58:49 +0000 (12:58 -0600)]
domain_conf: fix internal flag verification
While working on virDomainBlockCopy, I noticed we had a verify()
concerning internal XML flags that was incomplete after several
recent flag additions; move that up higher in the code to make it
harder to forget to modify on the next flag addition. Adjust
some formatting while at it.
* src/conf/domain_conf.c (verify): Move closer to internal flag
definitions. Cover missing flags ALLOW_ROM and ALLOW_BOOT.
Jincheng Miao [Wed, 27 Aug 2014 02:10:33 +0000 (10:10 +0800)]
qemu: call endjob in RevertToSnapshot
In qemuDomainRevertToSnapshot(), it will check snap->def->state.
But when the state is PMSUSPENDED/NOSTATE/BLOCKED, it forgets to
call qemuDomainObjEndJob.
https://bugzilla.redhat.com/show_bug.cgi?id=1134154
Bug introduced in commit 1e833899.
Signed-off-by: Jincheng Miao <jmiao@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Eric Blake [Tue, 26 Aug 2014 22:04:37 +0000 (16:04 -0600)]
API: Tweak virDomainOpenGraphics to return fd directly
Let's fix this before we bake in a painful API. Since we know
that we have exactly one non-negative fd on success, we might
as well return the fd directly instead of forcing the user to
pass in a pointer. Furthermore, I found some memory and fd
leaks while reviewing the code - the idea is that on success,
libvirtd will have handed two fds in two different directions:
one to qemu, and one to the RPC client.
* include/libvirt/libvirt.h.in (virDomainOpenGraphicsFD): Drop
unneeded parameter.
* src/driver.h (virDrvDomainOpenGraphicsFD): Likewise.
* src/libvirt.c (virDomainOpenGraphicsFD): Adjust interface to
return fd directly.
* daemon/remote.c (remoteDispatchDomainOpenGraphicsFd): Adjust
semantics.
* src/qemu/qemu_driver.c (qemuDomainOpenGraphicsFD): Likewise,
and plug fd leak.
* src/remote/remote_driver.c (remoteDomainOpenGraphicsFD):
Likewise, and plug memory and fd leak.
Eric Blake [Tue, 26 Aug 2014 21:16:48 +0000 (15:16 -0600)]
blockcopy: virDomainBlockCopy with XML destination, typed params
This commit (finally) adds the virDomainBlockCopy API, with the
intent that it will provide more power to the existing 'virsh
blockcopy' command.
'virsh blockcopy' was first added in Apr 2012 (v0.9.12), which
corresponds to the upstream qemu 1.2 timeframe. It was done as
a hack on top of the existing virDomainBlockRebase() API call,
for two reasons: 1) it was targetting a feature that landed first
in downstream RHEL qemu, but had not stabilized in upstream qemu
at the time (and indeed, 'drive-mirror' only landed upstream in
qemu 1.3 with slight differences to the first RHEL attempt,
and later gained further parameters like granularity and buf-size
that are also worth exposing), and 2) extending an existing API
allowed it to be backported without worrying about bumping .so
versions. A virDomainBlockCopy() API was proposed at that time
[1], but we decided not to accept it into libvirt until after
upstream qemu stabilized, and it ended up getting scrapped.
Whether or not RHEL should have attempted adding a new feature
without getting it upstream first is a debate that can be held
another day; but enough time has now elapsed that we are ready to
do the interface cleanly.
Delaying the creation of a clean API until now has also had a
benefit: we've only recently learned of a few shortcomings in the
original design: 1) it is unable to target a network destination
(such as a gluster volume) because it hard-coded the assumption
that the destination is a local file name. Because of all the
refactoring we've done to add virStorageSourcePtr, we are in a
better position to declare an API that parses XML describing a
host storage source as the copy destination, which was not
possible had we implemented virDomainBlockCopy as it had been
originally envisioned (although a network target will have to wait
until a later libvirt release compared to the API addition to
actually be implemented). 2) the design of using MiB/sec as the
bandwidth throttle is rather coarse; qemu is actually tuned to
bytes/second, and libvirt is preventing access to that level of
detail. A later patch will add flags to existing block job API
that can request bytes/second instead of back-compat MiB/s, but as
this is a new API, we can get it right to begin with.
At least I had the foresight to create 'virsh blockcopy' as a
separate command at the UI level (commit 1f06c00) rather than
leaking the underlying API overload of virDomainBlockRebase onto
shell users.
A further note on the bandwidth option: virTypedParameters
intentionally lacks unsigned long (since variable-width
interaction between mixed 32- vs. 64-bit client/server setups is
nasty), but we have to deal with the fact that we are interacting
with existing older code that mistakenly chose unsigned long
bandwidth at a point before we decided to prohibit it in all new
API. The typed parameter is therefore unsigned long long, but
the implementation (in a later patch) will have to do overflow
detection on 32-bit platforms, as well as capping the value to
match the LLONG_MAX>>20 cap of the existing MiB/s interfaces.
* include/libvirt/libvirt.h.in (virDomainBlockCopy): New API.
(virDomainBlockJobType, virConnectDomainEventBlockJobStatus):
Update related documentation.
* src/libvirt.c (virDomainBlockCopy): Implement it.
* src/libvirt_public.syms (LIBVIRT_1.2.8): Export it.
* src/driver.h (_virDriver): New driver callback.
Peter Krempa [Tue, 26 Aug 2014 19:11:42 +0000 (21:11 +0200)]
lib: Introduce API for retrieving bulk domain stats
The motivation for this API is that management layers that use libvirt
usually poll for statistics using various split up APIs we currently
provide. To get all the necessary stuff, the app needs to issue a lot of
calls and aggregate the results.
The APIs I'm introducing here:
1) Returns data in a format that we can expand in the future and is
(pseudo) hierarchical. The data is returned as typed parameters where
the fields are constructed as dot-separated strings containing names and
other stuff in a list of typed params.
2) Stats for multiple (all) domains can be queried at once and are
returned in one call. This will decrease the overhead necessary to issue
multiple calls per domain multiplied by the count of domains.
3) Selectable (bit mask) fields in the returned format. This will allow
to retrieve only specific stats according to the app's need.
The stats groups will be enabled using a bit field @stats passed as the
function argument. A few sample stats groups that this API will support:
(Note that this is only an example, the initial implementation supports
only VIR_DOMAIN_STATS_STATE while others will be added later.)
the returned typed params will use the following scheme
state.state = VIR_DOMAIN_RUNNING
state.reason = VIR_DOMAIN_RUNNING_BOOTED (the actual values according to
the enum)
cpu.count = 8
cpu.0.state = running
cpu.0.time = 1234
Erik Skultety [Tue, 26 Aug 2014 11:29:10 +0000 (13:29 +0200)]
blkdeviotune: check for overflow when parsing XML
According to docs/schemas/domaincommon.rng and _virDomainBlockIoTuneInfo
all the iotune values are interpreted as unsigned long long, however
according to qemu_monitor_json.c, qemu silently truncates numbers
larger than LLONG_MAX. There's really not much of a usage for such
large numbers anyway yet. This patch provides the same overflow
check during a domain start as it does during setting
a blkdeviotune element in qemu_driver.c and thus reports an error when
a larger number than LLONG_MAX is detected.
Alex Williamson [Thu, 21 Aug 2014 17:04:45 +0000 (11:04 -0600)]
Add new 'kvm' domain feature and ability to hide KVM signature
QEMU 2.1 added support for the kvm=off option to the -cpu command,
allowing the KVM hypervisor signature to be hidden from the guest.
This enables disabling of some paravirualization features in the
guest as well as allowing certain drivers which test for the
hypervisor to load. Domain XML syntax is as follows:
Jim Fehlig [Mon, 25 Aug 2014 22:46:00 +0000 (16:46 -0600)]
libxl: fix memory corruption introduced by commit b55cc5f4e
Commit b55cc5f4e did a shallow copy of libxl_{sdl,vnc}_info from the
domain config to the build info, which resulted in double-freeing
strings contained in the structures during cleanup, which later
resulted in a libvirtd crash. Fix by performing a deep copy of the
structure, VIR_STRDUP'ing embedded strings instead of simply copying
their pointers.
Fixes the following issue reported on the libvirt dev list
Eric Blake [Mon, 25 Aug 2014 22:26:56 +0000 (16:26 -0600)]
maint: drop spurious semicolons
I noticed a line 'int nparams = 0;;' in remote_dispatch.h, and
tracked down where it was generated. While at it, I found a
couple of other double semicolons. Additionally, I noticed that
commit df0b57a95 left a stale reference to the file name
remote_dispatch_bodies.h.
* src/conf/numatune_conf.c (virDomainNumatuneNodeParseXML): Drop
empty statement.
* tests/virdbustest.c (testMessageStruct, testMessageSimple):
Likewise.
* src/rpc/gendispatch.pl (remote_dispatch_bodies.h): Likewise, and
update stale comments.
Eric Blake [Tue, 15 Jul 2014 22:36:00 +0000 (16:36 -0600)]
spec: drop anything older than Fedora 13
RHEL 5 is based on libvirt 0.8.2, as was Fedora 13. RHEL 5 also
happens to be the oldest box that we actively support with a
buildbot, so it is time to clean up some crufty conditionals in
the spec file that no longer are necessary for modern Fedora.
Although it is probably okay to make further simplifications to
a newer minimum Fedora version, that can be done as a later patch.
This patch just focuses on cleaning any comparison of %{?fedora}
that will always be true or false once we assume a minimum of F13.
* libvirt.spec.in: Make with_audit default to on. Move other
conditionals to a single RHEL-5 block. Simplify any fedora
comparison older than 13. Document our assumptions.
When start libvirtd with commandline "/usr/sbin/libvirtd -d -l -v",
we expect verbose(info level) log if neither environment variable
nor config file about logging controls is set. But in fact we can't
get any info priority log in the default output file.
The log priority of default output is VIR_LOG_DEFAULT(VIR_LOG_WARN),
so the info log is filtered out.
To record info priority log we must parse option -v before setting the
default output.
After this patch, we get all verbose log in the default output file.
Erik Skultety [Mon, 25 Aug 2014 08:50:12 +0000 (10:50 +0200)]
iotune: setting an invalid value now reports error
When trying to set an invalid value into iotune element, standard
behavior was to not report any error, rather to reset all affected
subelements of the iotune element back to 0 which results in ignoring
those particular subelements by XML generator. Patch further
examines the return code of the virXPathULongLong function
and in case of an invalid non-integer value raises an error.
Fixed to preserve consistency with invalid value checking
of other elements.
projects / thirdparty / libvirt.git / log
