Replace:
if (virBufferError(&buf)) {
    virBufferFreeAndReset(&buf);
    virReportOOMError();
    ...
}

with:
if (virBufferCheckError(&buf) < 0)
    ...

This should not be a functional change (unless some callers
misused the virBuffer APIs - a different error would be reported
then)

Check if the buffer is in error state and report an error if it is.

This replaces the pattern:
if (virBufferError(buf)) {
    virReportOOMError();
    goto cleanup;
}
with:

if (virBufferCheckError(buf) < 0)
    goto cleanup;

Document typical buffer usage to favor this.
Also remove the redundant FreeAndReset - if an error has
been set via virBufferSetError, the content is already freed.

Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

virFileReadAll already logs an error. If reading the 'speed' file
fails with EINVAL, we log an error even though we ignore it. If it
fails with other errors, we log two errors.

Use virFileReadAllQuiet - ignore EINVAL and report just one error
in other cases.

Fixes this error on libvirtd startup:
2014-06-30 12:47:14.583+0000: 20971: error : virFileReadAll:1297 :
Failed to read file '/sys/class/net/wlan0/speed': Invalid argument

Just like virFileReadAll, but returns -errno instead
of reporting errors. Useful for ignoring some errors.

Rename them to comply with the naming policy.

When CPU comparison APIs return VIR_CPU_COMPARE_INCOMPATIBLE, the caller
has no clue why the CPU is considered incompatible with host CPU. And in
some cases, it would be nice to be able to get such info in a client
rather than having to look in logs.

To achieve this, the APIs can be told to return VIR_ERR_CPU_INCOMPATIBLE
error for incompatible CPUs and the reason will be described in the
associated error message.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

The parent directory doesn't necessarily need to be stored after we
don't mangle the path stored in the image. Remove it and tweak the code
to avoid using it.

Store backing chain paths as non-canonical. The canonicalization step
will be already taken. This will allow to avoid storing unnecessary
amounts of data.

Now that we store only relative names in virStorageSource's member
relPath the backingRelative member is obsolete. Remove it and adapt the
code to the removal.

Due to various refactors and compatibility with the virstoragetest the
relPath field of the virStorageSource structure was always filled either
with the relative name or the full path in case of absolutely backed
storage. Return its original purpose to store only the relative name of
the disk if it is backed relatively and tweak the tests.

This patch introduces a function that will allow us to resolve a
relative difference between two elements of a disk backing chain. This
function will be used to allow relative block commit and block pull
where we need to specify the new relative name of the image to qemu.

This patch also adds unit tests for the function to verify that it works
correctly.

virPortAllocatorSetUsed permits to set a port as already used and
prevent the port allocator to use it without any attempt to bind it.
Signed-off-by: NGiuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

If we are running on a system that is not capable of huge pages (e.g.
because the kernel is not configured that way) we still try to open
"/sys/kernel/mm/hugepages/" which however does not exist. We should
be tolerant to this specific use case.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

On the Linux kernel, if huge pages are allocated the size they cut off
from memory is accounted under the 'MemUsed' in the meminfo file.
However, we want the sum to be subtracted from 'MemTotal'. This patch
implements this feature. After this change, we can enable reporting
of the ordinary system pages in the capability XML:

<capabilities>

  <host>
    <uuid>01281cda-f352-cb11-a9db-e905fe22010c</uuid>
    <cpu>
      <arch>x86_64</arch>
      <model>Haswell</model>
      <vendor>Intel</vendor>
      <topology sockets='1' cores='1' threads='1'/>
      <feature/>
      <pages unit='KiB' size='4'/>
      <pages unit='KiB' size='2048'/>
      <pages unit='KiB' size='1048576'/>
    </cpu>
    <power_management/>
    <migration_features/>
    <topology>
      <cells num='4'>
        <cell id='0'>
          <memory unit='KiB'>4048248</memory>
          <pages unit='KiB' size='4'>748382</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>
        ...
      </cells>
    </topology>
  </host>
</capabilities>

You can see the beautiful thing about this: if you sum up all the
<pages/> you'll get <memory/>.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Introduce a common function that will take a callback to resolve links
that will be used to canonicalize paths on various storage systems and
add extensive tests.

To free string lists with some strings stolen from the middle we need to
walk the complete array. Introduce a new helper that takes the string
list size to free such string lists.

virNumaGetPages calls closedir(dir) in cleanup and dir could
be NULL if we jump there from the failed opendir() call.

While it's not harmful on Linux, FreeBSD libc crashes [1], so
make sure that dir is not NULL before calling closedir.

1: http://lists.freebsd.org/pipermail/freebsd-standards/2014-January/002704.html

One of previous commits (e6258a33) tried to build the huge page code
only on Linux since it's Linux centric indeed. But it failed miserably
as it used 'WITH_LINUX' which is an automake conditional not a gcc
one. In the sources we need to use __linux__.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

ioctl returns -1, not the errno value

Only three other callers possibly call closedir on a NULL argument.
Even though these probably won't be used on FreeBSD where this crashes,
let's be nice and only call closedir on an actual directory stream.

==== Invalid write of size 4
====    at 0x52E678C: virNumaGetDistances (virnuma.c:479)
====    by 0x5396890: nodeCapsInitNUMA (nodeinfo.c:1796)
====    by 0x203C2B: virQEMUCapsInit (qemu_capabilities.c:960)
====  Address 0xe10a1e0 is 0 bytes after a block of size 0 alloc'd
====    at 0x4C2A6D0: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
====    by 0x52A10D6: virAllocN (viralloc.c:191)
====    by 0x52E674D: virNumaGetDistances (virnuma.c:470)
====    by 0x5396890: nodeCapsInitNUMA (nodeinfo.c:1796)
====    by 0x203C2B: virQEMUCapsInit (qemu_capabilities.c:960)

The hugepage sizing and counting code gathers the information from sysfs
and thus isn't portable. Stub it out for non-Linux so that we can report
a better error. This patch also avoids calling sysinfo() on Mingw where
it isn't supported.

Don't return possibly incomplete result if virDirRead fails.

So far, we are doing compile time decisions on which architecture is
used. However, for testing purposes it's much easier if we pass host
architecture as parameter and then let the function decide which code
snippet for extracting host CPU info will be used.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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

It will also be reused later.

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: NMichal Privoznik <mprivozn@redhat.com>

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: NMichal Privoznik <mprivozn@redhat.com>

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

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

These functions will handle PCIe devices and their link capabilities
to query some info about it.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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: NEric Blake <eblake@redhat.com>

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: NMichal Privoznik <mprivozn@redhat.com>

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: NMichal Privoznik <mprivozn@redhat.com>

The purpose of this function is to fetch link state
and link speed for given NIC name from the SYSFS.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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: NEric Blake <eblake@redhat.com>