qemu_agent: Ignore expected EOFs

https://bugzilla.redhat.com/show_bug.cgi?id=892079

One of my previous patches (f2a4e5f176c408) tried to fix crashing
libvirtd on domain detroy. However, we need to copy pattern from
qemuProcessHandleMonitorEOF() instead of decrementing reference
counter. The rationale for this is, if qemu process is dying due
to domain being destroyed, we obtain EOF on both the monitor and
agent sockets. However, if the exit is expected, qemuProcessStop
is called, which cleans both agent and monitor sockets up. We
want qemuAgentClose() to be called iff the EOF is not expected,
so we don't leak an FD and memory. Moreover, there could be race
with qemuProcessHandleMonitorEOF() which could have already
closed the agent socket, in which case we don't want to do
anything.

virbitmaptest: Resolve Coverity errors

test1: Need to check for bitmap before using as well as free it properly
test2: need to check for bitsString2 before using it.

rpc: Revert Coverity tag message

Turns out the fix for VIR_FREE made this particular Coverity tag
unnecessary, so I'm removing it.

lxc_process: Avoid passing NULL iface->iname

A followon to commit id: 68dceb635 - if iface->iname is NULL, then
neither virNetDevOpenvswitchRemovePort() nor virNetDevVethDelete()
should be called.  Found by Coverity.

lxc_driver: Need to check for vm before calling virDomainUnlock(vm)

tests: Remove VIR_FREE() on static/stack buffer (der.data)

nodeinfo: Use sa_assert() instead of Coverity error tag

Fix nwfilter driver reload/shutdown handling when unprivileged

Although the nwfilter driver skips startup when running in a
session libvirtd, it did not skip reload or shutdown. This
caused errors to be reported when sending SIGHUP to libvirtd,
and caused an abort() in libdbus on shutdown due to trying
to remove a dbus filter that was never added

build: allow virObject to have no parent

When building with static analysis enabled, we turn on attribute
nonnull checking.  However, this caused the build to fail with:

../../src/util/virobject.c: In function 'virObjectOnceInit':
../../src/util/virobject.c:55:40: error: null argument where non-null required (argument 1) [-Werror=nonnull]

Creation of the virObject class is the one instance where the
parent class is allowed to be NULL.  Making things conditional
will let us keep static analysis checking for all other .c file
callers, without breaking the build on this one exception.

* src/util/virobject.c: Define witness.
* src/util/virobject.h (virClassNew): Use it to force most callers
to pass non-null parameter.

qemu: Support ram bar size for qxl devices

Adds a "ram" attribute globally to the video.model element, that changes
the resulting qemu command line only if video.type == "qxl".

<video>
  <model type='qxl' ram='65536' vram='65536' heads='1'/>
</video>

That attribute gets a default value of 64*1024. The schema is unchanged
for other video element types.

The resulting qemu command line change is the addition of

-global qxl-vga.ram_size=<ram>*1024

or

-global qxl.ram_size=<ram>*1024

For the main and secondary qxl devices respectively.

The default for the qxl ram bar is 64*1024 kilobytes (the same as the
default qxl vram bar size).

viralloc: Adjust definition of VIR_FREE() for Coverity

The Coverity static analyzer was generating many false positives for the
unary operation inside the VIR_FREE() definition as it was trying to evaluate
the else portion of the "?:" even though the if portion was (1).

Signed-off-by: Eric Blake <eblake@redhat.com>

conf: Improve error messages if parsing of vCPU count fails

conf: Check if number of vCPUs fits in the storage variable

The count of vCPUs for a domain is extracted as a usingned long variable
but is stored in a unsigned short. If the actual number was too large,
a faulty number was stored.

virsh-domain: Refactor error paths for cmdCPUStats

This patch fixes the following issues in the cpu-stats virsh command:

1) Renames label failed_params to no_memory to match coding style
2) Uses proper typed parameter cleanup in error paths to avoid leaks
3) Adds a ret variable and simplifies error labels
4) Changes error message to a slightly more descriptive one and gets rid
   of the newline at the end:

Before:
$ virsh cpu-stats tr
error: Failed to virDomainGetCPUStats()

error: Requested operation is not valid: domain is not running

After:
$ tools/virsh cpu-stats tr
error: Failed to retrieve CPU statistics for domain 'tr'
error: Requested operation is not valid: domain is not running

tests: Need to initialize 'test' properly on error path

In the error path, the test buffer is free'd, but due to how the free
routine is written the 'test' buffer pointer does not return to the caller
as NULL and then the free'd buffer address is returned to the caller.

virkeepalive: Remove erroneous VIR_FREE(msg)

The 'msg' free is handled via virNetMessageFree() already.

security: Need to initialize 'sens'

It was possible to call VIR_FREE in cleanup prior to initialization

interface: Need to initialize 'ifaces_list'

It was possible to call VIR_FREE in cleanup prior to initialization

storage: Need to initialize 'zerobuf'

It was possible to call VIR_FREE in cleanup prior to initialization.

virsh: Need to intialize 'str'

It was possible to call VIR_FREE in error prior to initialization.

lxc: Need to initialize 'dst'

It was possible to call VIR_FREE in cleanup prior to initialization

virfile: Need to initialize 'looppath'

It was possible to call VIR_FREE in cleanup prior to initialization.

commandtest: Need to initialize 'errbuf'

It was possible to call VIR_FREE in cleanup prior to initialization

virnetdev: Need to initialize 'pciConfigAddr'

It was possible to call VIR_FREE in cleanup prior to initialization

virnetsockettest: Need to initialize 'path'

It was possible to call VIR_FREE in cleanup prior to initialization

virnetserver: Need to initialize 'sigdata'

It was possible to call VIR_FREE in error prior to initialization

conf: Need to initialize variables before VIR_FREE

Resolve a couple of instances where variables were not initialized
prior to potential VIR_FREE call in cleanup path.

uml: Avoid resource leak of event in umlInofityEvent

If there was more than one inotify_event found in the read/while loop,
then only the last event found would have been queued.

fix typos in comments for VIR_DOMAIN_PROCESS_SIGNAL_{PWR,SYS}

storage: Add coverity[dead_error_condition] to avoid error

The local redefinition of PED_PARTITION_PROTECTED results in the error
but is not a problem especially if the built code doesn't have the latest
definitions.

network: Add coverity[leaked_handle] to ignore error

On error, the 'tapfd' in networkStartNetworkVirtual() is synonymous
with 'macTapIfName' and will be closed in the appropriate error path.

remote: Avoid coverity[leaked_storage] message.

Upon successful return of virNetClientStreamEventAddCallback() the
allocated cbdata field will be freed by virNetClientStreamEventRemoveCallback()
as cbOpaque using the free function remoteStreamCallbackFree().

qemu: Add coverity[negative_returns] tag

This avoids "Event negative_returns: A negative constant "-1" is passed as
an argument to a parameter that cannot be negative.".  The called function
uses -1 to determine whether it needs to traverse all the hostdevs.

rpc: Add coverity[dead_error_begin] tag

Coverity misses the nuance of VIR_FREE(privkey) setting privkey = NULL when
if (!(virFileExists(privkey))) is true and thus declares the code dead.

xen: Add coverity[ptr_arith] and [sign_extension] tags

The old cpu bitmap setting algorithm causes a couple of complaints which
have been tagged.

nodeinfo: Add coverity[dead_error_begin] and [returned_null] tags

The use of switch statements inside a bounded for loop resulted in some
false positives regarding the "default:" label which cannot be reached
since each of the other case statements use the possible for loop values.
A [dead_error_begin] was added before the default label.

Commit id ebdbe25a adjusted the algorithm and the caller guarantees that
the 'params' will have a '_' in the name being searched. Add the [returned_null]
tag to the two instances.

lxc: Add coverity[dead_error_begin] tag in switch stmts

The use of switch statements inside a bounded for loop resulted in some
false positives regarding the "default:" label which cannot be reached
since each of the other case statements use the possible for loop values.

xend: Address some Coverity false positives

The various _for_i loops with both u.s.car and u.s.cdr were being reported
as COPY_PASTE errors by Coverity.  This just quiets those messages.

selinux: Resolve resource leak using the default disk label

Commit id a994ef2d1 changed the mechanism to store/update the default
security label from using disk->seclabels[0] to allocating one on the
fly. That change allocated the label, but never saved it.  This patch
will save the label. The new virDomainDiskDefAddSecurityLabelDef() is
a copy of the virDomainDefAddSecurityLabelDef().

util: Need to check child JSON allocation before use

parallels: Need to handle virJSONValueObjectGetString error

rpc: Remove unnecessary calls

The code is not reachable as of commit id: bb85f229. Removed
virKeepAliveStop() and virObjectUnref() because 'ka' cannot be
anything but NULL at the cleanup label.

security: Use virStrcpy to move the label

qemu: Forbid snapshot names starting with '.'

Forbid the names to match the loading procedure of snapshots.

safe{read,write}: Document usage with nonblocking FD

Currently, whenever somebody calls saferead() on nonblocking FD
(safewrite() is totally interchangeable for purpose of this message)
he might get wrong return value. For instance, in the first iteration
some data is read. The number of bytes read is stored into local
variable 'nread'. However, in next iterations we can get -1 from
read() with errno == EAGAIN, in which case the -1 is returned despite
fact some data has already been read. So the caller gets confused.
Bare read() should be used for nonblocking FD.

docs: Generate documentation for virTypedParams* APIs

docs: event.c source file was renamed as virevent.c

spec: indent %if to make it easier to see conditions

Nested conditionals are hard to read if they are not indented.
We can't add arbitrary whitespace to everything in spec files,
but we CAN add spaces before %if and %define.  Use this trick,
plus a fancy sed script that rewrites a spec file into a C
file, so we can use cppi to keep our spec file nice.

For reference, the sed script converts code like:

|# RHEL-5 builds are client-only for s390, ppc
|%if 0%{?rhel} == 5
|    %ifnarch %{ix86} x86_64 ia64
|        %define client_only        1
|    %endif
|%endif

into the following for cppi:

|// # RHEL-5 builds are client-only for s390, ppc
|#if a // 0%{?rhel} == 5
|# if a // %{ix86} x86_64 ia64
|#  define client_only        1
|# endif
|#endif

and errors from 'make syntax-check' look like:
spec_indentation
cppi: mingw-libvirt.spec.in: line 130: not properly indented
maint.mk: incorrect preprocessor indentation

* libvirt.spec.in: Add some indentation to make it easier to follow
various conditionals.
* mingw-libvirt-spec.in: Likewise.
* cfg.mk (sc_spec_indentation): New syntax check to enforce it.

qemu: Reject attempts to create snapshots with names containig '/'

The snapshot name is used to create path to the definition save file.
When the name contains slashes the creation of the file fails. Reject
such names.

qemu: Don't return success if creation of snapshot save file fails

When the snapshot definition can't be saved, the
qemuDomainSnapshotCreate function succeeded without filling some of the
fields in the internal definition.

This patch removes the snapshot and returns failure if the XML file
cannot be written.

Log flags passed to qemuMigrationPrepare{Tunnel,Direct} APIs

We are already logging other arguments passed, however, @flags
were missing there.

build: Fix RPM build errors related to libvirt-lxc API

Added missing entries to makefile and spec.

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>

Fix race condition when destroying guests

When running virDomainDestroy, we need to make sure that no other
background thread cleans up the domain while we're doing our work.
This can happen if we release the domain object while in the
middle of work, because the monitor might detect EOF in this window.
For this reason we have a 'beingDestroyed' flag to stop the monitor
from doing its normal cleanup. Unfortunately this flag was only
being used to protect qemuDomainBeginJob, and not qemuProcessKill

This left open a race condition where either libvirtd could crash,
or alternatively report bogus error messages about the domain already
having been destroyed to the caller

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

docs: Add some style and color to the HTML documentation

Signed-off-by: Claudio Bley <cbley@av-test.de>

docs: don't use <i> and <tt> HTML tags

Use of <tt> is discouraged in HTML 4.x and has finally been obsoleted
in HTML 5. Likewise for the <i> tag.

Using tables for layout is (widely) considered bad style, too.

Use defintion lists, definition term and defintion description
elements instead.

Signed-off-by: Claudio Bley <cbley@av-test.de>

docs: Assign classes to documentation elements

In CSS the following class names are available:

* keyword     (keywords like "typedef", "struct")
* type        (types like "int", "void*")
* comment     (comments after members of enums or structs)
* directive   (preprocessor directives, #define)
* undisclosed (text saying that the API is not public)

Additionally, kill all of the left-over "programlisting" class
assignments. There are no CSS rules for them.

Signed-off-by: Claudio Bley <cbley@av-test.de>

Introduce virTypedParamsClear public API

The function is just a renamed public version of former
virTypedParameterArrayClear.

virsh: Use virTypedParams* APIs in cpu-stats

virsh: Use virTypedParams* APIs in node-memory-tune

virsh: Use virTypedParams* APIs in numatune

virsh: Use virTypedParams* APIs in memtune

virsh: Use virTypedParams* APIs in domblkstat

virsh: Use virTypedParams* APIs in schedinfo

virsh: Use virTypedParams* APIs in domiftune

virsh: Use virTypedParams* APIs in blkiotune

virsh: Use virTypedParams* APIs in blkdeviotune

Add virTypedParams* APIs

Working with virTypedParameters in clients written in C is ugly and
requires all clients to duplicate the same code. This set of APIs makes
this code for manipulating with virTypedParameters integral part of
libvirt so that all clients may benefit from it.

qemu: Simplify condition with already extracted flag

storage: Fix lvcreate parameter for backingStore.

When virStorageBackendLogicalCreateVol() creates a snapshot for a
logical volume with backingStore element, it fails with the message
below:

  2013-01-17 03:10:18.869+0000: 1967: error : virCommandWait:2345 :
  internal error Child process (/sbin/lvcreate --name lvm-snapshot -L 51200K
  -s=/dev/lvm-pool/lvm-volume) unexpected exit status 3: /sbin/lvcreate:
  invalid option -- '='  Error during parsing of command line.

This is because virCommandAddArgPair() uses '=' to connect the two
parameters, it's unsuitable for -s option of the lvcreate.

Signed-off-by: Atsushi Kumagai <kumagai-atsushi@mxc.nes.nec.co.jp>

build: fix build on BSD

A build on FreeBSD failed with:
util/virportallocator.c:108: error: storage size of 'addr' isn't known
util/virportallocator.c:123: error: 'INADDR_ANY' undeclared (first use in this function)

It turns out that while POSIX allows sockaddr_in to leak in through
<arpa/inet.h> (the way Linux does it), it is not mandatory, and
conforming applications are required to get it through <netinet/in.h>.

* src/util/virportallocator.c: Include header for struct
sockaddr_in.
* tests/virportallocatortest.c: Likewise.

network: Remove dead code getting, but not using ipdef

The fetch of 'ipdef' in networkRefreshDhcpDaemon() when the loop to fill
in ipv4def fails to find an ipv4 address with dhcp defined. The filled in
ipdef value was not used.  Code was made unnecessary with commit it 2d5cd1.

virobject: Remove the bogus ! from call to virObjectInitialize()

esx: No need to check for objectSpec

Coverity complains that the objectSpec != NULL check was unnecessary because
there was no way to get to the label with objectSpec = NULL.

qemu: Check valid activeDev before calling pciDeviceSetUsedBy

Followup fix for integer wraparound in port allocator

Change iterator variable datatype to int

qemu: Double mutex unlock in qemuDomainModifyDeviceFlags

The driver mutex was unlocked in qemuDomainModifyDeviceFlags before
entering qemuDomainObjBeginJobWithDriver where it will be unlocked once
more leaving it in an undefined state. The result was that two
threads were simultaneously looking up the domain hash table during
multiple parallel device attach/detach operations.
Luckily this triggered a virHashIterationError.

Signed-off-by: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>

fixed xt_physdev warning when defining ip(6)tables rules

When starting a VM, /var/log/messages was spammed with the following message:
 xt_physdev: using --physdev-out in the OUTPUT, FORWARD and POSTROUTING chains for non-bridged traffic is not supported anymore.

With each extra VM I start, the messages get amplified
exponentially. This results in longer starting times every new VM,
relative the the previously started VM. When I ran a test with
starting 100 equal VM's, the first VM started in about 2 seconds, the
100th VM took 48 seconds to start. I'm running a vanilla 3.7.1 kernel,
but I have the same issue on VM hosts with kernel 3.2.28 or 3.2.0,
running libvirt 0.9.12 and 0.9.8 respectively.

Looking into the warning, it seemed that iptables need an extra argument,
--physdev-is-bridged, in commands like:
 iptables -A libvirt-out -m physdev --physdev-is-bridged --physdev-out vnet99 -g FP-vnet99

With that, the warnings in /var/log/messages are gone and running the
test again proved the 100th VM started in 3.8 seconds.

conf: don't fail to parse <boot> when parsing a single device

This resolves:

  https://bugzilla.redhat.com/show_bug.cgi?id=895294

The symptom was that attempts to modify a network device using
virDomainUpdateDeviceFlags() would fail if the original device had a
<boot> element (e.g. "<boot order='1'/>"), even if the updated device
had the same <boot> element. Instead, the following error would be logged:

  cannot modify network device boot index setting

It's true that it's not possible to change boot order (internally
known as bootIndex) of a live device; qemuDomainChangeNet checks for
that, but the problem was that the information it was checking was
incorrect.

Explanation:

When a complete domain is parsed, a global (to the domain) "bootMap"
is passed down to the parse for each device; the bootMap is used to
make sure that devices don't have conflicting settings for their boot
orders.

When a single device is parsed by itself (as in the case of
virDomainUpdateDeviceFlags), there is no global bootMap that would be
appropriate to send, so NULL is sent instead. However, although the
lowest level function that parses just the boot order *does* simply
skip the sanity check in that case, the next higher level
"virDomainDeviceInfoParseXML" function refuses to call down to the
lower "virDomainDeviceBootParseXML" if bootMap is NULL. So, the boot
order is never set in the "new" device object, and when it is compared
to the original (which does have a boot order), they don't match.

The fix is to patch virDomainDeviceInfoParseXML to not care about
bootMap, and just always call virDomainDeviceInfoBootParseXML whenever
there is a <boot> element. When we are only parsing a single device,
we don't care whether or not any specified boot order is consistent
with the rest of the domain; we will always do this check later (in
the current case, we do it by verifying that the net bootIndex exactly
matches the old bootIndex).

network: use bandwidth from portgroup when appropriate

The bandwidth plug and unplug functions were assuming that an
interface's bandwidth setting was always specified directly in the
domain's <interface> definition, but that's not necessarily true - it
could have been obtained from a <portgroup> definition in the network
definition. This patch fixes those functions to use
virDomainNetGetActualBandwidth(), which gets the bandwidth pointer
from iface->data.network.actual if it exists, otherwise returns
iface->bandwidth.

network: Resolve some issues around vlan copying

Remove extraneous check for 'netdef' when dereferencing for vlan.nTags.
Prior code would already check if netdef was NULL.

Coverity complained about a path where the 'vlan' was potentially valid,
but a prior checks may not have allocated 'iface->data.network.actual',
so like other paths it needs to be allocated on the fly.

Move the copying of vlan up earlier in networkAllocateActualDevice, so
that actual.type gets properly set.

Since the first assignment to vlan is redundant except in the case of
jumping immediately to validate from the start of the function,
eliminate its initial setting at the top of the function in favor of
calling the helper function virDomainNetGetActualVlan() (which doesn't
depend on the local vlan pointer being initialized) down at validate:

Signed-off-by: Laine Stump <laine@redhat.com>

Fix inheritance of virNetClient

When creating the virClass object for virNetClient, we specified
virObject as the parent instead of virObjectLockable

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

lock_daemon: Simplify error handling

No need to check the run_dir variable twice for NULL.

Avoid integer wrap on remotePortMax in QEMU driver

The QEMU driver default max port is 65535, but it then increments
this by 1 to 65536. This maps to 0 in an unsigned short :-( This
was apparently done so that for() loops could use "< max" instead
of "<= max". Remove this insanity and just make the loop do the
right thing.

virsh-volume.c: Switch to C99 initialization of vshCmdOptDef

virsh-snapshot.c: Switch to C99 initialization of vshCmdOptDef

virsh-secret.c: Switch to C99 initialization of vshCmdOptDef

virsh-pool.c: Switch to C99 initialization of vshCmdOptDef

virsh-nwfilter.c: Switch to C99 initialization of vshCmdOptDef

virsh-nodedev.c: Switch to C99 initialization of vshCmdOptDef

virsh-network.c: Switch to C99 initialization of vshCmdOptDef

virsh-interface.c: Switch to C99 initialization of vshCmdOptDef

virsh-host.c: Switch to C99 initialization of vshCmdOptDef

virsh-domain-monitor.c: Switch to C99 initialization of vshCmdOptDef

virsh-domain.c: Switch to C99 initialization of vshCmdOptDef

virsh.c: Switch to C99 initialization of vshCmdOptDef

libvirt.c: fix typo in comment

Move comments after enum members

The api builder always associates comments to the last member it read,
not to the current member even if there was a comment for the previous
member and a comma was already seen.

This has the effect that the comment for the previous member gets
overwritten and the current member has no comment at all.

python: Avoid freeing uninitialized new_params pointer

include virterror_internal.h in threads.h

required by VIR_ONCE_GLOBAL_INIT using virSetError.

include util.h in cgroup.h

required by VIR_ENUM_DECL.