Per-domain directories were introduced in order to be able to
completely separate security labels for each domain (commit
f1f68ca3).  However when the domain
name is long (let's say a ridiculous 110 characters), we cannot
connect to the monitor socket because on length of UNIX socket address
is limited.  In order to get around this, let's shorten it in similar
fashion and in order to avoid conflicts, throw in an ID there as well.
Also save that into the status XML and load the old status XMLs
properly (to clean up after older domains).  That way we can change it
in the future.

The shortening can be seen in qemuxml2argv tests, for example in the
hugepages-pages2 case.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Found by inspection - after calling virStoragePoolObjAssignDef the
pool is part of the driver->pools.objs list and the failure path
for the virStoragePoolObjSaveDef will use virStoragePoolObjRemove
to remove the pool from the objs list which will unlock and free
the pool pointer (as pools->objs[i] during the loop). Since the call
doesn't clear the pool address from the callee, we need to set it
to NULL; otherwise, the virStoragePoolObjUnlock in the cleanup: code
will fail miserably.

While reviewing how storage driver used ObjListPtr's for reference
in some recent secret driver patches to use the same mechanism, I came
across an instance where the wrong API was called for error paths after
successfully allocating the storage pool pointer and inserting into
the driver pool list.

The path is after virStoragePoolObjAssignDef succeeds - the 'def' passed
in is assigned to pool->def (or newDef) so it shouldn't be the only thing
deleted. The pool is now part of driver->pools.objs, so it would need to
be removed (as happens in the storagePoolCreateXML error paths).

Rather than calling virStoragePoolDefFree to free the def which is now
assigned to the pool, call virStoragePoolObjRemove to ensure the pool
element is removed from the driver list and that anything stored in pool
is properly handled by virStoragePoolObjFree including the call to
virStoragePoolDefFree for the pool->{def|newDef} element.

Commit f1a89a8b allowed parsing configs from /etc/libvirt
without validating the emulator capabilities.

Check for the presence of a machine type in the qemu driver's
post parse function instead of crashing.

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

Revert commit 55e6d8cd.

This fix for https://bugzilla.redhat.com/show_bug.cgi?id=1267256
unconditionally required a machine type for all machine types
even though qemu is the only emulator using them.

Revert it to fix persistent configs for drivers with no machine type:
https://www.redhat.com/archives/libvir-list/2016-February/msg01228.html

libxlMakeNic opens a virConnect object and takes a reference on a
virNetwork object, but doesn't drop the references on all error
paths. Rework the function to follow the standard libvirt pattern
of using a local 'ret' variable to hold the function return value,
performing all cleanup and returning 'ret' at a 'cleanup' label.

Generates a false positive for Coverity, but it turns out there's no need
to check ret == -1 since if VIR_APPEND_ELEMENT is successful, the local
vol pointer is cleared anyway.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

Found by my Coverity checker - virCheckFlags call could return -1, but
not virCommandFree(destroy_cmd).
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

When parsing the barrier:limit values, use virStringSplitCount in order
to split the pair and make the approriate checks to get the data.
Signed-off-by: NJohn Ferlan <jferlan@redhat.com>

The virHostdevIsVirtualFunction() was called exactly twice, and in
both cases the return value was saved to a temporary variable before
being checked. This would be okay if it improved readability, but in
this case is pretty pointless.

Get rid of the temporary variable and check the return value
directly; while at it, change the check from '<= 0' to '!= 1' to
align it with the way other similar *IsVirtualFunction() functions
are used thorough the code.

virNetDevIsVirtualFunction() returns 1 if the interface is a
virtual function, 0 if it isn't and -1 on error. This means that,
despite the name suggesting otherwise, using it as a predicate is
not correct.

Fix two callers that were doing so adding an explicit check on
the return value.

Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

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

The corresponding chardev must be attached first, otherwise the
the qemu command line won't be complete (missing the host part),

Introduce support for domainInterfaceStats API call for querying
network interface statistics. Consequently it also enables the use of
`virsh domifstat <dom> <interface name>` command plus seeing the
interfaces names instead of "-" when doing `virsh domiflist <dom>`.

After successful guest creation we fill the network interfaces names
based on domain, device id and append suffix if it's emulated in the
following form: vif<domid>.<devid>[-emu].  We extract the network
interfaces info from the libxl_domain_config object in
libxlDomainCreateIfaceNames() to generate ifname. On domain cleanup we
also clear ifname, in case it was set by libvirt (i.e. being prefixed
with "vif"). We also skip these two steps in case the name of the
interface was manually inserted by the administrator. Since the
introduction of netprefix (commit a040ba9e), ifnames with a registered
prefix will be freed on virDomain{Obj,Def}Format*, thus eliminating
the migration issues observed with the reverted commit d2e5538b whereas
source and destination would have the same ifname.

For getting the interface statistics we resort to virNetInterfaceStats
and let libvirt handle the platform specific nits. Note that the
latter is not yet supported in FreeBSD.
Signed-off-by: NJoao Martins <joao.m.martins@oracle.com>

Signed-off-by: NChunyan Liu <cyliu@suse.com>

%zu is not always synonymous with uint64_t; on 32-bit machines,
size_t is only 32 bits.  Prefer "%lld"/'unsigned long long' when
the variable is under our control, and "%"PRIu64 when we are
stuck with 'uint64_t' from RBD.

Fixes errors such as:

../../src/storage/storage_backend_rbd.c: In function 'virStorageBackendRBDVolWipe':
../../src/storage/storage_backend_rbd.c:1281:15: error: format '%zu' expects argument of type 'size_t', but argument 8 has type 'uint64_t {aka long long unsigned int}' [-Werror=format=]
     VIR_DEBUG("Need to wipe %zu bytes from RBD image %s/%s",
               ^
../../src/util/virlog.h:90:73: note: in definition of macro 'VIR_DEBUG_INT'
     virLogMessage(src, VIR_LOG_DEBUG, filename, linenr, funcname, NULL, __VA_ARGS__)
                                                                         ^
../../src/storage/storage_backend_rbd.c:1281:5: note: in expansion of macro 'VIR_DEBUG'
     VIR_DEBUG("Need to wipe %zu bytes from RBD image %s/%s",
     ^
Signed-off-by: NEric Blake <eblake@redhat.com>

There's this check when building command line that whenever
domain has no graphics card configured we put -nographics onto
qemu command line. The check is 'if (!def->graphics)'. This
makes coverity think that def->graphics can be NULL, which is
true. But later in the code every access to def->graphics is
guarded by check for def->ngraphics, so no crash occurs. But this
is something that coverity fails to deduct.
In order to shut coverity up lets change the condition to
'if (!def->ngraphics)'.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

After 6604a3dd in which new helper function has been
introduced, the code calls virStringReplace and dereference the
result immediately. The string function can, however, return NULL
so this would SIGSEGV right away. Check for the return value of
the string function.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

After 457ff97f there are two defects in our code. In both of
them we use a signed variable to hold up a number of snapshots
that domain has. We use a helper function to count the number.
However, the helper function may fail in which case it returns
a negative one and control jumps to cleanup label where an
unsigned variable is used to iterate over array of snapshots. The
loop condition thus compare signed and unsigned variables which
in this specific case ends up badly for us.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

xl/libxl already supports qemu's network-based block backends
such as nbd and rbd. libvirt has supported configuring such
<disk>s for long time too. This patch adds support for rbd
disks in the libxl driver by generating a rbd device URL from
the virDomainDiskDef object. The URL is passed to libxl via the
pdev_path field of libxl_device_disk struct. libxl then passes
the URL to qemu for cosumption by the rbd backend.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

The target= setting in xl disk configuration can be used to encode
meta info that is meaningful to a backend. Leverage this fact to
support qdisk network disk types such as rbd. E.g. <disk> config
such as

   <disk type='network' device='disk'>
     <driver name='qemu' type='raw'/>
     <source protocol='rbd' name='pool/image'>
       <host name='mon1.example.org' port='6321'/>
       <host name='mon2.example.org' port='6322'/>
       <host name='mon3.example.org' port='6322'/>
     </source>
     <target dev='hdb' bus='ide'/>
     <address type='drive' controller='0' bus='0' target='0' unit='1'/>
   </disk>

can be converted to the following xl config (and vice versa)

  disk = [ "format=raw,vdev=hdb,access=rw,backendtype=qdisk,
            target=rbd:pool/image:auth_supported=none:mon_host=mon1.example.org\\:6321\\;mon2.example.org\\:6322\\;mon3.example.org\\:6322"
         ]

Note that in xl disk config, a literal backslash in target= must
be escaped with a backslash. Conversion of <auth> config is not
handled in this patch, but can be done in a follow-up patch.

Also add a test for the conversions.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

The most formal form of xl disk configuration uses key=value
syntax to define each configuration item, e.g.

format=raw, vdev=xvda, access=rw, backendtype=phy, target=disksrc

Change the xl disk formatter to produce this syntax, which allows
target= to contain meta info needed to setup a network-based
disksrc (e.g. rbd, nbd, iscsi). For details on xl disk config
format, see  $xen-src/docs/misc/xl-disk-configuration.txt

Update the disk config in the tests to use the formal syntax.
But add tests to ensure disks specified with the positional
parameter syntax are correctly converted to <disk> XML.
Signed-off-by: NJim Fehlig <jfehlig@suse.com>

Mostly labels names and whitespace.

No functional changes.

It may be useful in some cases to call TristateSwitch helper with TristateBool.
Document that enum values equivalency in the code.
Signed-off-by: NMarc-André Lureau <marcandre.lureau@redhat.com>

In case you will specify graphics like this:

<graphics type='spice' port='-1'/>

or

<graphics type='spice' port='-1' tlsPort='6000'/>

libvirt will automatically add autoport='no'.  This leads to an issue
that in qemuProcessStop() we don't release that port because we are
releasing both port if autoport=yes or only port marked as reserved.

If autoport=no but we request to generate port via '-1' we need to mark
that port as reserved in order to release it.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1299696Signed-off-by: NPavel Hrdina <phrdina@redhat.com>

Checking whether x > 0 before looping over [0..x] items doesn't make
sense and multi-line body must have curly brackets around it.

Best viewed with '-w'.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

This does nothing more than adding the new device and capability.
The device is present since QEMU 2.6.0.
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

There's a check if a domain definition has any graphics card and
if so, we iterate over each one of them. This makes no sense,
because even if it has none we can still iterate over.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

GIC v2 is the default, but checking against that specific version when
we want to know whether the default has been selected is potentially
error prone; using an alias instead makes it safer.

Similarly to VM startup always set the legacy affinity. Additionally we
don't need to report an explicit error since virProcessSetAffinity
reports them themselves.

Similarly to VM startup always set the legacy affinity. Additionally we
don't need to report an explicit error since virProcessSetAffinity
reports them themselves.

PostParse handles it for us now.

This causes some test suite churn; qemu's custom PostParse could is
now invoked before the generic AddImplicitControllers, so PCI
controllers end up sequentially in the XML before the generically
added IDE controllers. So it's just some XML reordering

Everywhere else in qemu driver code 'qemuCaps' is a virQEMUCapsPtr,
and virCapsPtr is generally named just 'caps'. Rename the offenders

Seems like the natural fit, since we are already adding other XML bits
in the PostParse routine.

Previously AddImplicitControllers was only called at the end of XML
parsing, meaning code that builds a DomainDef by hand had to manually
call it. Now those PostParse callers get it for free.

There's some test churn here; xen xm and sexpr test suite bits weren't
calling this before, but now they are, so you'll see new IDE controllers.
I don't think this will cause problems in practice, since the code already
needs to handle these implicit controllers like in the case when a user
defines their own XML.

virDomainObjWait is designed to be called in a loop. Make sure we break
the loop in case the domain dies to avoid waiting for an event which
will never happen.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Calling qemuProcessStop without a job opens a way to race conditions
with qemuDomainObjExitMonitor called in another thread. A real world
example of such a race condition:

  - migration thread (A) calls qemuMigrationWaitForSpice
  - another thread (B) starts processing qemuDomainAbortJob API
  - thread B signals thread A via qemuDomainObjAbortAsyncJob
  - thread B enters monitor (qemuDomainObjEnterMonitor)
  - thread B calls qemuMonitorSend
  - thread A awakens and calls qemuProcessStop
  - thread A calls qemuMonitorClose and sets priv->mon to NULL
  - thread B calls qemuDomainObjExitMonitor with priv->mon == NULL
  => monitor stays ref'ed and locked

Depending on how lucky we are, the race may result in a memory leak or
it can even deadlock libvirtd's event loop if it tries to lock the
monitor to process an event received before qemuMonitorClose was called.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

Stopping a domain without a job risks a race condition with another
thread which started a job a which does not expect anyone else to be
messing around with the same domain object.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>