conf: don't use "class" as name

Vim treats *.h files as cpp ones with respect to syntax highlighting.
Thus "class" in _virNodeDevCapPCIDev highlighted mistakenly.
This can be fixed by filetype detection code tunables but it
is more convinient to skip this tuning by every project member.

Let's just use "klass" as field name instead of _class or class
and add syntax rule.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

vz: build fix for virdomainsnapshotobjlist.h

Commit [1] moved snapshot list functions declaration into
its own file but missed a fix for vz driver.

[1] 9b75154c : snapshot: Break out virDomainSnapshotObjList into its own file

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>

virsh-pool: Offer only active pool for pool-refresh completer

Only active pools can be refreshed. But our completer offers just
all pool, even inactive ones.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

storagePoolRefreshFailCleanup: Clear volumes on failed refresh

If pool refresh failed, then the internal table of volumes is
probably left in inconsistent or incomplete state anyways. Clear
it out then. This has an advantage that we can move the
virStoragePoolObjClearVols() from those very few backends that
do call it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

storage_driver: Introduce storagePoolRefreshImpl()

This is a wrapper over refreshPool() call as at all places we are
doing basically the same. Might as well have a single function to
call.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

virISCSIDirectReportLuns: Drop ClearVols

In bf5cf610f206d5d54 I've fixed a problem where iscsi-direct
backend was reporting only the last LUN. The fix consisted of
moving virStoragePoolObjClearVols() one level up. However, as it
turns out, storage driver already calls it before calling
refreshPool callback (which is
virStorageBackendISCSIDirectRefreshPool() in this case).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

iscsi_direct: Don't overwrite error in virStorageBackenISCSIDirectWipeVol()

If virStorageBackendISCSIDirectVolWipeZero() fails, it has
already reported an error which is probably specific enough. Do
not overwrite it with some generic one.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

iscsi_direct: Make virStorageBackendISCSIDirectGetLun report error properly

This function reports error for one of the two error paths. This
is unfortunate as a caller see this function failing but doesn't
know right away if an error was reported.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

news: Add entry describing support for Xen's max grant frames

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

news: Document dropping support for certain init scripts

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

m4: Simplify LIBVIRT_CHECK_INIT_SCRIPT

Now that we no longer support the weird "redhat+systemd"
configuration, we can make our code slightly simpler.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Drop support for "Red Hat" init scripts

Despite the misleading name, these were supposed to be used
with a System V style init; however, none of the platforms we
target is using that kind of init anymore: almost all Linux
distributions have switched to systemd, those that haven't
(such as Gentoo and Alpine) are mostly using OpenRC with
custom init scripts, and the BSDs have been doing their own
thing all along.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Drop support for Upstart init scripts

Not a single one of the platforms we target still uses Upstart, and
the Upstart project itself has been abandoned for several years now.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

travis: Drop $DISTCHECK_CONFIGURE_FLAGS

It's no longer used.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

travis: Stop using --with-init-script

We're gonna drop support for non-systemd init scripts soon,
and we don't want Travis CI builds to break when we do.

Since we have init system auto-detection, we can just rely on
that and stop passing --with-init-script to configure entirely.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

snapshot: Break out virDomainSnapshotObjList into its own file

snapshot_conf.h was mixing three separate types: the snapshot
definition, the snapshot object, and the snapshot object list.
Separate out the snapshot object list code into its own file, and
update includes for affected clients.

This is just code motion, but done in preparation of sharing a lot of
the object list code with checkpoints.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

snapshot: Export two functions prior to file split

The next patch will require access to the helper functions
virDomainSnapshotDefFormatInternal and
virDomainSnapshotRedefineValidate from two different files; make the
file split easier by exporting these functions.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

snapshot: Break out virDomainSnapshotObj into its own file

snapshot_conf.h was mixing three separate types: the snapshot
definition, the snapshot object, and the snapshot object list.
Separate out the snapshot object code into its own file, which
includes moving a typedef to avoid circular inclusions.

Mostly straight code motion, although I fixed a comment along
the way, now that virDomainSnapshotForEachDescendent now
guarantees a topological visit (missed in b647d219).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

snapshot: Sort virconftypes.h

It's easier to locate a typedef if they are stored in sorted order;
do so mechanically via:

$ sed -i '/typedef struct/ {N; N; s/\n//g}' src/conf/virconftypes.h
$ # sorting the lines
$ sed -i '/typedef struct/ s/;/;\n/g' src/conf/virconftypes.h

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

conf: Split capabilities forward typedefs into virconftypes.h

As explained in the previous patch, collecting pointer typedefs into a
common header makes it easier to avoid circular inclusions.  Continue
the efforts by pulling the appropriate typedefs from capabilities.h
into the new header.

This patch is just straight code motion (all typedefs are listed in
the same order before and after the patch); a later patch will sort
things for legibility.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

conf: Split domain forward typedefs into virconftypes.h

Right now, snapshot_conf.h is rather large - it deals with three
separate types: virDomainSnapshotDef (the snapshot definition as it
maps to XML), virDomainSnapshotObj (an object containing a def and the
relationship to other snapshots), and virDomainSnapshotObjList (a list
of snapshot objects), where two of the three types are currently
public rather than opaque.  What's more, the types are circular: a
snapshot def includes a virDomainPtr, which contains a snapshot list,
which includes a snapshot object, which includes a snapshot def.

In order to split the three objects into separate files, while still
allowing each header to use sane typedefs to incomplete pointers, the
obvious solution is to lift the typedefs into yet another header, with
no other dependencies.  Start the split by factoring out all struct
typedefs from domain_conf.h (enum typedefs don't get used in function
signatures, and function typedefs tend not to suffer from circular
referencing, so those stay put).  The only other exception is
virDomainStateReason, which is only ever used directly rather than via
a pointer.

This patch is just straight code motion (all typedefs are listed in
the same order before and after the patch); a later patch will sort
things for legibility.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

object: Add sanity check on correct parent class

Checking that the derived class is larger than the requested parent
class saves us from some obvious mistakes, but as written, it does not
catch all the cases; in particular, it is easy to forget to update a
VIR_CLASS_NEW when changing the 'parent' member from virObject to
virObjectLockabale, but where the size checks don't catch that.  Add a
parameter for one more layer of sanity checking.

It would be cool if we could get gcc to stringize typeof(parent) into
the string name of that type, so that we could confirm that the
precise parent class is in use rather than just a struct that happens
to have the same size as the parent class.  But sizeof checks are
better than nothing.

Note that I did NOT change the fact that we require derived classes to
be larger (as the difference in size makes it easy to tell classes
apart), which means that even if a derived class has no functionality
to add (but rather exists for compiler-enforced type-safety), it must
still include a dummy member.  But I did fix the wording of the error
message to match the code.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

qemu: command: Override HOME variable for system QEMU

By default, qemu user's home dir points to '/' which shouldn't be used
at all. We therefore pass the HOME variable from the current variable
iff not running as SUID, which means that for systemd we never set it.
This patch makes sure, that for system QEMU this is always set to
libDir/<driver>, session mode is left untouched.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

qemu: command: Enforce setting XDG variables for system QEMU

For session mode, only XDG_CACHE_HOME is set, because we want to remain
integrating with services in user session, but for system mode, this
would have become reading/writing to '/' which carries the obvious issue
with permissions (also, '/' is the wrong location in 99.9% cases anyway).

Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

util: command: Introduce virCommandAddEnvXDG helper

Some modules/libraries within QEMU could make use of the XDG_ vars when
writing their data to the disk. Define the most common XDG variables
and point them to the specific driver's libDir, i.e.

XDG_CACHE_HOME -> /var/lib/libvirt/<driver>/.cache
XDG_DATA_HOME -> /var/lib/libvirt/<driver>/.local/share
XDG_CONFIG_HOME -> /var/lib/libvirt/<driver>/.config

Signed-off-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

qemu: hotplug: Merge virtio and non-virtio disk unplug code

The functions do basically exactly the same thing modulo few checks.
In case of virtio disks we check that the device is not multifunction as
that can't be unplugged at once. In case of USB and SCSI disks we
checked that no active block job is running.

The check for running blockjobs should have also been done for virtio
disks. By moving the multifunction check into the common function we fix
this case and also simplify the code.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu: hotplug: Use switch statement for selecting disk bus function

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu: hotplug: Use typecasted enum in qemuDomainDetachDeviceDiskLive

Use the correct type in switch and populate the missing cases.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

qemu: hotplug: Remove 'ret' variable in qemuDomainDetachDeviceDiskLive

We don't have any cleanup section, we can return the value directly.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

snapshot: More clarification about REDEFINE

Based on recent list questions about the proposed addition of
virDomainCheckpointCreateXML(REDEFINE), it is worth adding some
clarification to the existing snapshot redefine documentation that is
serving as the basis for checkpoints.

Normal snapshot creation requires very few elements from the user XML
(libvirt can pick sane defaults for items that are omitted, and many
fields, including <domain>, are documented as readonly output fields
ignored on input, produced by drivers that track it). But during
REDEFINE, the API wants the complete XML produced by an earlier
virDomainSnapshotGetXMLDesc; as the domain definition has likely
changed since the snapshot was first created, libvirt is unable to
recreate a <domain> sub-element that matches the original output
representing the domain state at the time the snapshot was first
created. In fact, reverting without a <domain> sub-element is risky
enough that we had to add a FORCE flag for virDomainSnapshotRevert().
In short, we only support omitting domain for qemu because of
backwards-compatibility to snapshots created before 0.9.5 started
capturing <domain>; even though there are other drivers like vbox that
do not output <domain> because they have other reliable ways to
revert.

And based on the confusion caused when omitting <domain> from snapshot
XML, the initial design for checkpoints in later patches will make
<domain> a mandatory element during its REDEFINE.

[Side note: the fact that <domain> can appear in <domainsnapshot> is a
reason we cannot add a new API for a bulk listing or redefine of all
snapshots of a single domain in one XML call (for example, a 1M
<domain> XML * 16 snapshots explodes into 16M in a bulk form, which
gets difficult to send over RPC). Perhaps we could add a flag to
request that the <domain> sub-element be omitted on output, but such
output is no longer suitable for sane REDEFINE input.]

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

virobject: Improve documentation

I had to inspect the code to learn whether a final virObjectUnref()
calls ALL dispose callbacks in child-to-parent order (akin to C++
destructors), or whether I manually had to call a parent-class dispose
when writing a child class dispose method.  The answer is the
former. (Thankfully, since VIR_FREE wipes out pointers for safety,
even if I had guessed wrong, I probably would not have tripped over a
double-free fault when the parent dispose ran for the second time).  I
also had to read the code to learn if a dispose method was even
mandatory (it is not, although getting NULL through VIR_CLASS_NEW
requires a macro).  While at it, the VIR_CLASS_NEW macro requires that
the virObject component at offset 0 be reached through the name
'parent', not 'object'.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

qemu_hotplug: Fix a rare race condition when detaching a device twice

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

If a device is detached twice from the same domain the following
race condition may happen:

1) The first DetachDevice() call will issue "device_del" on qemu
monitor, but since the DEVICE_DELETED event did not arrive in
time, the API ends claiming "Device detach request sent
successfully".

2) The second DetachDevice() therefore still find the device in
the domain and thus proceeds to detaching it again. It calls
EnterMonitor() and qemuMonitorSend() trying to issue "device_del"
command again. This gets both domain lock and monitor lock
released.

3) At this point, qemu sends us the DEVICE_DELETED event which is
going to be handled by the event loop which ends up calling
qemuDomainSignalDeviceRemoval() to determine who is going to
remove the device from domain definition. Whether it is the
caller that marked the device for removal or whether it is going
to be the event processing thread.

4) Because the device was marked for removal,
qemuDomainSignalDeviceRemoval() returns true, which means the
event is to be processed by the thread that has marked the device
for removal (and is currently still trying to issue "device_del"
command)

5) The thread finally issues the "device_del" command, which
fails (obviously) and therefore it calls
qemuDomainResetDeviceRemoval() to reset the device marking and
quits immediately after, NOT removing any device from the domain
definition.

At this point, the device is still present in the domain
definition but doesn't exist in qemu anymore. Worse, there is no
way to remove it from the domain definition.

Solution is to note down that we've seen the event and if the
second "device_del" fails, not take it as a failure but carry on
with the usual execution.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>

qemuMonitorJSONDelDevice: Return -2 on DeviceNotFound error

A caller might be interested in differentiating the cause for
error, especially if DeviceNotFound error occurred.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>

qemu_hotplug: Introduce and use qemuDomainDeleteDevice

The aim of this function will be to fix return value of
qemuMonitorDelDevice() in one specific case. But that is yet to
come. Right now this is nothing but a plain substitution.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>

run: Don't export unnecessary paths

We're using virFileFindResourceFull() to locate resources
nowadays, which makes exporting these information in the
environment unnecessary: see

  virDriverLoadModule() for LIBVIRT_DRIVER_DIR
  virLockManagerPluginNew() for LIBVIRT_LOCK_MANAGER_PLUGIN_DIR
  virLockManagerLockDaemonConnectionNew() for VIRTLOCKD_PATH
  doRemoteOpen() for LIBVIRTD_PATH

As further proof that we don't need to expose the information
this way anymore, we're not even exporting VIRTLOGD_PATH, which
would be necessary if virLogManagerConnect() didn't already
take care of that for us.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

qemu: Set job statsType for external memory snapshot

Any job which is able to provide statistics that can be queried via
virDomainGetJob{Stats,Info} has to set an appropriate statsType.

Without a proper statsType qemuDomainJobInfoToParams and
qemuDomainJobInfoToInfo have no idea what statistics should be sent to
the API caller.

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

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

test: storage: Fill in default vol types for every pool

Fill in a default volume type for every pool type, as reported
by the VolGetInfo API. Now that we cover the whole enum, report
an error for invalid values.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>

docs: hacking: Add 'Code coverage reports' section

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>

configure: Remove --enable-test-coverage

We provide a custom configure option --enable-test-coverage and
'make cov' target to generate code coverage reports. However gnulib
already provides a 'make coverage' which 'just works' and doesn't
require a special configure option.

This drops our custom implementation in favor of 'make coverage'.
Reports are now output to cov/index.html

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>

virt-driver-qemu.m4: Require YAJL

There is no way that qemu driver can work without being able to
format/parse JSON.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

virt-driver-qemu.m4: Make qemu driver optional

The basic idea of our configure script is to probe for things
rather than have them enabled by default. This is even more
visible in the next commit where configure fails if qemu driver
is enabled but no yajl is found.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

virt-yajl.m4: Drop useless check for qemu

The code tries to detect installed version of qemu to learn if it
uses HMP or QMP and enable YAJL based on that. Well, we support
only QMP and also minimal required version of qemu is 1.5.0 so
the check would have enabled yajl anyway.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

news: Document recent snapshot topological flag

A new API flag is news-worthy.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

conf: Introduce virDomainDefCputuneValidate helper

Introduce a simple validation helper to perform the cputune period and
quota checks so that we can get rid of those repetitive chunks. Since
this is a validation helper, this patch also moves the checks from the
'parse' phase into the 'validation' phase.

Signed-off-by: Suyang Chen <dawson0xff@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

Don't define abs_* variables ourselves

Apparently this was necessary in the past because old versions
of autoconf/automake didn't make them available, but these
days all of the platforms we target include recent enough
autotools - as evidenced by the fact that, for example, we
already use abs_top_srcdir in tools/ despite the fact that
tools/Makefile.am is missing the same boilerplate.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

tests: Don't redefine variables for TESTS_ENVIRONMENT

We already have code that defines all abs_* variables at the
top of tests/Makefile.am, so there is no point in redefining
them a second time (using a slightly different shell
incantation to boot).

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

Fix names for abs_top_{src,build}dir variables

According to the official documentation for autoconf[1], the
correct names for these variables are abs_top_{src,build}dir
rather than abs_top{src,build}dir; in fact, we're already
using the correct names in various places, so let's just make
everything nice and consistent.

[1] https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Preset-Output-Variables.html

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

tests: Stop looking for abs_top_srcdir in the environment

This code snippet has clearly been cargo-culted, and all its
instances can be safely dropped seeing as 1) a much better
way to handle the scenario in C programs would be to pass the
value via the preprocessor, and 2) the value is actually not
used anywhere after being defined.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

tests: Don't define TEST_DRIVER_DIR

It's no longer used anywhere.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

tests: Don't use TEST_DRIVER_DIR in virTestCaptureProgramExecChild()

TEST_DRIVER_DIR is defined as "$(top_builddir)/src/.libs"; however,
as of commit bc6e206322ae, virDriverLoadModule() will search (the
absolute version of) that directory automatically, which means
passing it through the environment is no longer necessary.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

tests: Drop LIBVIRT_DRIVER_DIR from TESTS_ENVIRONMENT

LIBVIRT_DRIVER_DIR is defined as (what is for all intents and
purposes equivalent to) "$(abs_top_builddir)/src/.libs"; however,
as of commit bc6e206322ae, virDriverLoadModule() will search that
directory automatically, which means passing it through the
environment is no longer necessary.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

tests: Drop CONFIG_HEADER from TESTS_ENVIRONMENT

It's no longer used as of commit a9694a8e183e.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

qemu: clean up qemuDomainRemoveInactiveCommon

Use VIR_AUTOFREE and saner formatting. No semantic change.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

domain_conf: Expose virDomainStorageNetworkParseHost

An upcoming patch wants to reuse XML parsing of both unix and tcp
network host descriptions in the context of setting up a backup
NBD server. Make that easier by refactoring the existing parser.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>

docs: Consistent spacing in *GetXMLDesc functions

We copy-and-paste a lot of our docs, as evidenced by the number of
*GetXMLDesc() functions which had the same unusual indentation and
missing capital in the second sentence of the returns paragraph.

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

vbox: Fix build after xenbus addition

Commit 09eb1ae0 added a new enum type for xenbus, and adjusted
affected switch statements in the qemu driver, but failed to notice
that the vbox driver had a similar switch statement.

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

xenconfig: Add support for max_grant_frames

Add support in the domXML<->native config converter for
max_grant_frames. Include a test for the conversion.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

libxl: Add support for max_grant_frames

Add support for setting max_grant_frames in libxl domain config
object and include a test to check that it is properly converted
from XML to libxl domain config.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

libxl: Add implicit xenbus controller

All Xen domains have a xenbus device. Implicitly add one if not
already explicitly specified in the domain config.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

conf: Add a new 'xenbus' controller type

xenbus is virtual controller (akin to virtio controllers) for Xen
paravirtual devices. Although all Xen VMs have a xenbus, it has
never been modeled in libvirt, or in Xen native VM config format
for that matter.

Recently there have been requests to support Xen's max_grant_frames
setting in libvirt. max_grant_frames is best modeled as an attribute
of xenbus. It describes the maximum IO buffer space (or DMA space)
available in xenbus for use by connected paravirtual devices. This
patch introduces a new xenbus controller type that includes a
maxGrantFrames attribute.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

apparmor: Check libvirtd profile status by name

Commit a3ab6d42 changed the libvirtd profile to a named profile,
breaking the apparmor driver's ability to detect if the profile is
active. When the apparmor driver loads it checks the status of the
libvirtd profile using the full binary path, which fails since the
profile is now referenced by name. If the apparmor driver is
explicitly requested in /etc/libvirt/qemu.conf, then libvirtd fails
to load too.

Instead of only checking the profile status by full binary path,
also check by profile name. The full path check is retained in case
users have a customized libvirtd profile with full path.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Jamie Strandboge <jamie@canonical.com>

util: Introduce virStringParseYesNo helper

This helper performs a conversion from a "yes|no" string to a
corresponding boolean. This allows us to drop several repetitive
if-then-else string->bool conversion blocks.

Signed-off-by: Shotaro Gotanda <g.sho1500@gmail.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

tests: Call virFileWrapperClearPrefixes() for tests using virFileWrapper

This is mostly to avoid a memleak that is not a true memleak
anyway - prefixes will be freed by kernel upon test exit.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

virFileWrapper: Use VIR_AUTOFREE()

This enables us to simplify the code a bit.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

tests: Turn virFileWrapperAddPrefix to void

In theory, it's nice to have virFileWrapperAddPrefix() return a
value that indicates if the function succeeded or not. But in
practice, nobody checks for that and in fact blindly believes
that the function succeeded. Therefore, make the function return
nothing and just abort() if it would fail.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

qemu_hotplug: Properly check for qemuMonitorDelDevice retval

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

news: Document firmware autoselection for QEMU driver

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>

qemuFirmwareFetchConfigs: Fix check for @privileged

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: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>

tests: Use testQemuCapsIterate()

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: Andrea Bolognani <abologna@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>

tests: Introduce testQemuCapsIterate()

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: Andrea Bolognani <abologna@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>

tests: Add testutilsqemu dependency for qemucaps2xmltest

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: Andrea Bolognani <abologna@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>

tests: Move code from DO_TEST() to doCapsTest()

This removes the awkard escaping and will allow us to perform
some more refactoring later on.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>

tests: Use virAsprintf() to build titles

We're using static string concatenation at the moment, but
that will no longer be a possibility in a bit.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>

tests: Move data directories into testQemuData

This removes a little duplication right away, and will allow
us to avoid introducing more later on.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>

tests: Move ret into testQemuData

This is not particularly useful right now, but will allow us
to refactor some functionality later on.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>

tests: Introduce testQemuDataInit() and testQemuDataReset()

These functions don't do anything too interesting right now,
but will be extended later on.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>

virsh: Add snapshot-list --topological

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: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

qemu: Support topological visits

snapshot_conf does all the hard work, the qemu driver just has to
accept the new flag.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

test: Support topological visits

snapshot_conf does all the hard work, the test driver just has to
accept the new flag.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

snapshots: Support topological visits

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: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

snapshots: Add flag to guarantee topological sort

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: Eric Blake <eblake@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

qemu: Enable firmware autoselection

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: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

qemuDomainDefValidate: Don't require SMM if automatic firmware selection enabled

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: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

qemu_process: Call qemuFirmwareFillDomain

When preparing domain call qemuFirmwareFillDomain() to fill in
desired firmware.

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

qemu_firmware: Introduce qemuFirmwareFillDomain()

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: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

qemufirmwaretest: Test qemuFirmwareFetchConfigs()

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

qemu_firmware: Introduce qemuFirmwareFetchConfigs

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: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>

test: Introduce qemufirmwaretest

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: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>

qemu: Introduce basic skeleton for parsing firmware description

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: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

conf: Introduce firmware attribute to <os/>

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: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

conf: Introduce VIR_DOMAIN_LOADER_TYPE_NONE

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: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

virDomainLoaderDefParseXML: Allow loader path to be NULL

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: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

qemu_capabilities: Expose qemu <-> libvirt arch translators

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: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

qemu_domain: Separate NVRAM VAR store file name generation

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: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

tools: vsh: Don't use assert()

It's meant for testing, not for production builds. Also we have a helper
for reporting OOM errors. Introduced by 23e0bf1c4eb6

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

tests: Drop unnecessary variables

In qemuxml2xmltest, both activeVcpus and blockjobs members
of the testInfo struct have been entirely unused ever since
commit d1a7fc8bb303.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>

virsh: Make self-test failures noisy

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: Eric Blake <eblake@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>

resctrl: Fix testing line

Forgot to remove this before pushing.

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

resctrl: Set MBA defaults properly

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: Martin Kletzander <mkletzan@redhat.com>

resctrl: Do not calculate free bandwidth for MBA

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: Martin Kletzander <mkletzan@redhat.com>