Luckily, the function returns only 0 or -1 so all the checks work
as expected. Anyway, our rule is that a positive value means
success so if the function ever returns a positive value these
checks will fail. Make them check for a negative value properly.

At the same time fix qemuDomainDetachExtensionDevice() reval
check. It is somewhat related to the aim of this patch.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>

The qemuFirmwareFetchConfigs() function is supposed to fetch all
firmware descriptions from paths defined by firmware.json
specification. This includes user's $HOME directory. However, it
was agreed that if libvirtd is running as privileged user then
his $HOME is ignored (thus $HOME is included in the search only
for regular users). Well, I got the condition wrong - it should
have been reversed.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NAndrea Bolognani <abologna@redhat.com>

With only a couple minor tweaks, we can make the existing
doCapsTest() functions with testQemuCapsIterate() and finally
remove the need to manually adjust the test programs every time
a new input file is introduced; moreover, this means that the
two lists can't possibly get out of sync anymore.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

This function iterates over a directory containing
capabilities-related data, extract some useful bits of
information from the file name, and calls a user-provided
callback.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

We're not using any of the functionality offered by the
module at the moment, but we will in just a second.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

This removes the awkard escaping and will allow us to perform
some more refactoring later on.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

We're using static string concatenation at the moment, but
that will no longer be a possibility in a bit.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

This removes a little duplication right away, and will allow
us to avoid introducing more later on.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

This is not particularly useful right now, but will allow us
to refactor some functionality later on.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

These functions don't do anything too interesting right now,
but will be extended later on.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

For snapshots, virsh already has a (shockingly naive [1]) client-side
topological sorter with the --tree option. But as a series of REDEFINE
calls must be presented in topological order, it's worth letting the
server do the work for us, especially since the server can give us a
topological sorting with less effort than our naive client
reconstruction.

[1] The XXX comment in virshSnapshotListCollect() about --tree being
O(n^3) is telling; https://en.wikipedia.org/wiki/Topological_sorting
is an interesting resource describing Kahn's algorithm and other
approaches for O(n) topological sorting for anyone motivated to use a
more elegant algorithm than brute force - but that doesn't affect this
patch.

For now, I am purposefully NOT implementing virsh fallback code to
provide a topological sort when the flag was rejected as unsupported;
we can worry about that down the road if users actually demonstrate
that they use new virsh but old libvirt to even need the fallback.
(The code we use for --tree could be repurposed to be such a fallback,
whether or not we keep it naive or improve it to be faster - but
again, no one should spend time on a fallback without evidence that we
need it.)

The test driver makes it easy to test:
$ virsh -c test:///default '
snapshot-create-as test a
snapshot-create-as test c
snapshot-create-as test b
snapshot-list test
snapshot-list test --topological
snapshot-list test --descendants a
snapshot-list test --descendants a --topological
snapshot-list test --tree
snapshot-list test --tree --topological
'

Without any flags, virsh does client-side sorting alphabetically, and
lists 'b' before 'c' (even though 'c' is the parent of 'b'); with the
flag, virsh skips sorting, and you can now see that the server handed
back data in a correct ordering. As shown here with a simple linear
chain, there isn't any other possible ordering, so --tree mode doesn't
seem to care whether --topological is used.  But it is possible to
compose more complicated DAGs with multiple children to a parent
(representing reverting back to a snapshot then creating more
snapshots along those divergent execution timelines), where it is then
possible (but not guaranteed) that adding the --topological flag
changes the --tree output (the client-side --tree algorithm breaks
ties based on alphabetical sorting between two nodes that share the
same parent, while the --topological sort skips the client-side
alphabetical sort and ends up exposing the server's internal order for
siblings, whether that be historical creation order or dependent on a
random hash seed).  But even if the results differ, they will still be
topologically correct.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

snapshot_conf does all the hard work, the qemu driver just has to
accept the new flag.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

snapshot_conf does all the hard work, the test driver just has to
accept the new flag.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Wire up support for VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL in the
domain-agnostic support code.

Clients of snapshot_conf using virDomainSnapshotForEachDescendant()
are using a depth-first visit but with postfix visits of a given
node. Changing this to a prefix visit of the given node instantly
turns this into a topologically-ordered visit.  (A prefix
breadth-first visit would also be topologically sorted, but that
requires a queue while our recursion naturally has a stack).

With that change, we now always have a topological sort for
virDomainSnapshotListAllChildren() regardless of the new public API
flag. Then with one more tweak, we can also get a topological rather
than a faster random hash visit for virDomainListAllSnapshots(), by
doing a descendent walk from our internal metaroot (there, we let the
public API flag control behavior, because a topological sort DOES
require more stack and slightly more time).

Note that virDomainSnapshotForEach() still uses a random hash visit;
we could change that signature to take a tri-state for random, prefix,
or postfix visit if we ever had clients that cared about the
distinctions, but for now, none of the drivers seem to care.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

When using virDomainSnapshotCreateXML with the REDEFINE flag on
multiple snapshot metadata XML descriptions, we require that a child
cannot be redefined before its parent.  Since libvirt already tracks a
DAG, it is more convenient if we can ensure that
virDomainListAllSnapshots() and friends have a way to return data in
an order that we can directly reuse, rather than having to
post-process the data ourselves to reconstruct the DAG.

Add VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL as our new guarantee (well, a
guarantee at the time of the API call conclusion; there's always a
possible TOCTTOU race where someone redefining snapshots in between
the API results and the client actually using the list might render
the list out-of-date). Four listing APIs are directly benefitted by
the new flag; additionally, since we document that the older racy
ListNames interfaces should be sized by using the same flags on their
Num counterparts, the Num interfaces must document when they accept
(and ignore) the flag.

We could have supported the new flag just for the ListAll APIs (to
discourage people from using the older racy Num/ListNames APIs), but
it feels weird to special-case this flag value as being applicable to
only a subset of the API while all other List-related flags are
trivially applicable to all 6.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NJán Tomko <jtomko@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

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

Now that everything is prepared for qemu driver we can enable
parser feature to allow users define such domains.

At the same time, introduce bunch of tests to test the feature.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

The firmware selection code will enable the feature if needed.
There's no need to require SMM to be enabled in that case.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>
Reviewed-by: NLaszlo Ersek <lersek@redhat.com>

When preparing domain call qemuFirmwareFillDomain() to fill in
desired firmware.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

And finally the last missing piece. This is what puts it all
together.

At the beginning, qemuFirmwareFillDomain() loads all possible
firmware description files based on algorithm described earlier.
Then it tries to find description which matches given domain.
The criteria are:

  - firmware is the right type (e.g. it's bios when bios was
    requested in domain XML)
  - firmware is suitable for guest architecture/machine type
  - firmware allows desired guest features to stay enabled (e.g.
    if s3/s4 is enabled for guest then firmware has to support
    it too)

Once the desired description has been found it is then used to
set various bits of virDomainDef so that proper qemu cmd line is
constructed as demanded by the description file. For instance,
secure boot enabled firmware might request SMM -> it will be
enabled if needed.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NLaszlo Ersek <lersek@redhat.com>

Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Implementation for yet another part of firmware description
specification. This one covers selecting which files to parse.

There are three locations from which description files can be
loaded. In order of preference, from most generic to most
specific these are:

  /usr/share/qemu/firmware
  /etc/qemu/firmware
  $XDG_CONFIG_HOME/qemu/firmware

If a file is found in two or more locations then the most specific
one is used. Moreover, if file is empty then it means it is
overriding some generic description and disabling it.

Again, this is described in more details and with nice examples
in firmware.json specification (qemu commit 3a0adfc9bf).

However, there's one slight difference - for the root user the
home directory is not searched. This follows rules laid out by
similar look up processes, e.g. PKI x509 certs are not searched
in /root but they are looked for under /home.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NLaszlo Ersek <lersek@redhat.com>

Test firmware description parsing so far.

The test files come from three locations:
1) ovmf-sb-keys.json and ovmf-sb.json come from OVMF
package from RHEL-7 (with slight name change to reflect their
features in filename too),

2) bios.json and aavmf.json come from example JSON documents from
firmware.json from qemu's git (3a0adfc9bf),

3) ovmf.json is then copied from ovmf-sb.json and stripped
of SECURE_BOOT and REQUIRES_SMM flags, OVMF path change,
description update and machine type expanded for both pc and q35
machine types.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>
Acked-by: NLaszlo Ersek <lersek@redhat.com>

The firmware description is a JSON file which follows
specification from qemu.git/docs/interop/firmware.json. The
description file basically says: Firmware file X is {bios|uefi},
supports these targets and machine types, requires these features
to be enabled on qemu cmd line and this is how you put it onto
qemu cmd line.

The firmware.json specification covers more (i.e. how to select
the right firmware) but that will be covered and implemented in
next commits.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

The idea is that using this attribute users enable libvirt to
automagically select firmware image for their domain. For
instance:

  <os firmware='efi'>
    <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
    <loader secure='no'/>
  </os>

  <os firmware='bios'>
    <type arch='x86_64' machine='pc-q35-4.0'>hvm</type>
  </os>

(The automagic of selecting firmware image will be described in
later commits.)

Accepted values are 'bios' and 'efi' to let libvirt select
corresponding type of firmware.

I know it is a good sign to introduce xml2xml test case when
changing XML config parser but that will have to come later.
Firmware auto selection is not enabled for any driver just yet so
any xml2xml test would fail right away.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

This is going to extend virDomainLoader enum. The reason is that
once loader path is NULL its type makes no sense. However, since
value of zero corresponds to VIR_DOMAIN_LOADER_TYPE_ROM the
following XML would be produced:

  <os>
    <loader type='rom'/>
    ...
  </os>

To solve this, introduce VIR_DOMAIN_LOADER_TYPE_NONE which would
correspond to value of zero and then use post parse callback to
set the default loader type to 'rom' if needed.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Except not really. At least for now.

In the future, the firmware will be selected automagically.
Therefore, it makes no sense to require the pathname of a
specific firmware binary in the domain XML. But since it is not
implemented do not really allow the path to be NULL. Only move
code around to prepare it for further expansion.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

In some cases, the string representing architecture is different
in qemu and libvirt. That is the reason why we have
virQEMUCapsArchFromString() and virQEMUCapsArchToString(). So
far, we did not need them outside of qemu_capabilities code, but
this will change shortly. Expose them then.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

Move the code that (possibly) generates filename of NVRAM VAR
store into a single function so that it can be re-used later.
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>
Reviewed-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NDaniel P. Berrangé <berrange@redhat.com>

It's meant for testing, not for production builds. Also we have a helper
for reporting OOM errors. Introduced by 23e0bf1cSigned-off-by: NPeter Krempa <pkrempa@redhat.com>

In qemuxml2xmltest, both activeVcpus and blockjobs members
of the testInfo struct have been entirely unused ever since
commit d1a7fc8b.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>

In local testing, I accidentally introduced a self-test failure,
and spent way too much time debugging it. Make sure the testsuite
log includes some hint as to why command option validation failed.
Lone exception: allocation failure is unlikely during self-test,
and if it happens, we are better off asserting (vsh.c can do this,
even if libvirt.so cannot).
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NErik Skultety <eskultet@redhat.com>
Acked-by: NPeter Krempa <pkrempa@redhat.com>

Forgot to remove this before pushing.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

Similarly to CAT, when you set some values in an group, remove the group and
recreate it, the previous values will be kept there.  In order to not get values
from a previous setting (a previous VM, for example), we need to set them to
sensible defaults.  The same way we do that for CAT, just set the same values as
the default group has.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

For CAT we calculate unallocated parts of the cache, however with MBA this does
not make sense as the purpose of that is to limit the bandwidth and the setting
is only proportional relative to bandwidth settings for other groups.

This means it makes sense to set the values to 100% even if there are other
groups with some allocations and that we don't need to find the available
(unallocated) bandwidth in all the groups.
Signed-off-by: NMartin Kletzander <mkletzan@redhat.com>

So far we are providing the suppressions file with a relative
path to valgrind. This apparently doesn't work on some distros
like Ubuntu and its derivates. Providing the absolute path fixes
the problem.
Signed-off-by: NShotaro Gotanda <g.sho1500@gmail.com>
Signed-off-by: NMichal Privoznik <mprivozn@redhat.com>

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

The existing behavior for ppc64 guests is to always add a USB
keyboard and mouse combo if graphics are present; unfortunately,
this means any attempt to use a USB tablet will cause both pointing
devices to show up in the guest, which in turn will result in poor
user experience.

We can't just stop adding the USB mouse or start adding a USB tablet
instead, because existing applications and users might rely on the
current behavior; however, we can avoid adding the USB mouse if a USB
tablet is already present, thus allowing users and applications to
create guests that contain a single pointing device.

https://bugzilla.redhat.com/show_bug.cgi?id=1683681Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

These are similar to the existing simple headless guests, but
also include a graphical output and some input devices.

Input files were generated by running

  $ virt-install \
    --name guest --os-variant fedora29 \
    --vcpus 4 --memory 4096 --disk size=5 \
    --graphics vnc \
    --print-xml

followed by minor tweaks.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>

Some devices (namely virtio-scsi, virtio-gpu, virtio-keyboard,
virtio-tablet and virtio-mouse, plus virtio-crypto which is
not supported by libvirt) don't follow the same rules as all
other virtio devices, which is something that ought to be
documented.
Signed-off-by: NAndrea Bolognani <abologna@redhat.com>
Reviewed-by: NCole Robinson <crobinso@redhat.com>