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.
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.
Peter Krempa [Tue, 1 Jul 2014 11:52:51 +0000 (13:52 +0200)]
qemu: copy: Accept 'format' parameter when copying to a non-existing img
We have the following matrix of possible arguments handled by the logic
statement touched by this patch:
| flags & _REUSE_EXT | !(flags & _REUSE_EXT)
-------+--------------------+----------------------
format| (1) | (2)
-------+--------------------+----------------------
!format| (3) | (4)
-------+--------------------+----------------------
In cases 1 and 2 the user provided a format, in cases 3 and 4 not. The
user requests to use a pre-existing image in 1 and 3 and libvirt will
create a new image in 2 and 4.
The difference between cases 3 and 4 is that for 3 the format is probed
from the user-provided image, whereas in 4 we just use the existing disk
format.
The current code would treat cases 1,3 and 4 correctly but in case 2 the
format provided by the user would be ignored.
The particular piece of code was broken in commit 35c7701c64508f975dfeb8
but since it was introduced a few commits before that it was never
released as working.
Eric Blake [Wed, 25 Jun 2014 20:54:36 +0000 (14:54 -0600)]
docs: publish correct enum values
We publish libvirt-api.xml for others to use, and in fact, the
libvirt-python bindings use it to generate python constants that
correspond to our enum values. However, we had an off-by-one bug
that any enum that relied on C's rules for implicit initialization
of the first enum member to 0 got listed in the xml as having a
value of 1 (and all later members of the enum were equally
botched).
The fix is simple - since we add one to the previous value when
encountering an enum without an initializer, the previous value
must start at -1 so that the first enum member is assigned 0.
The python generator code has had the off-by-one ever since DV
first wrote it years ago, but most of our public enums were immune
because they had an explicit = 0 initializer. The only affected
enums are:
- virDomainEventGraphicsAddressType (such as
VIR_DOMAIN_EVENT_GRAPHICS_ADDRESS_IPV4), since commit 987e31e
(libvirt v0.8.0)
- virDomainCoreDumpFormat (such as VIR_DOMAIN_CORE_DUMP_FORMAT_RAW),
since commit 9fbaff0 (libvirt v1.2.3)
- virIPAddrType (such as VIR_IP_ADDR_TYPE_IPV4), since commit 03e0e79 (not yet released)
Thanks to Nehal J Wani for reporting the problem on IRC, and
for helping me zero in on the culprit function.
Peter Krempa [Wed, 25 Jun 2014 16:11:17 +0000 (18:11 +0200)]
qemu: blockcopy: Don't remove existing disk mirror info
When creating a new disk mirror the new struct is stored in a separate
variable until everything went well. The removed hunk would actually
remove existing mirror information for example when the api would be run
if a mirror still exists.
LSN-2014-0003: Don't expand entities when parsing XML
If the XML_PARSE_NOENT flag is passed to libxml2, then any
entities in the input document will be fully expanded. This
allows the user to read arbitrary files on the host machine
by creating an entity pointing to a local file. Removing
the XML_PARSE_NOENT flag means that any entities are left
unchanged by the parser, or expanded to "" by the XPath
APIs.
Laine Stump [Thu, 1 May 2014 08:40:41 +0000 (11:40 +0300)]
qemu: fix crash when removing <filterref> from interface with update-device
If a domain network interface that contains a <filterref> is modified
"live" using "virsh update-device --live", libvirtd would crash. This
was because the code supporting live update of an interface's
filterref was assuming that a filterref might be added or modified,
but didn't account for removing the filterref, resulting in a null
dereference of the filter name.
Introduced with commit 258fb278, which was first in libvirt v1.0.1.
This addresses https://bugzilla.redhat.com/show_bug.cgi?id=1093301
qemu: make sure agent returns error when required data are missing
Commit 5b3492fa aimed to fix this and caught one error but exposed
another one. When agent command is being executed and the thread
waiting for the reply is woken up by an event (e.g. EOF in case of
shutdown), the command finishes with no data (rxObject == NULL), but
no error is reported, since this might be desired by the caller
(e.g. suspend through agent). However, in other situations, when the
data are required (e.g. getting vCPUs), we proceed to getting desired
data out of the reply, but none of the virJSON*() functions works well
with NULLs. I chose the way of a new parameter for qemuAgentCommand()
function that specifies whether reply is required and behaves
according to that.
On all the places where qemuAgentComand() was called, we did a check
for errors in the reply. Unfortunately, some of the places called
qemuAgentCheckError() without checking for non-null reply which might
have resulted in a crash.
So this patch makes the error-checking part of qemuAgentCommand()
itself, which:
a) makes it look better,
b) makes the check mandatory and, most importantly,
c) checks for the errors if and only if it is appropriate.
This actually fixes a potential crashers when qemuAgentComand()
returned 0, but reply was NULL. Having said that, it *should* fix the
following bug:
LXC: Fix return code evaulation in lxcCheckNetNsSupport()
Commit b9dd878f (util: make it easier to grab only regular command exit)
changed the call semantics of virCommandRun() and therefore of virRun()
too. But lxcCheckNetNsSupport() was not updated.
As consequence of this lxcCheckNetNsSupport always failed and broke LXC.
Signed-off-by: Richard Weinberger <richard@nod.at>
Peter Krempa [Mon, 24 Feb 2014 15:12:34 +0000 (16:12 +0100)]
gluster: Fix "key" attribute for gluster volumes
According to our documentation the "key" value has the following
meaning: "Providing an identifier for the volume which identifies a
single volume." The currently used keys for gluster volumes consist of
the gluster volume name and file path. This can't be considered unique
as a different storage server can serve a volume with the same name.
Unfortunately I wasn't able to figure out a way to retrieve the gluster
volume UUID which would avoid the possibility of having two distinct
keys identifying a single volume.
Use the full URI as the key for the volume to avoid the more critical
ambiguity problem and document the possible change to UUID.
While running virdbustest, it was found that valgrind pointed out
the following memory leaks:
==9996== 17 (8 direct, 9 indirect) bytes in 1 blocks are definitely lost in loss record 9 of 36
==9996== at 0x4A069EE: malloc (vg_replace_malloc.c:270)
==9996== by 0x4A06B62: realloc (vg_replace_malloc.c:662)
==9996== by 0x4C6B587: virReallocN (viralloc.c:245)
==9996== by 0x4C6B6AE: virExpandN (viralloc.c:294)
==9996== by 0x4C82B54: virDBusMessageDecodeArgs (virdbus.c:907)
==9996== by 0x4C83463: virDBusMessageDecode (virdbus.c:1141)
==9996== by 0x402C45: testMessageArrayRef (virdbustest.c:273)
==9996== by 0x404E71: virtTestRun (testutils.c:201)
==9996== by 0x401C2D: mymain (virdbustest.c:479)
==9996== by 0x4055ED: virtTestMain (testutils.c:789)
==9996== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==9996==
==9996== 28 (16 direct, 12 indirect) bytes in 1 blocks are definitely lost in loss record 12 of 36
==9996== at 0x4A06BE0: realloc (vg_replace_malloc.c:662)
==9996== by 0x4C6B587: virReallocN (viralloc.c:245)
==9996== by 0x4C6B6AE: virExpandN (viralloc.c:294)
==9996== by 0x4C82B54: virDBusMessageDecodeArgs (virdbus.c:907)
==9996== by 0x4C83463: virDBusMessageDecode (virdbus.c:1141)
==9996== by 0x402C45: testMessageArrayRef (virdbustest.c:273)
==9996== by 0x404E71: virtTestRun (testutils.c:201)
==9996== by 0x401C2D: mymain (virdbustest.c:479)
==9996== by 0x4055ED: virtTestMain (testutils.c:789)
==9996== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==9996==
On failures, virBhyveProcessStart() does not cleanup network
interfaces that could be created by virBhyveProcessBuildBhyveCmd(),
which results in a leaked tap device.
To fix that, extract network cleanup code to bhyveNetCleanup()
and use it in cleanup stage of virBhyveProcessStart().
Peter Krempa [Fri, 7 Mar 2014 10:38:18 +0000 (11:38 +0100)]
util: storagefile: Don't pursue backing chain of NULL image
When virStorageFileGetMetadata is called with NULL path argument, the
invalid pointer boils down through the recursive worker and is caught by
virHashAddEntry which is thankfully resistant to NULL arguments. As it
doesn't make sense to pursue backing chains of NULL volumes, exit
earlier.
This was noticed in the virt-aahelper-test with a slightly modified
codebase.
Peter Krempa [Fri, 7 Mar 2014 10:29:19 +0000 (11:29 +0100)]
storage: gluster: Fix crash when initialization of storage backend fails
The libgfapi function glfs_fini doesn't tolerate NULL pointers. Add a
check on the error paths as it's possible to crash libvirtd if the
gluster volume can't be initialized.
Matthias Bolte [Sat, 29 Mar 2014 11:16:38 +0000 (12:16 +0100)]
Prohibit Windows special chars in filenames
Using any of these chars [:*?"<>|] in a filename is forbidden on
Windows and breaks git operations on Windows as git is not able
to create those files/directories on clone or pull.
Because some of them can be used in UNIX filenames they tend to
creep into filenames; especially : in PCI/SCSI device names that
are used as filenames in test cases.
Matthias Bolte [Sat, 29 Mar 2014 10:58:32 +0000 (11:58 +0100)]
tests: Fix SCSI test data filenames for Windows
Windows doesn't allow : in filenames.
Commit 6fdece9a332fc668a89bde96af94e7b7cbf6750d added files with a : in
their names. This broke git operations on Windows as git is not able to
create those files on clone or pull.
Replace : with - in the offending filenames and adapt the test case.
As the tested Linux specific code expects the files to exist with : in
their path use symlinks to provide the name that way.
Cédric Bosdonnat [Tue, 25 Mar 2014 11:48:26 +0000 (12:48 +0100)]
Fix apparmor profile to make vfio pci passthrough work
See lp#1276719 for the bug description. As virt-aa-helper doesn't know
the VFIO groups to use for the guest, allow access to all
/dev/vfio/[0-9]* and /dev/vfio/vfio files if there is a potential need
for vfio
Fix Memory Leak in virQEMUCapsInitGuestFromBinary()
While running qemucaps2xmltest, it was found that valgrind pointed out
the following memory leaks:
==29896== 0 bytes in 1 blocks are definitely lost in loss record 1 of 65
==29896== at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==29896== by 0x4C6B45E: virAllocN (viralloc.c:191)
==29896== by 0x4232A9: virQEMUCapsGetMachineTypesCaps (qemu_capabilities.c:1999)
==29896== by 0x4234E7: virQEMUCapsInitGuestFromBinary (qemu_capabilities.c:789)
==29896== by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118)
==29896== by 0x41FFD1: virtTestRun (testutils.c:201)
==29896== by 0x41EE7A: mymain (qemucaps2xmltest.c:203)
==29896== by 0x42074D: virtTestMain (testutils.c:789)
==29896== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==29896==
==29896== 0 bytes in 1 blocks are definitely lost in loss record 2 of 65
==29896== at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==29896== by 0x4C6B45E: virAllocN (viralloc.c:191)
==29896== by 0x4232A9: virQEMUCapsGetMachineTypesCaps (qemu_capabilities.c:1999)
==29896== by 0x4234E7: virQEMUCapsInitGuestFromBinary (qemu_capabilities.c:789)
==29896== by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118)
==29896== by 0x41FFD1: virtTestRun (testutils.c:201)
==29896== by 0x41EEA3: mymain (qemucaps2xmltest.c:204)
==29896== by 0x42074D: virtTestMain (testutils.c:789)
==29896== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
Li Yang [Thu, 27 Mar 2014 06:10:57 +0000 (14:10 +0800)]
Modify help information of virsh list command
Use 'virsh list domain --title' option can get domain's title,
not description, the original help information 'show short
domain description' will confuse users, so modify it to
'show domain title'
Signed-off-by: Li Yang <liyang.fnst@cn.fujitsu.com>
While running qemucaps2xmltest, it was found that valgrind pointed out
the following memory leaks:
==27045== 160 (112 direct, 48 indirect) bytes in 1 blocks are definitely lost in loss record 51 of 65
==27045== at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==27045== by 0x4C6BACD: virAllocVar (viralloc.c:560)
==27045== by 0x4CAF095: virObjectNew (virobject.c:193)
==27045== by 0x421453: virQEMUCapsNew (qemu_capabilities.c:1805)
==27045== by 0x41F04F: testQemuCapsXML (qemucaps2xmltest.c:72)
==27045== by 0x41FFD1: virtTestRun (testutils.c:201)
==27045== by 0x41EE7A: mymain (qemucaps2xmltest.c:203)
==27045== by 0x42074D: virtTestMain (testutils.c:789)
==27045== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
==27045==
==27045== 160 (112 direct, 48 indirect) bytes in 1 blocks are definitely lost in loss record 52 of 65
==27045== at 0x4A0577B: calloc (vg_replace_malloc.c:593)
==27045== by 0x4C6BACD: virAllocVar (viralloc.c:560)
==27045== by 0x4CAF095: virObjectNew (virobject.c:193)
==27045== by 0x421453: virQEMUCapsNew (qemu_capabilities.c:1805)
==27045== by 0x41F04F: testQemuCapsXML (qemucaps2xmltest.c:72)
==27045== by 0x41FFD1: virtTestRun (testutils.c:201)
==27045== by 0x41EEA3: mymain (qemucaps2xmltest.c:204)
==27045== by 0x42074D: virtTestMain (testutils.c:789)
==27045== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
To ease mocking for bhyve unit tests move virBhyveTapGetRealDeviceName()
out of bhyve_command.c to virnetdevtap and rename it to
virNetDevTapGetRealDeviceName().
When searching for that message later, I found another series that had
been posted by Guannan Ren back in 2012 that somehow slipped between
the cracks:
That patch was very much out of date, but also pointed out some real
problems.
This patch fixes all the noted problems by refactoring
virNetworkDNSSrvDefParseXML() and networkDnsmasqConfContents(), then
verifies those fixes by added several new records to the test case.
Problems fixed:
* both service and protocol now have an underscore ("_") prepended on
the commandline, as required by RFC2782.
* when optional attributes aren't specified, the separating comma is
also now not placed on the dnsmasq commandline. If optional
attributes in the middle of the line are not specified, they are
replaced with a default value in the commandline (1 for port, 0 for
priority and weight).
(actually the would have generated an error, because "optional"
attributes weren't really optional.)
* The allowed characters for both service and protocol are now limited
to alphanumerics, plus a few special characters that are found in
existing names in /etc/services and /etc/protocols. (One exception
is that both of these files contain names with an embedded ".", but
"." can't be used in these fields of an SRV record because it is
used as a field separator and there is no method to escape a "."
into a field.) (Previously only the strings "tcp" and "udp" were
allowed for protocol, but this restriction has been removed, since
RFC2782 specifically says that it isn't limited to those, and that
anyway it is case insensitive.)
* the "domain" attribute is no longer required in order to recognize
the port, priority, and weight attributes during parsing. Only
"target" is required for this.
* if "target" isn't specified, port, priority, and weight are not
allowed (since they are meaningless - an empty target means "this
service is *not available* for this domain").
* port, priority, and weight are now truly optional, as the comments
originally suggested, but which was not actually true.
Michal Privoznik [Wed, 19 Mar 2014 14:46:07 +0000 (15:46 +0100)]
qemuDomainAttachDeviceFlags: Parse device xml as inactive
In all other drivers we are doing so. Moreover, we don't want to parse
runtime information in attach (even if the attach is meant as live)
because we are generating the runtime info ourselves. We can't trust
users they supply sane values anyway.
==1140== 9 bytes in 1 blocks are definitely lost in loss record 72 of 1,151
==1140== at 0x4A06C2B: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==1140== by 0x623C758: xmlStrndup (in /usr/lib64/libxml2.so.2.9.1)
==1140== by 0x50FD763: virXMLPropString (virxml.c:483)
==1140== by 0x510F8B7: virDomainDeviceInfoParseXML (domain_conf.c:3685)
==1140== by 0x511ACFD: virDomainChrDefParseXML (domain_conf.c:7535)
==1140== by 0x5121D13: virDomainDeviceDefParse (domain_conf.c:9918)
==1140== by 0x13AE6313: qemuDomainAttachDeviceFlags (qemu_driver.c:6926)
==1140== by 0x13AE65FA: qemuDomainAttachDevice (qemu_driver.c:7005)
==1140== by 0x51C77DA: virDomainAttachDevice (libvirt.c:10231)
==1140== by 0x127FDD: remoteDispatchDomainAttachDevice (remote_dispatch.h:2404)
==1140== by 0x127EC5: remoteDispatchDomainAttachDeviceHelper (remote_dispatch.h:2382)
==1140== by 0x5241F81: virNetServerProgramDispatchCall (virnetserverprogram.c:437)
When doing live attach, we are passing the inactive definition anyway
since we are passing the result of virDomainDeviceDefCopy() which does
inactive copy by default.
Moreover, we are doing the same mistake in qemuhotplugtest.
Just a side note - it makes perfect sense to parse the runtime info
like alias in qemuDomainDetachDevice and qemuDomainUpdateDeviceFlags()
as in some cases the only difference to distinguish two devices can be
just their alias.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Francesco Romani [Mon, 17 Mar 2014 15:19:44 +0000 (16:19 +0100)]
qemu: export disk snapshot support in capabilities
This patch adds an element to QEMU's capability XML, to
show if the underlying QEMU binary supports the live disk
snapshotting or not.
This allows any client to know ahead of time if the feature
is available.
Without this information available, the only way to check
for the snapshot support is to request one and check for
errors.
Signed-off-by: Francesco Romani <fromani@redhat.com>
John Ferlan [Tue, 25 Mar 2014 17:03:19 +0000 (13:03 -0400)]
Coverity: Resolve a FORWARD_NULL
Recent changes in the module seemed to have caused Coverity to reanalyze
certain parts of the code. Previously the code was modified via commit
id '11a11812' to resolve a different error (perhaps DEADCODE). Up through
commit id '7b3f1f8c' there were no issues.
The new error indicats the 'outbuf' was checked for NULL and then complains
because of the dereference. Adding checks for non-NULL prior to the deref
resulted in a DEADCODE message.
So, resolve using an sa_assert() to keep Coverity quiet especially since
it doesn't understand that outbuf will change as a result of a successful
virCommandRun() call.
John Ferlan [Tue, 25 Mar 2014 16:57:58 +0000 (12:57 -0400)]
Coverity: Resolve a CHECKED_RETURN message
Recent changes to the module seemed to have caused Coverity to find a new
issue regarding the failure to check the return from a sendmsg. The code
doesn't seem to care about the return status, so just added an ignore_value
to keep Coverity quiet.
Stefan Bader [Tue, 25 Mar 2014 15:28:36 +0000 (16:28 +0100)]
libxl: Create log directory earlier
Commit d9f19c30d054c86b15a304f4118baa4fa75af9d2 moved a lot of the
configuration setup into libxlDriverConfigNew().
However that tries to create the libxl/libxl-driver.log before the
libxl directory gets created in libxlStateInitialize().
This causes the daemon to fail on systems that have not had the directory
created before.
Move the code to create the libxl directory into libxlDriverConfigNew().
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
When lock failure is detected by sanlock, our sanlock_helper kill script
will try to restart (shutdown followed by start) the affected domain
when RESTART action is configured for it. While shutting down kills QEMU
and removes all its leases (which is what sanlock wants to happen),
trying to start it again just hangs because libvirt tries reacquire the
locks in the failed lock space. Hence, this action cannot be supported
by sanlock driver.
Sanlock expects that the configured kill script either kills the PID on
lock failure or removes all locks the PID owns. If none of the two
options happen, sanlock will reboot the host. Although IGNORE action is
supposed to ignore the request to kill the PID or remove all leases,
it's certainly not designed to cause the host to be rebooted. That said,
IGNORE action is incompatible with sanlock and should be forbidden by
libvirt.
Peter Krempa [Tue, 25 Mar 2014 07:17:27 +0000 (08:17 +0100)]
util: Sanitize ATTRIBUTE_NONNULL use in viriscsi.h
Some of the function attributes marked as nonnull actually explicitly
handle the arguments for NULL. All changed functions handle missing
"initiatoriqn" argument well and virISCSIScanTargets also handles well
if the return pointers are missing. Remove some of the liberaly used
ATTRIBUTE_NONNULLs as coverity and possibly other compilers that honor
the attribute fail to compile the code.
Qiao Nuohan [Sun, 23 Mar 2014 03:51:12 +0000 (11:51 +0800)]
add new virDomainCoreDumpWithFormat API
--memory-only option is introduced without compression supported. Now qemu
has support for dumping domain's memory in kdump-compressed format. This
patch adds a new virDomainCoreDumpWithFormat API, so that the format in
which qemu dumps domain's memory can be specified.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Eric Blake [Wed, 19 Mar 2014 17:11:16 +0000 (11:11 -0600)]
conf: prepare to track multiple host source files per <disk>
It's finally time to start tracking disk backing chains in
<domain> XML. The first step is to start refactoring code
so that we have an object more convenient for representing
each host source resource in the context of a single guest
<disk>. Ultimately, I plan to move the new type into src/util
where it can be reused by virStorageFile, but to make the
transition easier to review, this patch just creates the
new type then fixes everything until it compiles again.
projects / thirdparty / libvirt.git / log
