Peter Krempa [Mon, 30 Sep 2013 09:47:47 +0000 (11:47 +0200)]
qemu: process: Silence coverity warning when rewinding log file
The change in ef29de14c37d14abc546e90555a0093797facfdd that introduced
better error logging from qemu introduced a warning from coverity about
unused return value from lseek. Silence this warning and fix typo in the
corresponding error message.
Michal Privoznik [Mon, 30 Sep 2013 09:23:08 +0000 (11:23 +0200)]
python: Document virNodeGetInfo bug
The memory size in virNodeGetInfo python API binding is reported in MiB
instead of KiB (like we have in C struct). However, there already might
be applications out there relying on this inconsistence so we can't
simply fix it. Document this sad fact as known bug.
Currently the VMware version check code only supports two types of
VMware backends, Workstation and Player. But in the near future we will
have an additional one so we need to support more. Additionally, we
discover and cache the path to the vmrun binary so we should use that
path when using the corresponding binary from the VMware VIX SDK.
Michal Privoznik [Fri, 27 Sep 2013 16:04:31 +0000 (18:04 +0200)]
Makefile.am: Always include rule to make org.libvirt.api.policy
When running 'make dist' on a system without policykit, we currently
fail. This is because $(srcdir)/access/org.libvirt.api.policy is in
EXTRA_DIST, however, the rule to generate the file is conditional
whether we build with polkit or not.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Fri, 27 Sep 2013 14:47:12 +0000 (16:47 +0200)]
fchosttest: Run the test only under linux
Currently, we have functions to handle fc_host implemented just
for linux. On all other platforms an error is thrown. It makes no
sense to run the test on those platforms then.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Michal Privoznik [Fri, 27 Sep 2013 14:20:49 +0000 (16:20 +0200)]
genprotocol.pl: Fix code on FreeBSD too
On some systems (linux, cygwin and gnukfreebsd) rpcgen generates files
which when compiling produces this warning:
remote/remote_protocol.c: In function 'xdr_remote_node_get_cpu_stats_ret':
remote/remote_protocol.c:530: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]
Hence, on those systems we need to post-process the files by the
rpc/genprotocol.pl perl script. At the beginning of the script the OS is
detected via $^O perl variable. From my latest build on FreeBSD I see we
need to fix the code there too. On FreeBSD the variable contains
'freebsd' string:
http://perldoc.perl.org/perlport.html#PLATFORMS
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
The debug message said there was a timeout of 0 pending for -1 ms which
made me think this is where a hang was coming from but according to the
function comments this case means that there is no timeout pending so
make the debug message say that instead of saying there's a -1 ms
timeout.
While BSDs don't support process creation timestamp information via
PEERCRED for Unix sockets, we need to actually initialize the value
because it is used by the libvirt code.
Fix crash in libvirtd when events are registered & ACLs active
When a client disconnects from libvirtd, all event callbacks
must be removed. This involves running the public API
virConnectDomainEventDeregisterAny
This code does not run in normal API dispatch context, so no
identity was set. The result was that the access control drivers
denied the attempt to deregister callbacks. The callbacks thus
continued to trigger after the client was free'd causing fairly
predictable use of free memory & a crash.
This can be triggered by any client with readonly access when
the ACL drivers are active.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
It's possible to create a domain which will only use a TLS port
and will not have a non-TLS port set by using:
<graphics type='spice' autoport='yes' defaultMode='secure'/>
In such a setup, the 'graphics' node for the running domain will be:
<graphics type='spice' tlsPort='5900'
autoport='yes' listen='127.0.0.1'
defaultMode='secure'>
However, cmdDomDisplay loops over all the 'graphics' node, and it
ignores nodes which don't have a 'port' attribute. This means
'virsh domdisplay' will only return an empty string for domains
as the one above.
This commit looks for both 'port' and 'tlsPort' before deciding
to ignore a graphics node. It also makes sure 'port' is not printed
when it's not set.
This makes 'virsh domdisplay' return
'spice://127.0.0.1?tls-port=5900' for domains using only a TLS
port.
However, the child only unreferenced hookData.cfg and the parent only
removed the cfg reference. That said, we don't need to increment the
reference counter when assigning cfg to hookData. Both the child and the
parent will correctly remove the reference on cfg (the child will do
that through hookData).
Commit 6d7d0b1869ed293e3208d11f375cecea0129dfc5 (in 1.1.2) added bounds
checking to virDomainGetJobStats. But even at that time the API was able
to return 20 parameters while the limit was set to 16.
Eric Blake [Thu, 26 Sep 2013 21:14:38 +0000 (15:14 -0600)]
build: fix use of rpc.h on cygwin
Caused by commit 012c25e8 splitting out a convenience library.
CC libvirtd_conf_la-libvirtd-config.lo
In file included from ../src/rpc/virnetmessage.h:24:0,
from ../src/rpc/virnetserverprogram.h:27,
from ../src/rpc/virnetserver.h:32,
from libvirtd-config.c:31:
../src/rpc/virnetprotocol.h:9:21: fatal error: rpc/rpc.h: No such file
or directory
qemu: virDomainControllerFind may return 0 if controller found
The return value of virDomainControllerFind >=0 means that
the specific controller was found.
But some functions invoke it and treat 0 as not found.
This patch fix these incorrect invocation.
Peter Krempa [Thu, 26 Sep 2013 06:12:39 +0000 (08:12 +0200)]
conf: Don't crash on invalid chardev source definition of RNGs and other
Since commit 297c99a5 an invalid source definition XML of a character
device that is used as backend for RNG devices, smartcards and redirdevs
causes crash of the daemon when parsing such a definition.
The device types mentioned above are not a part of a regular character
device but are backends for other types. Thus when parsing such device
NULL is passed as the argument @chr_def. Later when checking the
validity of the definition @chr_def was dereferenced when parsing a UNIX
socket backend with missing path of the socket and crashed the daemon.
spec: Clean up distribution of ChangeLog (and others)
- Move COPYING* to libvirt-client, so every package pulls them in
- Move AUTHORS ChangeLog.gz NEWS README TODO from -daemon to -docs
- Drop duplicate distribution of docs in -python
Avoid double free in qemuMonitorCommonTestInit on OOM
The qemuMonitorCommonTestInit method did not allocate the
test object, so it should not free it upon failure. Doing
so causes a double free with the caller.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
For inexplicable reasons, the nwfilter XML parser is intentionally
ignoring errors that arise during parsing. As well as meaning that
users don't get any feedback on their XML mistakes, this will lead
it to silently drop data in OOM conditions.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Normally a lockspace resource is not freed while there are
active owners. During initial resource creation though, an
OOM error will trigger this scenario. virLockSpaceResourceFree
was not freeing the 'owners' field in this case.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virStoragePoolDefParseSource method would set def->nhosts
before allocating def->hosts. If the allocation failed due to
OOM, the cleanup code would crash accessing out of bounds.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The virDomainSnapshotDefParse method assigned to def->ndisks
before allocating def->disks. Thus if an OOM occurred, the
cleanup code would access out of bounds.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Don't clobber return value in virInterfaceDefParseProtoIPv6
Several places in virInterfaceDefParseProtoIPv6 clobber the
default 'ret' return value. So when jumping to cleanup on
error, 'ret' may mistakenly be set to 0 instead of -1. This
caused failure to report OOM errors, meaning data was silently
lost during parsing.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The methods for obtaining the Xen dom ID cannot distinguish
between returning -1 due to an error and returning -1 due to
the domain being shutoff. Change them to return the dom ID
via an output parameter.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The xenParseSxpr method sets def->nconsoles to 1 before allocating
the def->consoles array. If the allocation fails due to OOM the
cleanup code will thus crash accessing out of bounds.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The xenParseXM sets def->nconsoles to 1 before claling
VIR_REALLOC_N on def->consoles. So if the alloc fails
due to OOM, the cleanup code will crash accessing a
console that does not exist.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Fix leak of command line args in qemuParseCommandLine
If qemuParseCommandLine finds an arg it does not understand
it adds it to the QEMU passthrough custom arg list. If the
qemuParseCommandLine method hits an error for any reason
though, it just does 'VIR_FREE(cmd)' on the custom arg list.
This means all actual args / env vars are leaked. Introduce
a qemuDomainCmdlineDefFree method to be used for cleanup.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The 'qemuStringToArgvEnv' method splits up a string of command
line env/args to an 'arglist' array. It then copies env vars
to a 'progenv' array and args to a 'progargv' array. When
copyin the env vars, it NULL-ifies the element in 'arglist'
that is copied.
Upon OOM the 'virStringListFree' is called on progenv and
arglist. Unfortunately, because the elements in 'arglist'
related to env vars have been set to NULL, the call to
virStringListFree(arglist) doesn't free anything, even
though some non-NULL args vars still exist later in the
array.
To fix this leak, stop NULL-ifying the 'arglist' elements,
and change the cleanup code to only free elements in the
'arglist' array, not 'progenv'.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Fix missing jump to error cleanup in qemuParseCommandLineDisk
In a number of places in qemuParseCommandLineDisk, an error
is reported, but no 'goto error' jump is used. This causes
failure to report OOM conditions to the caller.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Fix leak on OOM in qemuBuildCommandLine dealing with sound card
The qemuBuildCommandLine code for parsing sound cards will leak
an intermediate variable if an OOM occurs. Move the free'ing of
the variable earlier to avoid the leak.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Fix failure to honour OOM status in qemuParseNBDString
In qemuParseNBDString, if the virURIParse fails, the
error is not reported to the caller. Instead execution
falls through to the non-URI codepath causing memory
leaks later on.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Fix leak of address string in qemuDomainPCIAddressGetNextSlot
qemuDomainPCIAddressGetNextSlot has a loop for finding
compatible PCI buses. In the loop body it creates a
PCI address string, but never frees this. This causes
a leak if the loop executes more than one iteration,
or if a call in the loop body fails.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
If virDomainVcpuPinDefArrayFree is called with def != NULL,
but nvcpupin == 0, then it leaks memory for 'def'. This is
an unusual scenario, but it hits when cleaning up after an
OOM during parsing of XML.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
00:1E.0 is the location of this controller on at least some actual Q35
hardware, so we try to replicate the placement. The bridge should work
just as well in any other location though, so if 00:1E.0 isn't
available, just allow it to be auto-assigned anywhere appropriate.
This device is identical to qemu's "intel-hda" device (known as "ich6"
in libvirt), but has a different PCI device ID (which matches the ID
of the hda audio built into the ich9 chipset, of course). It's not
supported in earlier versions of qemu, so it requires a capability
bit.
Laine Stump [Tue, 24 Sep 2013 13:38:32 +0000 (09:38 -0400)]
qemu: replace multiple strcmps with a switch on an enum
I'm not sure why this code was written to compare the strings that it
had just retrieved from an enum->string conversion, rather than just
look at the original enum values, but this yields the same results,
and is much more efficient (especially as you add more devices).
Although most devices available in qemu area defined as PCI devices,
and strictly speaking should only be attached via a PCI slot, in
practice qemu allows them to be attached to a PCIe slot and sometimes
this makes sense.
For example, The UHCI and EHCI USB controllers are usually attached
directly to the PCIe "root complex" (i.e. PCIe slots) on real
hardware, so that should be possible for a Q35-based qemu virtual
machine as well.
We still want to prefer a standard PCI slot when auto-assigning
addresses, though, and in general to disallow attaching PCI devices
via PCIe slots.
This patch makes that possible by adding a new
QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG flag. Three things are done
with this flag:
1) It is set for the "pcie-root" controller
2) qemuCollectPCIAddress() now has a set of nested switches that set
this "EITHER" flag for devices that we want to allow connecting to
pcie-root when specifically requested in the config.
3) qemuDomainPCIAddressFlagsCompatible() adds this new flag to the
"flagsMatchMask" if the address being checked came from config rather
than being newly auto-allocated by libvirt (this knowledge is
conveniently already available in the "fromConfig" arg).
Now any device having the EITHER flag set can be connected to
pcie-root if explicitly requested, but auto-allocated addresses for
those devices will still be standard PCI slots instead.
This patch only loosens the restrictions on devices that have been
specifically requested, but the setup is such that it should be fairly
easy to add new devices.
bridge driver: don't masquerade local subnet broadcast/multicast packets
Packets sent by guests on virbrN, *or* by dnsmasq on the same, to
- 255.255.255.255/32 (netmask-independent local network broadcast
address), or to
- 224.0.0.0/24 (local subnetwork multicast range)
are never forwarded, hence it is not necessary to masquerade them.
In fact we must not masquerade them: translating their source addresses or
source ports (where applicable) may confuse receivers on virbrN.
One example is the DHCP client in OVMF (= UEFI firmware for virtual
machines):
It expects DHCP replies to arrive from remote source port 67. Even though
dnsmasq conforms to that, the destination address (255.255.255.255) and
the source address (eg. 192.168.122.1) in the reply allow the UDP
masquerading rule to match, which rewrites the source port to or above
1024. This prevents the DHCP client in OVMF from accepting the packet.
util/viriptables: add/remove rules that short-circuit masquerading
The functions
- iptablesAddForwardDontMasquerade(),
- iptablesRemoveForwardDontMasquerade
handle exceptions in the masquerading implemented in the POSTROUTING chain
of the "nat" table. Such exceptions should be added as chronologically
latest, logically top-most rules.
The bridge driver will call these functions beginning with the next patch:
some special destination IP addresses always refer to the local
subnetwork, even though they don't match any practical subnetwork's
netmask. Packets from virbrN targeting such IP addresses are never routed
outwards, but the current rules treat them as non-virbrN-destined packets
and masquerade them. This causes problems for some receivers on virbrN.
Peter Krempa [Wed, 18 Sep 2013 14:23:14 +0000 (16:23 +0200)]
qemu: Wire up better early error reporting
The previous patches added infrastructure to report better errors from
monitor in some cases. This patch finalizes this "feature" by enabling
this enhanced error reporting on early phases of VM startup. In these
phases the possibility of qemu producing a useful error message is
really high compared to running it during the whole life cycle. After
the start up is complete, the feature is disabled to provide the usual
error messages so that users are not confused by possibly irrelevant
messages that may be in the domain log.
The original motivation to do this enhancement is to capture errors when
using VFIO device passthrough, where qemu reports errors after the
monitor is initialized and the existing error catching code couldn't
catch this producing a unhelpful message:
# virsh start test
error: Failed to start domain test
error: Unable to read from monitor: Connection reset by peer
With this change, the message is changed to:
# virsh start test
error: Failed to start domain test
error: internal error: early end of file from monitor: possible problem:
qemu-system-x86_64: -device vfio-pci,host=00:1a.0,id=hostdev0,bus=pci.0,addr=0x5: vfio: error, group 8 is not viable, please ensure all devices within the iommu_group are bound to their vfio bus driver.
qemu-system-x86_64: -device vfio-pci,host=00:1a.0,id=hostdev0,bus=pci.0,addr=0x5: vfio: failed to get group 8
qemu-system-x86_64: -device vfio-pci,host=00:1a.0,id=hostdev0,bus=pci.0,addr=0x5: Device 'vfio-pci' could not be initialized
Peter Krempa [Wed, 18 Sep 2013 14:17:39 +0000 (16:17 +0200)]
qemu: monitor: Produce better errors on monitor hangup
Change the monitor error code to add the ability to access the qemu log
file using a file descriptor so that we can dig in it for a more useful
error message. The error is now logged on monitor hangups and overwrites
a possible lesser error. A hangup on the monitor usualy means that qemu
has crashed and there's a significant chance it produced a useful error
message.
The functionality will be latent until the next patch.
Peter Krempa [Wed, 18 Sep 2013 14:12:17 +0000 (16:12 +0200)]
qemu: monitor: Add infrastructure to access VM logs for better err msgs
Early VM startup errors usually produce a better error message in the
machine log file. Currently we were accessing it only when the process
exited during certain phases of startup. This will help adding a more
comprehensive error extraction for early qemu startup phases.
This patch adds infrastructure to keep a file descriptor for the machine
log file that will be used in case an error happens.
Peter Krempa [Wed, 18 Sep 2013 12:43:52 +0000 (14:43 +0200)]
qemu_process: Make qemuProcessReadLog() more versatile and reusable
Teach the function to skip character device definitions printed by qemu
at startup in addition to libvirt log messages and make it usable from
outside of qemu_process.c. Also add documentation about the func.
Check return value of virDomainControllerInsert when parsing QEMU args
The parsing of '-usb' did not check for failure of the
virDomainControllerInsert method. As a result on OOM, the
parser mistakenly attached USB disks to the IDE controller.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The testutils.c file had some fprintfs which had not been
converted from %d to %zu, when 'testCounter' change to be
a size_t. This was a build breaker if --enable-test-oom
was enabled
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Eric Blake [Tue, 24 Sep 2013 12:49:10 +0000 (06:49 -0600)]
maint: update to latest gnulib
Since we're about to freeze, it's time to pick up the latest
upstream gnulib. Among other changes, gnulib now guarantees the
use of some -f flags that we were previously manually adding.
* .gnulib: Update to latest, in part for warning improvements.
* m4/virt-compile-warnings.m4 (LIBVIRT_COMPILE_WARNINGS): Drop
flags that are now guaranteed by gnulib.
* bootstrap: Resync to gnulib.
Always open files in binary mode in virFDStreamOpenFileInternal
On win32, using text mode for binary files might result in short
reads since ASCII character 0x1A is interpreted as EOF. Also, it
could lead to problems using the seek functions because of the \r
handling.
Don't ignore allocation failure in virCommandAddEnvPassCommon
The virCommandAddEnvPassCommon method ignored the failure to
pre-allocate the env variable array with VIR_RESIZE_N. While
this is harmless, it confuses the test harness which is trying
to validate OOM handling of every individual allocation call.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When the various viralloc.c functions were changed to use the
normal error reporting code, the OOM injection code paths
were not updated to report errors.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Fix missing OOM check in qemuParseCommandLine when splitting strings
The qemuParseCommandLine method did not check the return value of
virStringSplit to see if OOM had occurred. This lead to dereference
of a NULL pointer on OOM.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Fix error checking of qemuParseKeywords return status
Most callers of qemuParseKeywords were assigning its return
value to a 'size_t' variable. Then then also checked '< 0'
for error condition, but this will never be true with the
unsigned size_t variable. Rather than using 'ssize_t', change
qemuParseKeywords so that the element count is returned via
an output parameter, leaving the return value solely as an
error indicator.
This avoids a crash accessing beyond the end of an error
upon OOM.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The qemuStringToArgvEnv method was changed to use virStringFreeList
to free the 'arglist' array. This method assumes the string list
array is NULL terminated, however, qemuStringToArgvEnv was not
ensuring this when populating 'arglist'. This caused an out of
bounds access by virStringFreeList when OOM occured in the initial
loop of qemuStringToArgvEnv
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When parsing the RBD hosts, it increments the 'nhosts' counter
before increasing the 'hosts' array allocation. If an OOM then
occurs when increasing the array allocation, the cleanup block
will attempt to access beyond the end of the array. Switch
to using VIR_EXPAND_N instead of VIR_REALLOC_N to protect against
this mistake
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Fix crash on OOM in qemuDomainCCWAddressSetCreate()
If OOM occurs in qemuDomainCCWAddressSetCreate, it jumps to
a cleanup block and frees the partially initialized object.
It then mistakenly returns the address of the just free'd
pointer instead of NULL.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Fix crash if OOM occurs when creating virConnectPtr
If a OOM error occurs in virGetConnect, this may cause the
virConnectDispose method to de-reference a NULL pointer,
since the close callback will not have been initialized.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
projects / libvirt.git / log