interface: clean up virInterfaceDefDevFormat

This modifies the formatting function of virInterface to be a proper
mirror of the parse function, including the addition of a
"parentIfType" arg so that we can decide whether or not it is
appropriate to emit the elements that are only in toplevel interfaces,
as well as the <link> element (which isn't allowed for bridge
interfaces).

Since the restructuring of the code necessarily changes the order of
some of the elements, some test case data had to be updated.

interface: clean up virInterfaceDefParseXML

the switch cases for the 4 different interface types had repetitive
code which has now been pulled out as common. While touching those
lines, some extra usage of "!= NULL" etc has been eliminated to make
things more compact and inline with current coding practices.

NB: parentIfType == VIR_INTERFACE_TYPE_LAST means that this is a
toplevel interface (not a subordinate of a bridge or bond). Only
toplevel interfaces can have a start mode, mtu, or IP address element.

interface: move parsing of bridge attributes into appropriate function

For some reason the bridge stp mode and delay were put directly into
the "bridge" case of the switch in virInterfaceDefParseXML(), although
they are inside the <bridge> element, and so should be parsed in the
function created for that purpose - virInterfaceBridgeDefFormat().

interface: report link state for bonds and vlans too

The interface state for bonds and vlans does seem to reflect the state
of the underlying physical devices, at least in some cases, so it
makes sense to allow reporting it (netcf now does).

The link state/speed for bridge devices is meaningless though, so we
don't even look for it.

security: Rename virSecurityManagerSetImageLabel to *Disk*

I'm going to add functions that will deal with individual image files
rather than whole disks. Rename the security function to make room for
the new one.

security: Sanitize type of @migrated in virSecurityManagerRestoreAllLabel

Also remove one spurious ATTRIBUTE_UNUSED guarding the @migrated
argument.

util: Don't require full disk definition when getting imagelabels

The image labels are stored in the virStorageSource struct. Convert the
virDomainDiskDefGetSecurityLabelDef helper not to use the full disk def
and move it appropriately.

util: storagefile: Introduce helper to free storage source perms

It will also be reused later.

util: seclabel: Add deep copy function for device labels

storage: encryption: Add deep copy function for storage encryption

libxl: limit support for specifying an interface script

Generally, <interface> ... <script> is only supported for
type='ethernet'.  Due to the long and pervasive use of

  <interface type='bridge'>
    ...
    <script path='foo'/>
  </interface>

in Xen domain configuration, it was agreed to allow the use
of <script> with type='bridge' for backwards compatibility.  See
the following discussion thread

http://www.redhat.com/archives/libvir-list/2013-April/msg00755.html

This patch limits the use of <script> to interface types ethernet
and bridge, raising an unsupported config error if <script> is
specified for all other interface types.

While at it, use VIR_ERR_CONFIG_UNSUPPORTED instead of
VIR_ERR_INTERNAL_ERROR when reporting unsupported interface types.

libxl: support interface type=network

Add support for <interface type='network'> in the libxl driver.

virsh: fix broken code in freepages

Commit 9e3efe53 broke the build under valgrind or clang, by writing
8 bytes through an allocation of 4 bytes.  It also risks multiplication
overflow when mallocing (that's a pervasive problem that needs an
audit in the rest of the code, but we might as well fix this one while
we are here), and had a typo.

* tools/virsh-host.c (cmdFreepages): Avoid integer overflow and
undefined behavior.

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

interface: allow reordering of elements in xml

The interface xml schema was written with strict rules about the
ordering of the elements. This was never intentional, but just due to
omission of <interleave> in the appropriate places. This patch just
adds in <interleave> wherever there is more than one element, and
re-indents everything else appropriately.

docs: fix some typos in formatdomain.html

In section "Block / character devices" of "Host device assignment",
the description of hostdev element has some error:

For a block device, the type should be "storage", not "block";
For a character device, the type should be "misc", not "char".

Signed-off-by: Jincheng Miao <jmiao@redhat.com>

nodeinfo: Implement nodeGetFreePages

And add stubs to other drivers like: lxc, qemu, uml and vbox.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

virsh: Expose virNodeGetFreePages

The new API is exposed under 'freepages' command.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Introduce virNodeGetFreePages

The aim of the API is to get information on number of free pages
on the system. The API behaves similar to the
virNodeGetCellsFreeMemory(). User passes starting NUMA cell, the
count of nodes that he's interested in, pages sizes (yes,
multiple sizes can be queried at once) and the counts are
returned in an array.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

virCaps: expose pages info

There are two places where you'll find info on page sizes. The first
one is under <cpu/> element, where all supported pages sizes are
listed. Then the second one is under each <cell/> element which refers
to concrete NUMA node. At this place, the size of page's pool is
reported. So the capabilities XML looks something like this:

<capabilities>

  <host>
    <uuid>01281cda-f352-cb11-a9db-e905fe22010c</uuid>
    <cpu>
      <arch>x86_64</arch>
      <model>Westmere</model>
      <vendor>Intel</vendor>
      <topology sockets='1' cores='1' threads='1'/>
      ...
      <pages unit='KiB' size='4'/>
      <pages unit='KiB' size='2048'/>
      <pages unit='KiB' size='1048576'/>
    </cpu>
    ...
    <topology>
      <cells num='4'>
        <cell id='0'>
          <memory unit='KiB'>4054408</memory>
          <pages unit='KiB' size='4'>1013602</pages>
          <pages unit='KiB' size='2048'>3</pages>
          <pages unit='KiB' size='1048576'>1</pages>
          <distances/>
          <cpus num='1'>
            <cpu id='0' socket_id='0' core_id='0' siblings='0'/>
          </cpus>
        </cell>
        <cell id='1'>
          <memory unit='KiB'>4071072</memory>
          <pages unit='KiB' size='4'>1017768</pages>
          <pages unit='KiB' size='2048'>3</pages>
          <pages unit='KiB' size='1048576'>1</pages>
          <distances/>
          <cpus num='1'>
            <cpu id='1' socket_id='0' core_id='0' siblings='1'/>
          </cpus>
        </cell>
        ...
      </cells>
    </topology>
    ...
  </host>

  <guest/>

</capabilities>

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

virnuma: Introduce pages helpers

For future work we need two functions that fetches total number of
pages and number of free pages for given NUMA node and page size
(virNumaGetPageInfo()).

Then we need to learn pages of what sizes are supported on given node
(virNumaGetPages()).

Note that system page size is disabled at the moment as there's one
issue connected. If you have a NUMA node with huge pages allocated the
kernel would return the normal size of memory for that node. It
basically ignores the fact that huge pages steal size from the system
memory. Until we resolve this, it's safer to not confuse users and
hence not report any system pages yet.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

nodeinfo: Rename nodeGetFreeMemory to nodeGetMemory

For future work we want to get info for not only the free memory
but overall memory size too. That's why the function must have
new signature too.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

virnuma: Introduce virNumaNodeIsAvailable

Not on all hosts the set of NUMA nodes IDs is continuous. This is
critical, because our code currently assumes the set doesn't contain
holes. For instance in nodeGetFreeMemory() we can see the following
pattern:

    if ((max_node = virNumaGetMaxNode()) < 0)
        return 0;

    for (n = 0; n <= max_node; n++) {
        ...
    }

while it should be something like this:

    if ((max_node = virNumaGetMaxNode()) < 0)
        return 0;

    for (n = 0; n <= max_node; n++) {
        if (!virNumaNodeIsAvailable(n))
            continue;
        ...
    }

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

virsh: expose new active commit controls

Add knobs to virsh to manage a 2-phase active commit of the top
layer, similar to knobs already present on blockcopy.  While this
code will fail until later patches actually implement the new
knobs in the qemu driver, doing it now proves that the API is
usable and also makes it easier for testing the qemu changes as
they are made.

* tools/virsh-domain.c (cmdBlockCommit): Add --active, --pivot,
and --keep-overlay options, modeled after blockcopy.
(blockJobImpl): Support --active flag.
* tools/virsh.pod (blockcommit): Document new flags.
(blockjob): Mention 2-phase commit interaction.

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

blockjob: use stable disk string in job event

When the block job event was first added, it was for block pull,
where the active layer of the disk remains the same name.  It was
also in a day where we only cared about local files, and so we
always had a canonical absolute file name.  But two things have
changed since then: we now have network disks, where determining
a single absolute string does not really make sense; and we have
two-phase jobs (copy and active commit) where the name of the
active layer changes between the first event (ready, on the old
name) and second (complete, on the pivoted name).

Adam Litke reported that having an unstable string between events
makes life harder for clients.  Furthermore, all of our API that
operate on a particular disk of a domain accept multiple strings:
not only the absolute name of the active layer, but also the
destination device name (such as 'vda').  As this latter name is
stable, even for network sources, it serves as a better string
to supply in block job events.

But backwards-compatibility demands that we should not change the
name handed to users unless they explicitly request it.  Therefore,
this patch adds a new event, BLOCK_JOB_2 (alas, I couldn't think of
any nicer name - but at least Migrate2 and Migrate3 are precedent
for a number suffix).  We must double up on emitting both old-style
and new-style events according to what clients have registered for
(see also how IOError and IOErrorReason emits double events, but
there the difference was a larger struct rather than changed
meaning of one of the struct members).

Unfortunately, adding a new event isn't something that can easily
be broken into pieces, so the commit is rather large.

* include/libvirt/libvirt.h.in (virDomainEventID): Add a new id
for VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2.
(virConnectDomainEventBlockJobCallback): Document new semantics.
* src/conf/domain_event.c (_virDomainEventBlockJob): Rename field,
to ensure we catch all clients.
(virDomainEventBlockJobNew): Add parameter.
(virDomainEventBlockJobDispose)
(virDomainEventBlockJobNewFromObj)
(virDomainEventBlockJobNewFromDom)
(virDomainEventDispatchDefaultFunc): Adjust clients.
(virDomainEventBlockJob2NewFromObj)
(virDomainEventBlockJob2NewFromDom): New functions.
* src/conf/domain_event.h: Add new prototypes.
* src/libvirt_private.syms (domain_event.h): Export new functions.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Generate two
different events.
* src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Likewise.
* src/remote/remote_protocol.x
(remote_domain_event_block_job_2_msg): New struct.
(REMOTE_PROC_DOMAIN_EVENT_BLOCK_JOB_2): New RPC.
* src/remote/remote_driver.c
(remoteDomainBuildEventBlockJob2): New handler.
(remoteEvents): Register new event.
* daemon/remote.c (remoteRelayDomainEventBlockJob2): New handler.
(domainEventCallbacks): Register new event.
* tools/virsh-domain.c (vshEventCallbacks): Likewise.
(vshEventBlockJobPrint): Adjust client.
* src/remote_protocol-structs: Regenerate.

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

Fix xmconfigtest

Commit ac63014c introduced a regression in the conversion of Xen
xm config to XML by emitting an empty <cmdline>.  Prior to this
commit, <cmdline> was omitted if the xm config was missing (or
contained an empty) 'extra='.

xen: handle root= in xen-xm configuration files.

In addition to extra= xm supported a root= option which was supposed
to be incorporated into the final command line. Handle that for "virsh
domxml-from-native xen-xm". Tested with the libxl backend.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>

vbox_snapshot_conf: fix wrong use of 'xmlSaveFormatFileEnc'

The function 'xmlSaveFormatFileEnc' has a last option to set
if you want to format dumped xml with whitespaces or not.

Older libxml2, the one used in RHEL6, take this option as it is
but newer libxml2 check this option if it's true or not. This
small difference somehow makes things messy on RHEL6 and generated
xml had extra new line and extra whitespaces.

We should pass 1 instead if -1 because the -1 confuses the libxml2.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

tests: fix vbox snapshot xmls

On RHEL6 the vboxsnapshotxmltest fails because of wrong xml that
is generated by libvirt. However the core issue is in the xml data
itself with the wrong indentation.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

virsh: man: Correctly spell QEMU

s/QEemu/QEMU/g

virsh: man: Fix examples and docs for virsh version

The version command now takes the --daemon parameter for a while and the
output example was pretty outdated.

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

virt-aa-helper: allow access to /dev/vhost-net if needed

Only allow the access if it is a KVM domain which has a NIC which wants
non-userspace networking.

This addresses https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1322568

Signed-off-by: Serge Hallyn <serge.hallyn@ubuntu.com>

Added example script on how to convert LXC container config

vbox: fix a segfault when taking a snapshot

there is a segfault in the vbox driver when taking a snapshot in the
following functions:
- vboxDomainGetXMLDesc
- vboxSnapshotGetReadWriteDisks
- vboxSnapshotGetReadOnlyDisks

The virStorageSourcePtr in virDomainDiskDef was not correctly allocated.

(The problem stems from the fact that commit 4dc5d8f and commit bc3f5f1
were written in one order but applied in another; so each tested in
isolation passed, but the combination introduces the problem due to
changed semantics).

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

vbox: snapshot: Avoid memleaks in functions dealing with disk arrays

In virVBoxSnapshotConfRemoveFakeDisks and
virVBoxSnapshotConfDiskIsInMediaRegistry the disk array constructed from
all the disks would be leaked at the end of the function and on
allocation errors. Also the temporary disk list would be leaked.

Add a cleanup section and free the memory properly.

Found by coverity.

vbox: snapshot: Avoid memleak in virVBoxSnapshotConfAllChildren

On re-allocation failure the function would leak already allocated
memory.

uuid: Fix coverity warning of unchecked return value

Coverity checks for patterns of handling return values of functions.
Some recent addition must have tripped a threshold where coverity now
complains that we usually check the return value of virUUIDGenerate but
don't do it in one place. Add a check to make coverity happy.

network: bridge: Avoid freeing uninitialized pointer on cleanup path

The cleanup path in networkBuildDhcpDaemonCommandLine could cause a
crash by freeing uninitialized pointer.

net: leaseshelper: Refactor copying of old entries to avoid double free

When copying entries from the old lease file into the new array the old
code would copy the pointer of the json object into the second array
without removing it from the first. Afterwards when both arrays were
freed this might lead to a crash due to access of already freed memory.

Refactor the code to use the new array item stealing helper added to the
json code so that the entry resides just in one array.

net: leaseshelper: Ignore corrupted lease file and rewrite it

Instead of reporting an error and terminating, rewrite the file with
the newly learned info.

net: leaseshelper: Don't crash if DNSMASQ doesn't provide lease expiry

The value is provided via environment and causes a crash if not defined.

util: json: Add helpers for manipulating json arrays

Add a checker to determine whether a JSON object is an array and a
helper to steal objects from a JSON array.

util: json: Unify function header formatting

Use consistent formatting of function headers:
- two newlines separating functions
- function return type on separate line
- one argument per line

blockjob: don't remove older-style mirror XML

Commit 7c6fc39 introduced a regression in the XML produced for older
clients.  The argument at the time was that clients shouldn't be
depending on output-only data for something that is only going to
be triggered for a transient guest; but John Ferlan reported that
the automated testsuite was such a client.  It's better to be safe
than sorry by guaranteeing back-compat cruft.  Note that later
patches will be using <mirror> for active block commit, but there
we don't have to worry about back-compat.

* src/conf/domain_conf.c (virDomainDiskDefFormat): Restore old
style output when necessary.
* docs/schemas/domaincommon.rng: Validate back-compat style.
* docs/formatdomain.html.in: Update the documentation.
* tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml:
Update tests.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise.

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

blockjob: avoid compiler uncertainty in info sizing

We have a policy of avoiding enum types in structs in our public
API, because it is possible for a client to choose compiler options
that can change the in-memory ABI of that struct based on whether
the enum value occupies an int or a minimal size.  But we missed
this for virDomainBlockJobInfo.  We got lucky on little-endian
machines - if the enum fits minimal size (a char), we still end
up padding to the next long before the next field; but on
big-endian, a client interpreting the enum as a char would always
see 0 when the server supplies contents as an int.

* include/libvirt/libvirt.h.in (virDomainBlockJobInfo): Enforce
particular sizing.

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

blockjob: document recent job addition

I noticed that the web page lacked documentation on block jobs:
http://libvirt.org/html/libvirt-libvirt.html#virDomainBlockJobType
not only for the recently added active commit, but also for all
the other job types.

* include/libvirt/libvirt.h.in (virDomainBlockJobType): Document
recent addition.

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

Increase the size of REMOTE_MIGRATE_COOKIE_MAX to REMOTE_STRING_MAX

During guest migration, if the domain xml is bigger than 16384 which is
easily possible for a guest with good number of disks, message encode fails
for xdr_remote_domain_migrate_perform3_ret().
So, Increase the COOKIE_MAX to STRING_MAX value.

Signed-off-by: Shivaprasad G Bhat <shivaprasadbhat@gmail.com>

nodedev: Introduce <pci-express/> to PCI devices

This new element is there to represent PCI-Express capabilities
of a PCI devices, like link speed, number of lanes, etc.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

virpci: Introduce virPCIDeviceIsPCIExpress and friends

These functions will handle PCIe devices and their link capabilities
to query some info about it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

blockcommit: require base below top

The block commit code looks for an explicit base file relative
to the discovered top file; so for a chain of:
  base <- snap1 <- snap2 <- snap3
and a command of:
  virsh blockcommit $dom vda --base snap2 --top snap1
we got a sane message (here from libvirt 1.0.5):
error: invalid argument: could not find base 'snap2' below 'snap1' in chain for 'vda'

Meanwhile, recent refactoring has slightly reduced the quality of the
libvirt error messages, by losing the phrase 'below xyz':
error: invalid argument: could not find image 'snap2' in chain for 'snap3'

But we had a one-off, where we were not excluding the top file
itself in searching for the base; thankfully qemu still reports
the error, but the quality is worse:
  virsh blockcommit $dom vda --base snap2 --top snap2
error: internal error unable to execute QEMU command 'block-commit': Base '/snap2' not found

Fix the one-off in blockcommit by changing the semantics of name
lookup - if a starting point is specified, then the result must
be below that point, rather than including that point.  The only
other call to chain lookup was blockpull code, which was already
forcing the lookup to omit the active layer and only needs a
tweak to use the new semantics.

This also fixes the bug exposed in the testsuite, where when doing
a lookup pinned to an intermediate point in the chain, we were
unable to return the name of the parent also in the chain.

* src/util/virstoragefile.c (virStorageFileChainLookup): Change
semantics for non-NULL startFrom.
* src/qemu/qemu_driver.c (qemuDomainBlockJobImpl): Adjust caller,
to keep existing semantics.
* tests/virstoragetest.c (mymain): Adjust to expose new semantics.

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

storage: better tests of lookup

Add some more tests of what happens when we restrict a lookup
to begin at a point in the middle of a chain.  In particular,
we want to ensure that a parent is not found when starting at
the child.  This commit also demonstrates that we have a slight
difference in behavior on what parent we report when filtering
is in effect; as the determination of the parent affects the
code in block commit, exposing this in the testsuite will help
justify changes in future patches that tweak the semantics of
what lookups are allowed.

* tests/virstoragetest.c (testStorageLookup): Test user input.
(TEST_LOOKUP_TARGET): Add parameter.
(mymain): Add lookup tests.

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

storage: renumber lookup tests

The next patch will be adding tests, including adding a parameter
for testing more conditions.  For ease of review of that patch, I
want to create common context lines that don't change when the new
tests are added (it's easier to visually review additions than it
is to review an entire chunk of tests rewritten into another
larger chunk of tests).

* tests/virstoragetest.c (mymain): Add a parameter and renumber
the lookup tests.

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

storage: add alias for less typing

Typing chain->backingStore->backingStore gets old after a while;
introduce some alias variables to make the test more compact.

* tests/virstoragetest.c (mymain): Introduce some shorthand.

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

qemu: snapshot: Don't mark all block disks for metadata reuse

For block devices used as snapshot source the new snapshot code would
set the reuse flag. This inhibits to take snapshot without specially
preparing the block image before taking the snapshot.

Fortunately this is not a regression as only the new way of specifying
snapshot source is affected.

For the followin snapshot XML:
 <domainsnapshot>
   <disks>
     <disk name='vda' type='block'>
       <driver type='qcow2'/>
       <source dev="/dev/andariel/testsnap" />
     </disk>
   </disks>
 </domainsnapshot>

You'd get:
error: internal error: unable to execute QEMU command 'transaction': Image is not in qcow2 format

After this patch the snapshot is created successfully.

leaseshelper: fix another crash

We create a 'lease_new' when we are adding new lease entry, then later
in the code we add the 'lease_new' into a 'leases_array_new' which
leads into the crash because we double free the 'lease_new'.

To prevent the double free we set the 'lease_new' to NULL after
successful append into the 'leases_array_new'.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

bhyve: silent destroy command errors on cleanup

When virBhyveProcessStart() fails, it tries to unload
a guest that could have been already loaded using
bhyveload(8) to make sure not to leave it hanging in memory.

However, we could fail before loading a VM into memory,
so 'bhyvectl --destroy' command will fail and print
an error message that looks confusing to users.

So ignore errors when running this in cleanup.

bhyve: do not cleanup unallocated networks on fail

virBhyveProcessStart() calls bhyveNetCleanup() if it fails. However,
it might fail earlier than networks are allocated, so modify
bhyveNetCleanup() to check if net->ifname is not NULL before
going further with the cleanup.

bhyve: fix crash in bhyveBuildNetArgStr

bhyveBuildNetArgStr() calls virNetDevTapCreateInBridgePort() and
passes tapfd = NULL, but tapfdSize = 1. That is wrong, because
if virNetDevTapCreateInBridgePort() crashes after successfully
creating a TAP device, it'll jump to 'error' label, that
loops over tapfd and calls VIR_FORCE_CLOSE:

   for (i = 0; i < tapfdSize && tapfd[i] >= 0; i++)

In that case we get a segfault.

As the bhyve code doesn't use tapfd, pass NULL and set tapfdSize to 0.

storage: report VIR_ERR_NO_STORAGE_VOL when the file doesn't exist

Report VIR_ERR_NO_STORAGE_VOL instead of a system error when lstat
fails because the file doesn't exist.

Fixes this problem in virt-install:

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

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

bhyve: implement PCI address allocation

Automatically allocate PCI addresses for devices instead
of hardcoding them in the driver code. The current
allocation schema is to dedicate an entire slot for each devices.

Also, allow having arbitrary number of devices.

virNetDevGetLinkInfo: Don't report link speed if NIC's not up

The kernel's more broken than one would think. Various drivers report
various (usually spurious) values if the interface is in other state
than 'up' . While on some we experience -EINVAL when read()-ing the
speed sysfs file, with other drivers we might get anything from 0 to
UINT_MAX. If that's the case it's better to not report link speed.
Well, the interface is not up anyway.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

tests: virstoragetest: Fix output when hitting errors

When the test is failing but the debug output isn't enabled the
resulting line would look ugly like and would not contain the actual
difference.

TEST: virstoragetest
      .................chain member 1!chain member 1!chain member 1!

Store the member index in the actual checked string to hide this problem

blockcommit: update error messages related to block jobs

A future patch will add two-phase block commit jobs; as the
mechanism for managing them is similar to managing a block copy
job, existing errors should be made generic enough to occur
for either job type.

* src/conf/domain_conf.c (virDomainHasDiskMirror): Update
comment.
* src/qemu/qemu_driver.c (qemuDomainDefineXML)
(qemuDomainSnapshotCreateXML, qemuDomainRevertToSnapshot)
(qemuDomainBlockJobImpl, qemuDomainBlockCopy): Update error
message.
* src/qemu/qemu_hotplug.c (qemuDomainDetachDiskDevice): Likewise.

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

virsh: improve blockcopy UI

Peter's review of an early version of my addition of active block
commit pointed out some issues that I was copying from the block
copy code; fix them up now before perpetuating them.

For virsh commands that manage a single API call, it's nice to have
a 1:1 mapping of options to flags, so that we can test that
lower-layer software handles flag combinations correctly.  But where
virsh is introducing syntactic sugar to combine multiple API calls
into a single user interface, we might as well make that interface
compact.  That is, we should allow the shorter command-line of
'blockcopy $dom $disk --pivot' without having to explicitly specify
--wait, because this isn't directly a flag passed to a single
underlying API call.

Also, my use of embedded ?: ternaries bordered on unreadable.

* tools/virsh-domain.c (cmdBlockCopy): Make --pivot, --finish,
and --timeout imply --wait. Drop excess ?: operators.
* tools/virsh.pod (blockcopy): Update documentation.

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

virNodeDevCapPCIDevParseXML: Initialize numa_node variable

With one of my recent patches (1c70277) libvirt's capable of
reporting NUMA node locality for PCI devices. The node ID is
stored in pci_dev.numa_node variable. However, since zero is
valid NUMA node ID, the default is -1 as it is in kernel too.
So, if the PCI device is not tied to any specific NUMA node, the
default is then NOT printed into XML. Therefore, when parsing
node device XML, the <node/> element is optional. But currently,
if it's not there, we must set sane default, otherwise after
parsing in the memory representation doesn't match the XML. We
are already doing this in other place: udevProcessPCI().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

blockcommit: fix regression with explicit top argument

Commit f586965 accidentally changed the semantics of the
virDomainBlockCommit command; where it previously looked for
an explicit top argument from the top of the chain, it now
starts from the backing file of the top.  Of course, until
we allow active commits, the only difference it makes is in
the quality of the error message, but with code for active
commit coming soon, we need to support an explicit mention
of the active layer.

* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Start looking
from top of chain.

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

virsh: forbid negative vcpu argument to vcpupin

The vcpupin command allowed specifying a negative number for the --vcpu
argument. This would the overflow when the underlying virDomainPinVcpu
API was called.

 $ virsh vcpupin r7 -1 0
 error: numerical overflow: input too large: 4294967295

Switch the vCPU variable to a unsigned int and parse it using the
corresponding function.

Also improve the vcpupin test to cover all the defects.

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

Signed-off-by: Jincheng Miao <jmiao@redhat.com>
Signed-off-by: Peter Krempa <pkrempa@redhat.com>

virsh: Reject negative numbers in vshCommandOptULongLong

To follow the new semantics of the vshCommandOptToU* functions convert
this one to reject negative numbers too. To allow using -1 for "maximum"
semantics for the vol-*load two bandwidth functions that use this helper
introduce vshCommandOptULongLongWrap.

virsh: Reject negative numbers in vshCommandOptUL

To follow the new semantics of the vshCommandOptToU* functions convert
this one to reject negative numbers too. To allow using -1 for "maximum"
semantics for the two bandwidth functions that use this helper introduce
vshCommandOptULWrap. Although currently the migrate-setspeed function
for the qemu driver will reject -1 as maximum.

virsh: Reject negative numbers in vshCommandOptUInt

Use virStrToLong_uip instead of virStrToLong_ui to reject negative
numbers in the helper. None of the callers expects the wraparound
"feature" for negative numbers.

Also add a function that allows wrapping of negative numbers as it might
be used in the future and be explicit about the new semantics in the
function docs.

Fix crash when saving a domain with type none dac label

qemuDomainGetImageIds did not check if there was a label
in the seclabel, thus crashing on
<seclabel type='none' model='dac'/>

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

vbox_snapshot_conf: Resolve Coverity warnings

Clean up some Coverity warnings from commit id '4dc5d8f1'

vbox_temp: Resolve Coverity warnings

Clean up code to resolve Coverity RESOURCE_LEAK's from commit id's
'632b9600' and 'b739f807'.

security: Don't skip labelling for network disks

A network disk might actually be backed by local storage. Also the path
iterator actually handles networked disks well now so remove the code
that skips the labelling in dac and selinux security driver.

storage: volume: Rework lookup of volume objects

Add a helper to do all the lookup steps and remove a ton of duplicated
code.

storage: Clean up unlocking of storage pool objects

Most of the APIs now don't reach the cleanup section when the pool
object wasn't found and thus don't need to check before unlocking it.

storage: pool: Fix handling of errors on pool lookup failure

Rework internal pool lookup code to avoid printing the raw UUID buffer
in the case a storage pool can't be found:

 $ virsh pool-name e012ace0-0460-5810-39ef-1bce5fa5a4dd
 error: failed to get pool 'e012ace0-0460-5810-39ef-1bce5fa5a4dd'
 error: Storage pool not found: no storage pool with matching uuid à¬à`X9ï_¥¤Ý

The rework is mostly done by switching the lookup code to the newly
introduced helper virStoragePoolObjFromStoragePool

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

build: prefer -fstack-protector-strong to -all

Try -fstack-protector-strong first on Linux. If that fails,
fall back to -fstack-protector-all.

build: remove ssp-buffer-size

This option only makes sense for -fstack-protector.
With -fstack-protector-all or -fstack-protector-strong,
functions are protected regardless of buffer size.

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

build: remove duplicit warning suppression

These warnings have already been added to $dontwarn.

libxl: Resolve Coverity warnings

Resolve two Coverity issues introduced by commit id '9b8d6e1e'

Signed-off-by: John Ferlan <jferlan@redhat.com>

virsh: include bhyve in virsh -V output

Add 'Bhyve' in hypervisor list reported by 'virsh -V'
if it's compiled it.

leaseshelper: fix crash

Commit baafe668 introduced new leaseshelper with a crash of freeing
env string. Calling 'getenv()' inside 'virGetEnvAllowSUID()' may
return a static string and we definitely should not free it.

The author probably want to free the copy of that string.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

maint: exempt graphic binaries from syntax check

Roman Bogorodskiy reported a syntax-check failure when using
FreeBSD; complaining that:

prohibit_empty_first_line
tools/libvirt_win_icon_16x16.ico:1:
tools/libvirt_win_icon_32x32.ico:1:
tools/libvirt_win_icon_48x48.ico:1:
tools/libvirt_win_icon_64x64.ico:1:
maint.mk: Prohibited empty first line

In reality, the first 'line' of that file is NOT empty; but since
it is a binary file, awk is not required to handle it gracefully.
The simplest solution is to exempt all image files from syntax
checks in the first place - after all, we only store them in git
because they are inconvenient to regenerate, but they are not our
preferred format for making modifications, and syntax check should
only cover files that we are likely to modify.

* cfg.mk (VC_LIST_ALWAYS_EXCLUDE_REGEX): Exempt images.
(exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF): Simplify.
(exclude_file_name_regexp--sc_trailing_blank): Likewise.

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

qemu: Properly label FDs when restoring domain with static label

When saving domain with relabel=no, the file that gets created must have the
context set anyway.  That way restore can be successful without the need of
relabelling the file.

Signed-off-by: Shivaprasad G Bhat <sbhat@linux.vnet.ibm.com>

vmware: make version parsing more robust

Since commit d69415d4, vmware version is parsed from both stdout and
stderr. This patch makes version parsing work even if there is garbage
(libvirt debug messages for example) in the command output.

Add test data for this case.

virnetdev: Use ifname in virNetDevGetLinkInfo

If we're compiling on non-Linux platform, the virNetDevGetLinkInfo()
is a dummy function which barely logs debug message that getting link
info is not supported. However, while the debug message was prepared
for printing the interface name too, I actually forgot to pass the
variable which resulted in build error on platforms like mingw or
FreeBSD.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

node_device: Expose link state & speed

While exposing the info under <interface/> in previous patch works, it
may work only in cases where interface is configured on the host.
However, orchestrating application may want to know the link state and
speed even in that case. That's why we ought to expose this in nodedev
XML too:

virsh # nodedev-dumpxml net_eth0_f0_de_f1_2b_1b_f3
<device>
  <name>net_eth0_f0_de_f1_2b_1b_f3</name>
  <path>/sys/devices/pci0000:00/0000:00:19.0/net/eth0</path>
  <parent>pci_0000_00_19_0</parent>
  <capability type='net'>
    <interface>eth0</interface>
    <address>f0:de:f1:2b:1b:f3</address>
    <link speed='1000' state='up'/>
    <capability type='80203'/>
  </capability>
</device>

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

interface_backend_udev: Implement link speed & state

In the previous commit the helper function was prepared, so now
we can wire it up and benefit from it. The Makefile change is
required because we're including virnedev,h which includes
virnetlink.h which tries to include netlink/msg.h. However this
file is not under /usr/include directly but is dependent on libnl
used.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

virnetdev: Introduce virNetDevGetLinkInfo

The purpose of this function is to fetch link state
and link speed for given NIC name from the SYSFS.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

virInterface: Expose link state & speed

Currently it is not possible to determine the speed of an interface
and whether a link is actually detected from the API. Orchestrating
platforms want to be able to determine when the link has failed and
where multiple speeds may be available which one the interface is
actually connected at. This commit introduces an extension to our
interface XML (without implementation to interface driver backends):

  <interface type='ethernet' name='eth0'>
    <start mode='none'/>
    <mac address='aa:bb:cc:dd:ee:ff'/>
    <link speed='1000' state='up'/>
    <mtu size='1492'/>
    ...
  </interface>

Where @speed is negotiated link speed in Mbits per second, and state
is the current NIC state (can be one of the following:  "unknown",
"notpresent", "down", "lowerlayerdown","testing", "dormant", "up").

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

nodeinfo: avoid uninitialized variable on error

Commit 8ba0a58 introduced a compiler warning that I hit during
a run of ./autobuild.sh:

../../src/nodeinfo.c: In function 'nodeCapsInitNUMA':
../../src/nodeinfo.c:1853:43: error: 'nsiblings' may be used uninitialized in this function [-Werror=maybe-uninitialized]
         if (virCapabilitiesAddHostNUMACell(caps, n, memory,
                                           ^

Sure enough, nsiblings starts uninitialized, and is set by a call
to virNodeCapsGetSiblingInfo, but that function fails to assign
through the pointer if virNumaGetDistances fails.

* src/nodeinfo.c (nodeCapsInitNUMA): Initialize nsiblings.

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

storage: fix memory leak with encrypted images

Jim Fehlig reported a regression found by libvirt-TCK tests:

> ~ # perl /usr/share/libvirt-tck/tests/qemu/100-disk-encryption.t
...
> ok 4 - defined persistent domain config
> # Starting inactive domain config
> libvirt error code: 1, message: internal error: unable to execute QEMU command
> 'cont': 'drive-ide0-0-1'
> (/var/cache/libvirt-tck/300-disk-encryption/demo.qcow2) is encrypted

Commit 2279d560 converted a boolean into a pointer with the intent of
transferring that pointer out of a temporary object into the caller's
data structure.  The temporary structure meant that meta->encryption
was always NULL on entry, so we could get away with blindly allocating
the pointer when the header said so.  But later, commit 8823272d
tweaked things to do backing chain detection in-place, rather than via
a temporary object; this has the net result that meta->encryption can
be non-NULL on entry.  Not only did this turn the latent behavior into
a memory leak, it is also a behavior regression: blindly allocating a
new pointer wipes out what secrets we already knew about the chain,
making it impossible to restart the domain.

Of course, no one in their right mind should be relying on qcow2
encryption - it is fundamentally flawed.  And sadly, the TCK tests
don't get run often enough, and this shows that our virstoragetest
does not exercise encrypted images at all.  Otherwise, we could
have avoided a release containing this regression.

* src/util/virstoragefile.c (virStorageFileGetMetadataInternal):
Don't nuke an already-existing encryption.

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

vbox: fix compilation error

clang complains about possibly uninitialized variable:

vbox/vbox_snapshot_conf.c:1355:9: error: variable 'ret' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
    if (!(xPathContext = xmlXPathNewContext(xml))) {
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

So init 'ret' with NULL.

virsh: Add details about specified migration host

the 'migration_host' description may be a bit difficult to
understand for some users, so enhance the manual

Signed-off-by: Chen Fan <chen.fan.fnst@cn.fujitsu.com>
Signed-off-by: Eric Blake <eblake@redhat.com>

blockcommit: document semantics of committing active layer

Now that qemu 2.0 allows commit of the active layer, people are
attempting to use virsh blockcommit and getting into a stuck
state, because libvirt is unprepared to handle the two-phase
commit required by qemu.

Stepping back a bit, there are two valid semantics for a
commit operation:

1. Maintain a 'golden' base, and a transient overlay. Make
changes in the overlay, and if everything appears to work,
commit those changes into the base, but still keep the overlay
for the next round of changes; repeat the cycle as desired.

2. Create an external snapshot, then back up the stable state
in the backing file. Once the backup is complete, commit the
overlay back into the base, and delete the temporary snapshot.

Since qemu doesn't know up front which of the two styles is
preferred, a block commit of the active layer merely gets
the job into a synchronized state, and sends an event; then
the user must either cancel (case 1) or complete (case 2),
where qemu then sends a second event that actually ends the
job.  However, until commit e6bcbcd, libvirt was blindly
assuming the semantics that apply to a commit of an
intermediate image, where there is only one sane conclusion
(the job automatically ends with fewer elements in the chain);
and getting stuck because it wasn't prepared for qemu to enter
a second phase of the job.

This patch adds a flag to the libvirt API that a user MUST
supply in order to acknowledge that they will be using two-phase
semantics.  It might be possible to have a mode where if the
flag is omitted, we automatically do the case 2 semantics on
the user's behalf; but before that happens, I must do additional
patches to track the fact that we are doing an active commit
in the domain XML.  Later patches will add support of the flag,
and once 2-phase semantics are working, we can then decide
whether to relax things to allow an omitted flag to cause an
automatic pivot.

* include/libvirt/libvirt.h.in (VIR_DOMAIN_BLOCK_COMMIT_ACTIVE)
(VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT): New enums.
* src/libvirt.c (virDomainBlockCommit): Document two-phase job
when committing active layer, through new flag.
(virDomainBlockJobAbort): Document that pivot also occurs after
active commit.
* tools/virsh-domain.c (vshDomainBlockJob): Cover new job.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Explicitly
reject active copy; later patches will add it in.

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

docs: fix a typo in hacking.html.in

Signed-off-by: Wang Rui <moon.wangrui@huawei.com>
Signed-off-by: Eric Blake <eblake@redhat.com>

vbox_tmpl.c: Add function for undefining snapshot

All snapshots information will be deleted from the vbox XML, but
differencing disks will be kept so the user will be able to redefine the
snapshot.

vbox_tmpl.c: Patch for redefining snapshots

The machine is unregistered and its vbox XML file is changed in order to
add snapshot information. The machine is then registered with the
snapshot to redefine.

Add vbox_snapshot_conf struct

This structure contains the data to be saved in the VirtualBox XML file
and can be manipulated with severals exposed functions.
The structure is created by vboxSnapshotLoadVboxFile taking the
machine XML file.
It also can rewrite the XML by using vboxSnapshotSaveVboxFile.

vbox_tmpl.c: Better XML description for snapshots

It will be needed for the future patches because we will
redefine snapshots