The qemu JSON monitor test allows to test also expected command
arguments. As the error from the monitor simulator is returned as a
simulated qemu error (in JSON) all other JSON contained in the error
message needs to be escaped. This will happen if the monitor command
under test receives a JSON array as an argument.

This will improve the error message from:
libvirt:  error : internal error: cannot parse json { "error":  { "desc":
"Invalid value of argument 'keys' of command 'send-key': expected 'ble'
got '[{"type":"number","data":43},{"type":"number","data":26},
{"type":"number","data":46},{"type":"number","data":32}]'",
"class": "UnexpectedCommand" } }: lexical error: invalid string in json text.

To:
libvirt: QEMU Driver error : internal error: unable to execute QEMU
command 'send-key': Invalid value of argument 'keys' of command
'send-key': expected 'ble' got '[{"type":"number","data":43},
{"type":"number","data":26},{"type":"number","data":46},
{"type":"number","data":32}]'

This improvement will not have any effect on tests executing as
expected, but it will help test development.

If the leasehelper_path couldn't be found the code would leak the
freshly constructed command structure. Re-arrange code to avoid the
problem.

Found by coverity, broken by baafe668.

virstoragetest now requires parts of the storage driver to be built.
Without this change the test can't be compiled on platforms that don't
build the storage driver (mingw).

make[2]: *** No rule to make target `../src/libvirt_driver_storage_impl.la', needed by `virstoragetest.exe'.  Stop.

Broken by commit 713cc3b0

qemuMonitorJSONSendKey declares the "holdtime" argument as unsigned int
while the command was constructed in qemuMonitorJSONMakeCommand using
the "P" modifier which took a unsigned long from the variable
arguments which then made it possible to access uninitialized memory.

This broke the qemumonitorjsontest on 32bit fedora 20:
64) qemuMonitorJSONSendKey
... libvirt: QEMU Driver error : internal error: unsupported data type 'W' for arg 'WVSƒì ‹D$0è‘wÿÿÃAå' FAILED

Uncovered by upstream commit f744b831.

Additionally add test for the hold-time option.

The 'libxl_domain_config' object is stack allocated which means its
memory contents are undefined. The libxl_domain_config_dispose() call
is only safe if the memory is initialized to a defined state. Not all
code paths which reach libxl_domain_config_dispose() will ensure that
libxl_domain_config_init() is called. Move the libxl_domain_config_init()
call earlier in the function to ensure all codepaths have defined
memory state.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

To allow the test suite to creat the XML option object,
move the virDomainXMLOptionNew call into a libxlCreateXMLConf
method.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

To make it easier to test, change libxlBuildDomainConfig so
that it takes a virPortAllocatorPtr instead of the larger
libxlDriverPrivatePtr object.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

To make it easier to unit test, change libxlBuildDomainConfig
so that it takes 'virDomainDefPtr' and 'libxl_ctx *' objects
as separate parameters.
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>

Some of the APIs already return int since they can produce errors that
need to be propagated. For consistency reasons, this patch changes the
rest of the APIs to also return int even though they do not fail or
report any errors.

In general, we should only remove a backend after seeing DEVICE_DELETED
event for a corresponding frontend.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

In general, we should only remove a backend after seeing DEVICE_DELETED
event for a corresponding frontend. This doesn't make any difference for
disks attached using -drive or drive_add since QEMU automatically
removes their backends but it's still better to make our code
consistent. And it may start making difference in case we switch to
attaching disks using -blockdev.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

[1] reported that we are removing network's backend too early. I didn't
really get the reproducer but libvirt behaves strangely when a guest
does not confirm the removal, e.g., it does not support PCI hotplug. In
such case, detaching a network device leaves its frontend in place but
removes the backend, which makes the device unusable for the guest.
Moreover attaching the same device again succeeds and both the guest and
libvirt will see two network interfaces attached but only one of them is
actually working.

I checked with Paolo Bonzini and he confirmed we should only remove a
backend after seeing DEVICE_DELETED event for a corresponding frontend.

[1] https://www.redhat.com/archives/libvir-list/2014-March/msg01740.htmlSigned-off-by: NJiri Denemark <jdenemar@redhat.com>

All the fields crammed into two lines weren't easy to parse by human
eyes. Split up the format string into lines and put it into a central
variable so that changes in two places aren't necessary.

This patch adds option to specify that a json qemu command argument is
optional without the need to use if's or ternary operators to pass the
list. Additionally all the modifier characters are documented to avoid
user confusion.

To allow using the array manipulation macros on the arrays returned by
virStringSplit we need to know the count of the elements in the array.
Modify virStringSplit to return this value, rename it and add a helper
with the old name so that we don't need to update all the code.

Now we don't need to skip backing chain detection for remote disks.

Use the new backing store parser in the backing chain crawler. This
change needs one test change where information about the NBD image are
now parsed differently.

Add parsers for relative and absolute backing names for local and remote
storage files.

This parser parses relative paths as relative to their parents and
absolute paths according to the protocol or local access.

For remote storage volumes, all URI based backing file names are
supported and for the qemu colon syntax the NBD protocol is supported.

Use virStorageFileAccess() to to check whether the file is accessible in
the main part of the metadata crawler.

Use virStorageFileReadHeader() to read headers of storage files possibly
on remote storage to retrieve the image metadata.

The backend information is now parsed by
virStorageFileGetMetadataInternal which is now exported from the util
source and virStorageFileGetMetadataFromFDInternal now doesn't need to
be exported.

Use the virStorageFileGetUniqueIdentifier() function to get a unique
identifier regardless of the target storage type instead of relying on
canonicalize_path().

A new function that checks whether we support a given image is
introduced to avoid errors for unimplemented backends.

Add a new function wrapper and tweak the storage file backend lookup
function so that it can be used without reporting error. This will be
useful in the metadata crawler code where we need silently break if
metadata retrieval is not supported for the current storage type.

Stat the path of the storage file being tested to set the correct type
into the virStorageSource. This will avoid breaking the test suite when
inquiring metadata of directory paths in the next patches.

When walking the backing chain we previously set the storage type to
_FILE and let the virStorageFileGetMetadataFromFDInternal update it to
the correct type later on.

This patch moves the actual storage type determination to the place
where we parse the backing store name so that the code can later be
switched to use virStorageFileReadHeader() directly.

My future work will modify the metadata crawler function to use the
storage driver file APIs to access the files instead of accessing them
directly so that we will be able to request the metadata for remote
files too. To avoid linking the storage driver to every helper file
using the utils code, the backing chain traversal function needs to be
moved to the storage driver source.

Additionally the virt-aa-helper and virstoragetest programs need to be
linked with the storage driver as a result of this change.

Add a storage driver API equivalent of the access() function.
Implementations for the filesystem and gluster backends are provided.

Different protocols have different means to uniquely identify a storage
file. This patch implements a storage driver API to retrieve a unique
string describing a volume. The current implementation works for local
storage only and returns the canonical path of the volume.

To add caching support the local filesystem driver now has a private
structure holding the cached string, which is created only when it's
initially accessed.

This patch provides the implementation for local files only for start.

It was just very recently that we transfered from:

  enum virSomeEnumName{
      ...
  };

to:
  typedef enum {
      ...
  } virSomeEnumName;

This change requires some code adaptation, which wasn't done for
xenapi driver. With this fix we are able to build again.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

In 9dd02965 the virNumaGetNodeMemory was introduced, however the
comment describing the function mentions virNumaGetNodeMemorySize.
And there's one typo in virNumaIsAvailable() description.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

In "src/conf/domain_conf.h" there are many enum declarations. The
cleanup in this header filer was started, but it wasn't enough and
there are many other files that has enum variables declared. So, the
commit was starting to be big. This commit finish the cleanup in this
header file and in other files that has enum variables, parameters,
or functions declared.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>
Signed-off-by: NEric Blake <eblake@redhat.com>

In "src/conf/domain_conf.h" there are many enumerations (enum)
declarations to be converted as a typedef too. As mentioned before,
it's better to use a typedef for variable types, function types and
other usages. I think this file has most of those enum declarations
at "src/conf/". So, me and Eric Blake plan to keep the cleanups all
over the source code. This time, most of the files changed in this
commit are related to part of one file: "src/conf/domain_conf.h".
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>

In "src/cpu/" there are some enumerations (enum) declarations.
Similar to the recent cleanup to "src/util", "src/conf" and other
directories, it's better to use a typedef for variable types,
function types and other usages. Other enumeration and folders will
be changed to typedef's in the future. Specially, in files that are
in different places of "src/util" and "src/conf". Most of the files
changed in this commit are related to CPU (cpu_map.h) enums.
Signed-off-by: NJulio Faracco <jcfaracco@gmail.com>

Our public free functions explicitly don't accept NULL pointers
(sigh). Therefore, callers must do something like this:

    if (dev)
        virNodeDeviceFree(dev);

And we are not doing that on two places I've found. This leads to
dummy error message thrown by virsh:

    virsh # nodedev-dumpxml nonexistent-device
    error: Could not find matching device 'nonexistent-device'
    error: invalid node device pointer in virNodeDeviceFree
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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

With parallel boot, network addresses might not yet be assigned [1],
but binding to wildcard addresses should work.

For non-wildcard addresses, ADDRCONFIG is still used. Document this
in libvirtd.conf.

[1] http://www.freedesktop.org/wiki/Software/systemd/NetworkTarget/

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

Currently, we don not acquire any job when removing a device after
DEVICE_DELETED event was received from QEMU. This means that if there is
another API running at the time DEVICE_DELETED is delivered and the API
acquired a job, we may happily change the definition of the domain the
API is working with whenever it unlocks the domain object (e.g., to talk
with its monitor). That said, we have to acquire a job before finishing
device removal to make things safe. However, doing so in the main event
loop would cause a deadlock so we need to move most of the event handler
into a separate thread.

Another good reason for both acquiring a job and handling the event in a
separate thread is that we currently remove a device backend immediately
after removing its frontend while we should only remove the backend once
we already received DEVICE_DELETED event. That is, we will have to talk
to QEMU monitor from the event handler.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

If QEMU supports DEVICE_DELETED event, we always call
qemuDomainRemoveDevice from the event handler. However, we will need to
push this call away from the main event loop and begin a job for it (see
the following commit), we need to make sure the device is fully removed
by the original thread (and within its existing job) in case the
DEVICE_DELETED event arrives before qemuDomainWaitForDeviceRemoval times
out.

Without this patch, device removals would be guaranteed to never finish
before the timeout because the could would be blocked by the original
job being still active.
Signed-off-by: NJiri Denemark <jdenemar@redhat.com>

On freebsd there isn't known "setlocale" so we have to include locale.h

Introduce helper program to catch events from dnsmasq and maintain a custom
lease file per network. It supports dhcpv4 and dhcpv6. The file is saved as
"<interface-name>.status".

Each lease contains the following info:
<expiry-time (epoch time)> <mac> <iaid> <ip-address> <hostname> <clientid>

Example of custom leases file content:
[
    {
        "iaid": "1221229",
        "ip-address": "2001:db8:ca2:2:1::95",
        "mac-address": "52:54:00:12:a2:6d",
        "hostname": "Fedora20",
        "client-id": "00:04:1a:c1:d9:6b:5a:0a:e2:bc:f8:4b:1e:37:2e:38:22:55",
        "expiry-time": 1393244216
    },
    {
        "ip-address": "192.168.150.208",
        "mac-address": "52:54:00:11:56:b3",
        "hostname": "Wani-PC",
        "client-id": "01:52:54:00:11:56:b3",
        "expiry-time": 1393244248
    }
]

src/Makefile.am:
   * Add options to compile the helper program

src/network/bridge_driver.c:
   * Introduce networkDnsmasqLeaseFileNameCustom()
   * Invoke helper program along with dnsmasq
   * Delete the .status file when corresponding n/w is destroyed.

src/network/leaseshelper.c
   * Helper program to create the custom lease file

When looking up storage volumes virsh uses multiple lookup steps. Some
of the steps don't require a pool name specified. This resulted into a
possibility that a volume would be part of a different pool than the
user specified:

Let's have a /var/lib/libvirt/images/test.qcow image in the 'default'
pool and a second pool 'emptypool':

Currently we'd return:
  $ virsh vol-info --pool emptypool /var/lib/libvirt/images/test.qcow
  Name:           test.qcow
  Type:           file
  Capacity:       100.00 MiB
  Allocation:     212.00 KiB

After the fix:
 $ tools/virsh vol-info --pool emptypool /var/lib/libvirt/images/test.qcow
 error: Requested volume '/var/lib/libvirt/images/test.qcow' is not in pool 'emptypool'

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