The function repeatedly checked the first element rather than iterating
through the array.
Reported-by: NDaniel P. Berrangé <berrange@redhat.com>
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>
Tested-by: NDaniel P. Berrangé <berrange@redhat.com>

Allocate space also for the terminating NULL.
Reported-by: NDaniel P. Berrangé <berrange@redhat.com>
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>
Tested-by: NDaniel P. Berrangé <berrange@redhat.com>

Some of the node device APIs are a little odd because they accept a
virNodeDevicePtr object but are still implemented by the virt drivers.
The first thing the virt drivers need to do is get the XML config
associated with the node device, and that means talking to the node
device driver.

This worked previously because with monolithic libvirtd, both the
virt driver and node device driver were in the same daemon and thus
a single virConnectPtr can talk to both drivers.

With the split daemon world though, the virNodeDevicePtr passed into
the APIs is associated with the QEMU driver virConnectPtr, which has
no ability to invoke APIs against the node device driver. We must thus
get a duplicate virNodeDevicePtr object which is associated with a
virConnectPtr for the node device driver.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The node device APIs are a little unusual because we don't use a
"remote_nonnull_node_device" object on the wire, instead we just
have a "remote_string" for the device name. This meant dispatcher
code generation needed special cases. In doing so we mistakenly
used the virNodeDeviceLookupByName() API which gets dispatched
into the driver, instead of get_nonnull_node_device() which
directly populates a virNodeDevicePtr object.

This wasn't a problem with monolithic libvirtd, as the
virNodeDeviceLookupByName() API call was trivially satisfied
by the registered driver, albeit with an extra (undesirable)
authentication check. With the split daemons, the call to
virNodeDeviceLookupByName() fails in virtqemud, because the
node device driver obviously doesn't exist in that daemon.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Despite their names, the following APIs:

    virNodeDeviceDettach
    virNodeDeviceDetachFlags
    virNodeDeviceReAttach
    virNodeDeviceReset

are all handled by the virt drivers, not the node device driver.
A bug in the RPC generator meant that these APIs were sent to
the nodedev driver for handling. This caused breakage with the
split daemons, since nothing was available to process them.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Commit 7b79ee2f makes assumptions about die_id parsing in
the sysfs that aren't true for Power hosts. In both Power8
and Power9, running 5.6 and 4.18 kernel respectively,
'die_id' is set to -1:

$ cat /sys/devices/system/cpu/cpu0/topology/die_id
-1

This breaks virHostCPUGetDie() parsing because it is trying to
retrieve an unsigned integer, causing problems during VM start:

virFileReadValueUint:4128 : internal error: Invalid unsigned integer
value '-1' in file '/sys/devices/system/cpu/cpu0/topology/die_id'

This isn't necessarily a PowerPC only behavior. Linux kernel commit
0e344d8c70 added in the former Documentation/cputopology.txt, now
Documentation/admin-guide/cputopology.rst, that:

  To be consistent on all architectures, include/linux/topology.h
  provides default definitions for any of the above macros that are
  not defined by include/asm-XXX/topology.h:

  1) topology_physical_package_id: -1
  2) topology_die_id: -1
  (...)

This means that it might be expected that an architecture that
does not implement the die_id element will mark it as -1 in
sysfs.

It is not required to change die_id implementation from uInt to
Int because of that. Instead, let's change the parsing of the
die_id in virHostCPUGetDie() to read an integer value and, in
case it's -1, default it to zero like in case of file not found.
This is enough to solve the issue Power hosts are experiencing.

Fixes: 7b79ee2fSigned-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>

During startup the udev node device driver impl uses a background thread
to populate the list of devices to avoid blocking the daemon startup
entirely. There is no synchronization to the public APIs, so it is
possible for an application to start calling APIs before the device
initialization is complete.

This was not a problem in the old approach where libvirtd was started
on boot, as initialization would easily complete before any APIs were
called.

With the use of socket activation, however, APIs are invoked from the
very moment the daemon starts. This is easily seen by doing a

  'virsh -c nodedev:///system list'

the first time it runs it will only show one or two devices. The second
time it runs it will show all devices. The solution is to introduce a
flag and condition variable for APIs to synchronize against before
returning any data.
Reviewed-by: NMichal Privoznik <mprivozn@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Don't rely on error check and assign hostname only when non-NULL.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

If a block-commit fails we should at least re-enable the bitmaps so that
the operation can be re-tried.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Merge the bitmaps into base of the block commit after the job finishes.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Active layer block commit makes the 'base' image the new top image of
the disk after it finishes. This means that all bitmap operations need
to be handled prior to this happening as we'd lose writes otherwise.

The ideal place is to handle it when pivoting to the new image as only
guest-writes would be happening after this point.

Use qemuBlockBitmapsHandleCommitFinish to calculate the merging
transaction.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

On start of the commit job, we need to disable any active bitmap in the
base. Use qemuBlockBitmapsHandleCommitStart to calculate which and call
the appropriate QMP APIs. We use blockdev-reopen to make the 'base'
writable to disable the bitmaps.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Add an argument to qemuBlockJobDiskNewCommit to propagate the list of
disabled bitmaps into the job data structure.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Use the 'snapshots-synthetic-broken' test data for block-commit.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Test handling of more complex cases of merging bitmaps accross
snapshots.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Add code for testing the two necessary steps of handling bitmaps during
block commit and exercise the code on the test data which we have for
bitmap handling.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

qemuBlockBitmapsHandleCommitStart prepares for disabling the bitmaps in
the 'base' of the commit job so that the bitmaps are not dirtied by the
commit job. This needs to be done prior to start of the commit job.

qemuBlockBitmapsHandleCommitFinish then calculates the necessary merges
that agregate all the bitmaps between the commited images and write them
into the base bitmap.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Set the 'id' field of the backing chain properly so that we can look
up images, and initialize 6 images instead of 10 as we don't use more
currently.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Starting a commit job will require disabling bitmaps in the base image
so that they are not dirtied by the commit job. We need to store a list
of the bitmaps so that we can later re-enable them.

Add a field and status XML handling code as well as a test.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

I'll be adding more fields to care about so splitting the code out will
be better long-term.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NPavel Mores <pmores@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

I'll be adding more fields to care about so splitting the code out will
be better long-term.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NPavel Mores <pmores@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Since capabilities are not present for inactive VMs we'd report that we
don't support '--delete' or committing while checkpoints exist rather
than the proper error.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

The code deleting checkpoints needs the name of the parent checkpoint's
disk's bitmap but was using the disk alias instead. This would create
wrong bitmaps after deleting some checkpoints.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Qemu's bitmap APIs don't reopen the appropriate images read-write for
modification. It's libvirt's duty to reopen them via blockdev-reopen
if we wish to modify the bitmaps.

Use the new helpers to reopen the images for bitmap manipulation.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Introduce a set of helpers to call blockdev-reopen in certain scenarios

Libvirt will use the QMP command to turn certain members of the backing
chain read-write for bitmap manipulation and we'll also want to use it
to replace/install the backing chain of a qcow2 format node.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Introduce the monitor code for using blockdev-reopen.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

This capability will be asserted once qemu stabilizes 'blockdev-reopen'.
For now we just add the capability so that we can introduce some code
that will use the reopening call. This will show our willingness to
adopt use of reopen and help qemu developers stabilize it.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Signed-off-by: NGaurav Agrawal <agrawalgaurav@gnome.org>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Signed-off-by: NJán Tomko <jtomko@redhat.com>

Add sample data files for validating handling of a QEMU guest started
with:

  -smp 7,maxcpus=16,sockets=2,dies=2,cores=2,threads=2
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

The logic for querying hotpluggable CPUs needs to sort the list
of CPUs returned by QEMU. Unfortunately our sorting method failed
to use the die_id field, so CPUs were not correctly sorted.

This is seen when configuring a guest with partially populated
CPUs

  <vcpu placement='static' current='1'>16</vcpu>
  <cpu...>
    <topology sockets='4' dies='2' cores='1' threads='2'/>
  </cpu>

Then trying to start it would fail:

  # virsh -c qemu:///system start demo
  error: Failed to start domain demo
  error: internal error: qemu didn't report thread id for vcpu '0'
Reviewed-by: NPeter Krempa <pkrempa@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

Signed-off-by: NJán Tomko <jtomko@redhat.com>
Reported-by: NDr. David Alan Gilbert <dgilbert@redhat.com>

We currently don't model the 'ssh' protocol properties properly and
since it seems impossible for now (agent path passed via environment
variable). To allow libguestfs to work as it used in pre-blockdev era we
must carry the properties over to the command line. For this instance we
just store it internally and format it back.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Add testing of the interpretation of the JSON pseudo-protocol backing
store into JSON structs for blockdev. This will be used to test JSON
pseudo-URIs used by libguestfs while actually also validating the output
against the QMP schema. Since libguestfs uses obsolete/undocumented
values the outputs will differ and a benefit is that modern output is
used now.

The example test case covers the fields and values used by libguestfs
when using the https driver.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

The test was invoking the JSON formatter with the 'legacy' flag thus
formatting bunch of obsolete JSON blockdev definitions. We also should
test the modern ones. Add a boolean and re-run all the tests in both
cases.

Additionally for any modern invocation we should also validate that the
output conforms to the QAPI schema.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Move lookup of the schema root earlier so that multiple functions
can use it for validation.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Multiple tests require the schema. Extract the loading into a separate
variable to avoid issues with ownership of the pointer.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

libguestfs abuses a quirk of qemu's parser to accept also other variants
of the 'sslverify' field which would be valid on the command line but
are not documented in the QMP schema.

If we encounter the 'off' string instead of an boolean handle it rather
than erroring out to continue support of pre-blockdev configurations.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Add support for parsing the recently added fields from backing file
pseudo-protocol strings.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Pass in the correct fields.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>

Pass the alias of the secret object holding the cookie data as
'cookie-secret' to qemu.
Signed-off-by: NPeter Krempa <pkrempa@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>