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)
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140724
Reported-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
(cherry picked from commit 3e745e8f77)
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 35c7701c64
but since it was introduced a few commits before that it was never
released as working.
(cherry picked from commit 42619ed05d)
Signed-off-by: Eric Blake <eblake@redhat.com>
Conflicts:
src/qemu/qemu_driver.c - no refactoring of commit 7b7bf001
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.
* docs/apibuild.py (CParser.parseEnumBlock): Fix implicit enum
values.
Signed-off-by: Eric Blake <eblake@redhat.com>
(cherry picked from commit 9b291bbe20)
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.
(cherry picked from commit 02b364e186)
This fixes a regression introduced in commit ff5f30b.
Signed-off-by: Eric Blake <eblake@redhat.com>
Conflicts:
src/qemu/qemu_driver.c - no refactoring of commit 7b7bf001
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.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
(cherry picked from commit d6b27d3e4c)
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
(cherry picked from commit 0eac9d1e90)
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.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1058149
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 736e017e36)
by moving qemuAgentCommand() after qemuAgentCheckError().
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit e9d09fe196)
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:
https://bugzilla.redhat.com/show_bug.cgi?id=1058149
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 5b3492fadb)
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>
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==
Signed-off-by: Eric Blake <eblake@redhat.com>
'virsh help event' included a summary line "event - (null)"
due to a misnamed info field.
* tools/virsh-domain.c (info_event): Use correct name.
* tools/virsh-network.c (info_network_event): Likewise.
Signed-off-by: Eric Blake <eblake@redhat.com>
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().
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.
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.
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.
Windows doesn't allow : in filenames.
Commit 6fdece9a33 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.
virNodeDeviceListCaps will always return empty for a pci nodedevice,
actually it should return 'pci'.
It is because the loop variable ncaps isn't increased.
Introduced by commit be2636f.
https://bugzilla.redhat.com/show_bug.cgi?id=1081932
Signed-off-by: Jincheng Miao <jmiao@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
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
Signed-off-by: Eric Blake <eblake@redhat.com>
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)
Signed-off-by: Eric Blake <eblake@redhat.com>
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)
At this point unittest covers 4 basic cases:
- minimal working XML for bhyve
- same as above, but with virtio disk
- ACPI and APIC args test
- MAC address test
To ease mocking for bhyve unit tests move virBhyveTapGetRealDeviceName()
out of bhyve_command.c to virnetdevtap and rename it to
virNetDevTapGetRealDeviceName().
A patch submitted by Steven Malin last week pointed out a problem with
libvirt's DNS SRV record configuration:
https://www.redhat.com/archives/libvir-list/2014-March/msg00536.html
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:
https://www.redhat.com/archives/libvir-list/2012-July/msg00236.html
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.
<srv service='sip' protocol='udp' domain='example.com'
target='tests.example.com' port='5060' priority='10'
weight='150'/>
before: srv-host=sip.udp.example.com,tests.example.com,5060,10,150
after: srv-host=_sip._udp.example.com,tests.example.com,5060,10,150
* if "domain" wasn't specified in the <srv> element, the extra
trailing "." will no longer be added to the dnsmasq commandline.
<srv service='sip' protocol='udp' target='tests.example.com'
port='5060' priority='10' weight='150'/>
before: srv-host=sip.udp.,tests.example.com,5060,10,150
after: srv-host=_sip._udp,tests.example.com,5060,10,150
* 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).
<srv service='sip' protocol='udp' target='tests.example.com'
port='5060'/>
before: srv-host=sip.udp.,tests.example.com,5060,,
after: srv-host=_sip._udp,tests.example.com,5060
(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.
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>
The test is loosely inspired from qemucapabilitiestest
and qemuxml2xmltest.
Added a new test instead of extending an existing one because
the feature being tested don't really fits nicely in any
existing place.
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>
