]> xenbits.xensource.com Git - libvirt.git/log
libvirt.git
6 years agoqemu: fill in virCapsEnum 'report'
Cole Robinson [Wed, 6 Mar 2019 23:21:45 +0000 (18:21 -0500)]
qemu: fill in virCapsEnum 'report'

Set report=true for all enums currently formatted in the XML

Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agoconf: domcaps: Add virCapsEnum 'report'
Cole Robinson [Wed, 6 Mar 2019 23:11:01 +0000 (18:11 -0500)]
conf: domcaps: Add virCapsEnum 'report'

virCapsEnum report is an internal bool indicating whether we
should format the enum in the XML at all. This is unused for
now but will be handled in future patches.

We use a plain bool instead of tristate because the case here
is a bit different than the explicit @supported output. We
already report the equivalent of supported=YES|NO based on
what enum values are filled in. This adds report=false to
handle the ABSENT case.

Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agoconf: domcaps: Don't output XML on tristate ABSENT
Cole Robinson [Tue, 19 Feb 2019 19:12:47 +0000 (14:12 -0500)]
conf: domcaps: Don't output XML on tristate ABSENT

Change domcaps to skip formatting XML if the default
TRISTATE_BOOL_ABSENT is found. Now when domcaps is extended, driver
XML output won't change until an explicit TRISTATE_BOOL value is set
in driver code.

Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agoschemas: domcaps: Make more elements optional
Cole Robinson [Sat, 9 Feb 2019 00:10:17 +0000 (19:10 -0500)]
schemas: domcaps: Make more elements optional

Upcoming changes will make outputting these subelements optional.

While we are here drop the useless interleave: since this is an output
only format the elements are always in the same order

Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agobhyve: domcaps: fill in explicit supported BOOL_NO
Cole Robinson [Tue, 19 Feb 2019 19:28:52 +0000 (14:28 -0500)]
bhyve: domcaps: fill in explicit supported BOOL_NO

<hostdev> and <features> are not supported. <loader>, <graphics>,
and <video> are supported conditionally

Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agolibxl: domcaps: fill in explicit supported BOOL_NO
Cole Robinson [Tue, 19 Feb 2019 19:22:23 +0000 (14:22 -0500)]
libxl: domcaps: fill in explicit supported BOOL_NO

None of the <feature> bits are supported, and the <loader> piece
is only conditionally supported

Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agoqemu: domcaps: fill in explicit supported BOOL_NO
Cole Robinson [Tue, 19 Feb 2019 19:15:06 +0000 (14:15 -0500)]
qemu: domcaps: fill in explicit supported BOOL_NO

Only gic->supported needs an explicit BOOL_NO setting, all other
'supported' values are handling things correctly

Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agoconf: domcaps: use virTristateBool for 'supported'
Cole Robinson [Fri, 8 Feb 2019 23:03:20 +0000 (18:03 -0500)]
conf: domcaps: use virTristateBool for 'supported'

Switch most 'supported' handling to use virTristateBool, so eventually
we can handle the ABSENT state.

For now the XML formatter treats ABSENT the same as FALSE, so there's
no functional output change. This will be addressed in later patches

Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agoconf: domcaps: Add single line formatting macro
Cole Robinson [Fri, 8 Feb 2019 23:32:48 +0000 (18:32 -0500)]
conf: domcaps: Add single line formatting macro

Similar to the macros we have for formatting enums, add a macro to
simplify formatting the pattern:

  <FOO supported='yes|no'/>

Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agotests: domcaps: Remove 'full' test
Cole Robinson [Tue, 19 Feb 2019 17:52:30 +0000 (12:52 -0500)]
tests: domcaps: Remove 'full' test

The 'full' test verifies the output of a virDomainCapsPtr built
by hand. It has the following problems:

The domcaps test suite nowadays has 3 hypervisor driver implementations
which should give us plenty of opportunity to get full domcaps coverage.
I don't think this test has much value. And it has the following issues:

- Requires manual intervention to test new domcaps XML, which is easy
  to miss, for example gic bits aren't covered there.
- The SET_ALL_BITS trick it uses to fill in enums will output
  values that are never reported by any driver implementation
  (strings like 'default')

Let's remove it

Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agotests: domcaps: Remove unused typedef
Cole Robinson [Tue, 19 Feb 2019 17:51:37 +0000 (12:51 -0500)]
tests: domcaps: Remove unused typedef

Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agotests: domcaps: Add a default 'empty' test
Cole Robinson [Tue, 19 Feb 2019 17:44:34 +0000 (12:44 -0500)]
tests: domcaps: Add a default 'empty' test

The 'empty' demonstrates XML generated when only bare minimum caps
data has been filled in. This will demonstrate changes that alter
the default XML output.

Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agotests: Document how to add new replies files
Andrea Bolognani [Wed, 13 Mar 2019 13:28:07 +0000 (14:28 +0100)]
tests: Document how to add new replies files

We already document how to generate them, so might as well
go the extra mile and document the remaining steps.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Acked-by: Michal Privoznik <mprivozn@redhat.com>
6 years agonode_device_hal.c: Follow _class -> klass rename
Michal Privoznik [Mon, 18 Mar 2019 13:32:55 +0000 (14:32 +0100)]
node_device_hal.c: Follow _class -> klass rename

In 0eca80e60 _class was renamed to klass for variety of struct
members. However, gather_usb_cap() was missed out in this rename
leaving FreeBSD build broken.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
6 years agoDrop needless virtType validation
Cole Robinson [Sun, 17 Mar 2019 19:53:40 +0000 (15:53 -0400)]
Drop needless virtType validation

This code originates from:

commit d0aa10fdd6c108ad442886e4451b2629a3dc8b86
Author: Daniel P. Berrange <berrange@redhat.com>
Date:   Tue Mar 3 12:03:44 2009 +0000

    QEMU security driver usage for sVirt support (James Morris, Dan Walsh, Daniel Berrange)

Originally in the qemudDomainGetSecurityLabel function. It doesn't
appear to have done anything useful back then either. The other two
instances look like copy+paste

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agostorageVolWipePattern: Don't take shortcut to refreshPool()
Michal Privoznik [Thu, 28 Feb 2019 16:35:14 +0000 (17:35 +0100)]
storageVolWipePattern: Don't take shortcut to refreshPool()

In d16f803d780 we've tried to solve an issue that after wiping an
image its format might have changed (e.g. from qcow2 to raw) but
libvirt wasn't probing the image format. We fixed this by calling
virStorageBackendRefreshVolTargetUpdate() which is what
refreshPool() would end up calling. But this shortcut is not good
enough because the function is called only for local types of
volumes (like dir, fs, netfs). But now that more backends support
volume wiping we have to call the function with more caution.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
6 years agostorage_backend_iscsi_direct: Simplify vol zeroing
Michal Privoznik [Fri, 1 Mar 2019 10:42:20 +0000 (11:42 +0100)]
storage_backend_iscsi_direct: Simplify vol zeroing

So far we have two branches: either we zero BLOCK_PER_PACKET
(currently 128) block at once, or if we're close to the last block
then we zero out one block at the time. This is very suboptimal.
We know how many block are there left. Might as well just write
them all at once.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
6 years agovirsh: man: Document asynchronous behaviour of detach-device-alias
Peter Krempa [Mon, 18 Mar 2019 07:08:57 +0000 (08:08 +0100)]
virsh: man: Document asynchronous behaviour of detach-device-alias

This command is fully async. Note that users can use virsh event to be
notified of the guest actually removing the device.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
6 years agovirsh: man: Document quirks of device-detach and friends
Peter Krempa [Mon, 18 Mar 2019 07:08:04 +0000 (08:08 +0100)]
virsh: man: Document quirks of device-detach and friends

Mention that successful return does not equal to device being detached
similarly as we do at the API level.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
6 years agonews: update for new class element of PCI nodedev capability
Nikolay Shirokovskiy [Tue, 12 Mar 2019 10:59:35 +0000 (13:59 +0300)]
news: update for new class element of PCI nodedev capability

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
6 years agoxml: nodedev: add class info for pci capability
Nikolay Shirokovskiy [Tue, 19 Feb 2019 12:41:37 +0000 (15:41 +0300)]
xml: nodedev: add class info for pci capability

This info can be useful to filter devices visible
to mgmt clients so that they won't see devices that
unsafe/not meaningful to pass thru.

Provide class info the way it is provided by udev or
kernel that is as single 6-digit hexadecimal.

Class element is not optional. I guess this should not
break users that use virNodeDeviceCreateXML because
they probably specify only scsi_host capability on
input and then node device driver gets other capabilities
from udev after device appeared.

HAL driver does not get support for the new element in
this patch.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
6 years agoconf: don't use "class" as name
Nikolay Shirokovskiy [Tue, 12 Mar 2019 08:08:00 +0000 (11:08 +0300)]
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>
6 years agovz: build fix for virdomainsnapshotobjlist.h
Nikolay Shirokovskiy [Mon, 18 Mar 2019 08:03:04 +0000 (11:03 +0300)]
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>
6 years agovirsh-pool: Offer only active pool for pool-refresh completer
Michal Privoznik [Fri, 1 Mar 2019 09:56:42 +0000 (10:56 +0100)]
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>
6 years agostoragePoolRefreshFailCleanup: Clear volumes on failed refresh
Michal Privoznik [Thu, 28 Feb 2019 17:07:02 +0000 (18:07 +0100)]
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>
6 years agostorage_driver: Introduce storagePoolRefreshImpl()
Michal Privoznik [Fri, 1 Mar 2019 09:55:23 +0000 (10:55 +0100)]
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>
6 years agovirISCSIDirectReportLuns: Drop ClearVols
Michal Privoznik [Thu, 28 Feb 2019 16:43:57 +0000 (17:43 +0100)]
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>
6 years agoiscsi_direct: Don't overwrite error in virStorageBackenISCSIDirectWipeVol()
Michal Privoznik [Wed, 6 Mar 2019 14:43:04 +0000 (15:43 +0100)]
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>
6 years agoiscsi_direct: Make virStorageBackendISCSIDirectGetLun report error properly
Michal Privoznik [Wed, 6 Mar 2019 14:27:55 +0000 (15:27 +0100)]
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>
6 years agonews: Add entry describing support for Xen's max grant frames
Jim Fehlig [Wed, 13 Mar 2019 22:06:11 +0000 (16:06 -0600)]
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>
6 years agonews: Document dropping support for certain init scripts
Andrea Bolognani [Thu, 14 Mar 2019 10:59:42 +0000 (11:59 +0100)]
news: Document dropping support for certain init scripts

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
6 years agom4: Simplify LIBVIRT_CHECK_INIT_SCRIPT
Andrea Bolognani [Fri, 15 Mar 2019 15:57:40 +0000 (16:57 +0100)]
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>
6 years agoDrop support for "Red Hat" init scripts
Andrea Bolognani [Fri, 15 Mar 2019 15:47:27 +0000 (16:47 +0100)]
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>
6 years agoDrop support for Upstart init scripts
Andrea Bolognani [Thu, 14 Mar 2019 10:56:13 +0000 (11:56 +0100)]
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>
6 years agotravis: Drop $DISTCHECK_CONFIGURE_FLAGS
Andrea Bolognani [Fri, 15 Mar 2019 15:37:09 +0000 (16:37 +0100)]
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>
6 years agotravis: Stop using --with-init-script
Andrea Bolognani [Thu, 14 Mar 2019 10:57:03 +0000 (11:57 +0100)]
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>
6 years agosnapshot: Break out virDomainSnapshotObjList into its own file
Eric Blake [Fri, 15 Mar 2019 02:19:18 +0000 (21:19 -0500)]
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>
6 years agosnapshot: Export two functions prior to file split
Eric Blake [Fri, 15 Mar 2019 02:12:51 +0000 (21:12 -0500)]
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>
6 years agosnapshot: Break out virDomainSnapshotObj into its own file
Eric Blake [Fri, 15 Mar 2019 01:03:40 +0000 (20:03 -0500)]
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>
6 years agosnapshot: Sort virconftypes.h
Eric Blake [Fri, 15 Mar 2019 01:20:13 +0000 (20:20 -0500)]
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>
6 years agoconf: Split capabilities forward typedefs into virconftypes.h
Eric Blake [Fri, 15 Mar 2019 00:44:34 +0000 (19:44 -0500)]
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>
6 years agoconf: Split domain forward typedefs into virconftypes.h
Eric Blake [Fri, 15 Mar 2019 00:44:34 +0000 (19:44 -0500)]
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>
6 years agoobject: Add sanity check on correct parent class
Eric Blake [Fri, 15 Mar 2019 14:41:18 +0000 (09:41 -0500)]
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>
6 years agoqemu: command: Override HOME variable for system QEMU
Erik Skultety [Wed, 6 Mar 2019 12:29:01 +0000 (13:29 +0100)]
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>
6 years agoqemu: command: Enforce setting XDG variables for system QEMU
Erik Skultety [Fri, 8 Mar 2019 11:15:07 +0000 (12:15 +0100)]
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>
6 years agoutil: command: Introduce virCommandAddEnvXDG helper
Erik Skultety [Mon, 4 Mar 2019 11:47:08 +0000 (12:47 +0100)]
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>
6 years agoqemu: hotplug: Merge virtio and non-virtio disk unplug code
Peter Krempa [Fri, 15 Mar 2019 14:09:14 +0000 (15:09 +0100)]
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>
6 years agoqemu: hotplug: Use switch statement for selecting disk bus function
Peter Krempa [Fri, 15 Mar 2019 14:04:00 +0000 (15:04 +0100)]
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>
6 years agoqemu: hotplug: Use typecasted enum in qemuDomainDetachDeviceDiskLive
Peter Krempa [Fri, 15 Mar 2019 13:52:58 +0000 (14:52 +0100)]
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>
6 years agoqemu: hotplug: Remove 'ret' variable in qemuDomainDetachDeviceDiskLive
Peter Krempa [Fri, 15 Mar 2019 14:32:52 +0000 (15:32 +0100)]
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>
6 years agosnapshot: More clarification about REDEFINE
Eric Blake [Thu, 14 Mar 2019 02:38:52 +0000 (21:38 -0500)]
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>
6 years agovirobject: Improve documentation
Eric Blake [Fri, 15 Mar 2019 04:14:41 +0000 (23:14 -0500)]
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>
6 years agoqemu_hotplug: Fix a rare race condition when detaching a device twice
Michal Privoznik [Thu, 14 Mar 2019 10:02:52 +0000 (11:02 +0100)]
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>
6 years agoqemuMonitorJSONDelDevice: Return -2 on DeviceNotFound error
Michal Privoznik [Thu, 14 Mar 2019 07:46:37 +0000 (08:46 +0100)]
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>
6 years agoqemu_hotplug: Introduce and use qemuDomainDeleteDevice
Michal Privoznik [Tue, 12 Mar 2019 12:49:24 +0000 (13:49 +0100)]
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>
6 years agorun: Don't export unnecessary paths
Andrea Bolognani [Thu, 14 Mar 2019 09:14:19 +0000 (10:14 +0100)]
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>
6 years agoqemu: Set job statsType for external memory snapshot
Jiri Denemark [Thu, 14 Mar 2019 14:33:26 +0000 (15:33 +0100)]
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>
6 years agotest: storage: Fill in default vol types for every pool
Cole Robinson [Thu, 7 Feb 2019 16:29:24 +0000 (11:29 -0500)]
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>
6 years agodocs: hacking: Add 'Code coverage reports' section
Cole Robinson [Wed, 13 Mar 2019 17:03:15 +0000 (13:03 -0400)]
docs: hacking: Add 'Code coverage reports' section

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
6 years agoconfigure: Remove --enable-test-coverage
Cole Robinson [Wed, 13 Mar 2019 16:07:24 +0000 (12:07 -0400)]
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>
6 years agovirt-driver-qemu.m4: Require YAJL
Michal Privoznik [Wed, 13 Mar 2019 15:40:14 +0000 (16:40 +0100)]
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>
6 years agovirt-driver-qemu.m4: Make qemu driver optional
Michal Privoznik [Thu, 14 Mar 2019 12:30:26 +0000 (13:30 +0100)]
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>
6 years agovirt-yajl.m4: Drop useless check for qemu
Michal Privoznik [Wed, 13 Mar 2019 15:36:12 +0000 (16:36 +0100)]
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>
6 years agonews: Document recent snapshot topological flag
Eric Blake [Thu, 14 Mar 2019 01:08:50 +0000 (20:08 -0500)]
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>
6 years agoconf: Introduce virDomainDefCputuneValidate helper
Suyang Chen [Thu, 14 Mar 2019 03:54:31 +0000 (21:54 -0600)]
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>
6 years agoDon't define abs_* variables ourselves
Andrea Bolognani [Tue, 12 Mar 2019 15:33:05 +0000 (16:33 +0100)]
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>
6 years agotests: Don't redefine variables for TESTS_ENVIRONMENT
Andrea Bolognani [Tue, 12 Mar 2019 15:31:20 +0000 (16:31 +0100)]
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>
6 years agoFix names for abs_top_{src,build}dir variables
Andrea Bolognani [Tue, 12 Mar 2019 14:11:47 +0000 (15:11 +0100)]
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>
6 years agotests: Stop looking for abs_top_srcdir in the environment
Andrea Bolognani [Tue, 12 Mar 2019 13:21:18 +0000 (14:21 +0100)]
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>
6 years agotests: Don't define TEST_DRIVER_DIR
Andrea Bolognani [Wed, 13 Mar 2019 15:20:53 +0000 (16:20 +0100)]
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>
6 years agotests: Don't use TEST_DRIVER_DIR in virTestCaptureProgramExecChild()
Andrea Bolognani [Wed, 13 Mar 2019 13:47:03 +0000 (14:47 +0100)]
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>
6 years agotests: Drop LIBVIRT_DRIVER_DIR from TESTS_ENVIRONMENT
Andrea Bolognani [Wed, 13 Mar 2019 15:21:06 +0000 (16:21 +0100)]
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>
6 years agotests: Drop CONFIG_HEADER from TESTS_ENVIRONMENT
Andrea Bolognani [Tue, 12 Mar 2019 13:36:09 +0000 (14:36 +0100)]
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>
6 years agoqemu: clean up qemuDomainRemoveInactiveCommon
Eric Blake [Wed, 6 Mar 2019 03:11:19 +0000 (21:11 -0600)]
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>
6 years agodomain_conf: Expose virDomainStorageNetworkParseHost
Eric Blake [Tue, 9 Oct 2018 20:07:35 +0000 (15:07 -0500)]
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>
6 years agodocs: Consistent spacing in *GetXMLDesc functions
Eric Blake [Wed, 13 Mar 2019 19:42:30 +0000 (14:42 -0500)]
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>
6 years agovbox: Fix build after xenbus addition
Eric Blake [Thu, 14 Mar 2019 01:20:22 +0000 (20:20 -0500)]
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>
6 years agoxenconfig: Add support for max_grant_frames
Jim Fehlig [Fri, 8 Mar 2019 18:51:57 +0000 (11:51 -0700)]
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>
6 years agolibxl: Add support for max_grant_frames
Jim Fehlig [Thu, 7 Mar 2019 22:16:09 +0000 (15:16 -0700)]
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>
6 years agolibxl: Add implicit xenbus controller
Jim Fehlig [Thu, 7 Mar 2019 00:43:21 +0000 (17:43 -0700)]
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>
6 years agoconf: Add a new 'xenbus' controller type
Jim Fehlig [Wed, 6 Mar 2019 22:59:29 +0000 (15:59 -0700)]
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>
6 years agoapparmor: Check libvirtd profile status by name
Jim Fehlig [Fri, 1 Mar 2019 21:34:17 +0000 (14:34 -0700)]
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>
6 years agoutil: Introduce virStringParseYesNo helper
Shotaro Gotanda [Wed, 13 Mar 2019 06:30:10 +0000 (15:30 +0900)]
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>
6 years agotests: Call virFileWrapperClearPrefixes() for tests using virFileWrapper
Michal Privoznik [Wed, 13 Mar 2019 08:56:47 +0000 (09:56 +0100)]
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>
6 years agovirFileWrapper: Use VIR_AUTOFREE()
Michal Privoznik [Wed, 13 Mar 2019 08:54:45 +0000 (09:54 +0100)]
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>
6 years agotests: Turn virFileWrapperAddPrefix to void
Michal Privoznik [Wed, 13 Mar 2019 08:45:25 +0000 (09:45 +0100)]
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>
6 years agoqemu_hotplug: Properly check for qemuMonitorDelDevice retval
Michal Privoznik [Tue, 12 Mar 2019 08:28:18 +0000 (09:28 +0100)]
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>
6 years agonews: Document firmware autoselection for QEMU driver
Michal Privoznik [Wed, 13 Mar 2019 10:05:08 +0000 (11:05 +0100)]
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>
6 years agoqemuFirmwareFetchConfigs: Fix check for @privileged
Michal Privoznik [Wed, 13 Mar 2019 09:50:54 +0000 (10:50 +0100)]
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>
6 years agotests: Use testQemuCapsIterate()
Andrea Bolognani [Thu, 7 Mar 2019 15:02:37 +0000 (16:02 +0100)]
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>
6 years agotests: Introduce testQemuCapsIterate()
Andrea Bolognani [Thu, 7 Mar 2019 14:54:55 +0000 (15:54 +0100)]
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>
6 years agotests: Add testutilsqemu dependency for qemucaps2xmltest
Andrea Bolognani [Wed, 6 Mar 2019 17:21:31 +0000 (18:21 +0100)]
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>
6 years agotests: Move code from DO_TEST() to doCapsTest()
Andrea Bolognani [Thu, 7 Mar 2019 13:53:18 +0000 (14:53 +0100)]
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>
6 years agotests: Use virAsprintf() to build titles
Andrea Bolognani [Thu, 7 Mar 2019 14:37:40 +0000 (15:37 +0100)]
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>
6 years agotests: Move data directories into testQemuData
Andrea Bolognani [Thu, 7 Mar 2019 13:33:20 +0000 (14:33 +0100)]
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>
6 years agotests: Move ret into testQemuData
Andrea Bolognani [Thu, 7 Mar 2019 13:24:14 +0000 (14:24 +0100)]
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>
6 years agotests: Introduce testQemuDataInit() and testQemuDataReset()
Andrea Bolognani [Thu, 7 Mar 2019 13:22:37 +0000 (14:22 +0100)]
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>
6 years agovirsh: Add snapshot-list --topological
Eric Blake [Fri, 8 Mar 2019 04:12:01 +0000 (22:12 -0600)]
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>
6 years agoqemu: Support topological visits
Eric Blake [Fri, 8 Mar 2019 04:29:55 +0000 (22:29 -0600)]
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>
6 years agotest: Support topological visits
Eric Blake [Fri, 8 Mar 2019 04:29:55 +0000 (22:29 -0600)]
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>