Commit Graph

2803 Commits

Author SHA1 Message Date
Maxiwell S. Garcia
dfd70ca1eb qemu: Do not override config XML in case of snapshot revert
Snapshot create operation saves the live XML and uses it to replace the
domain definition in case of revert. But the VM config XML is not saved
and the revert operation does not address this issue. This commit
prevents the config XML from being overridden by snapshot definition.

An active domain stores both current and new definitions. The current
definition (vm->def) stores the live XML and the new definition
(vm->newDef) stores the config XML. In an inactive domain, only the
config XML is persistent, and it's saved in vm->def.

The revert operation uses the virDomainObjAssignDef() to set the
snapshot definition in vm->newDef, if domain is active, or in vm->def
otherwise. But before that, it saves the old value to return to
caller. This return is used here to restore the config XML after
all snapshot startup process finish.

Signed-off-by: Maxiwell S. Garcia <maxiwell@linux.ibm.com>
2019-05-20 09:08:54 +02:00
Michal Privoznik
b58a6b050e qemuDomainSnapshotCreateXML: Don't leak parsed snapshot definition
This function gets snapshot XML (provided by used) as an
argument. It parses it into a local variable @def and then sets
some more members (e.g. it creates a copy of live domain XML).
Then it proceeds to checking if snapshot XML is valid (e.g. it
contains as many disks as currently in the domain). If this fails
then the control jumps to endjob label and subsequently return
from the function. This is where AUTOFREE function for @def is
ran. Well, because the code says to run plain VIR_FREE() we leak
some memory because @def is actually an object and therefore
it should have been declared as AUTOUNREF.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
2019-05-14 16:42:59 +02:00
Eric Blake
57387ff54b snapshot: Make virDomainSnapshotDef a virObject
This brings about a couple of benefits:
- use of VIR_AUTOUNREF() simplifies several callers
- Fixes a todo about virDomainMomentObjList not being polymorphic enough

Signed-off-by: Eric Blake <eblake@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>
2019-05-09 10:02:53 -05:00
Eric Blake
098043eddd snapshot: s/current/parent/ as prep for virObject
VIR_CLASS_NEW insists that descendents of virObject have 'parent' as
the name of their inherited base class member at offset 0. While it
would be possible to write a new class-creation macro that takes the
actual field name 'current', and rewrite VIR_CLASS_NEW to call the new
macro with the hard-coded name 'parent', it seems less confusing if
all object code uses similar naming. Thus, this is a mechanical rename
in preparation of making virDomainSnapshotDef a descendent of
virObject.

Signed-off-by: Eric Blake <eblake@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>
2019-05-09 09:48:07 -05:00
Eric Blake
36603bc568 snapshot: s/parent/parent_name/ as prep for virObject
VIR_CLASS_NEW insists that descendents of virObject have 'parent' as
the name of their inherited base class member at offset 0. While it
would be possible to write a new class-creation macro that takes the
actual field name, and rewrite VIR_CLASS_NEW to call the new macro
with the hard-coded name 'parent', so that we could make
virDomainMomentDef use a custom name for its base class, it seems less
confusing if all object code uses similar naming. Thus, this is a
mechanical rename in preparation of making virDomainSnapshotDef a
descendent of virObject, when we can no longer use 'parent' for a
different purpose than the base class.

Signed-off-by: Eric Blake <eblake@redhat.com>
Acked-by: Peter Krempa <pkrempa@redhat.com>
2019-05-09 09:43:41 -05:00
Peter Krempa
1a828a578e qemu: Rename qemuDomainDiskChainElement(Revoke|Prepare)
Use qemuDomainStorageSourceAccess(Allow|Revoke) instead.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-05-09 15:55:35 +02:00
Michal Privoznik
f0f1e79bf5 qemuConnectOpen: Drop unused @cfg and simplify
After 65a372d6e0 the @cfg variable is no longer used. This means
we can drop it and therefore drop 'cleanup' label with it.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
2019-05-04 23:39:35 +02:00
Daniel P. Berrangé
04e4307d34 Revert "network: use 'bridge' as actual type instead of 'network'"
This caused the live XML to report the 'bridge' type instead of the
'network' type, which is a behavioural regression.

It also breaks 'virsh domif-setlink', 'virsh update-device' and
'virsh domiftune'

This reverts commit 518026e159.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-04-30 14:42:34 +01:00
Michal Privoznik
572c50849c qemu: Check for user alias collisions in coldplug
https://bugzilla.redhat.com/show_bug.cgi?id=1697676

If an user tries to attach a device with colliding user alias
then we attach it happily and thus leave domain unable to start.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2019-04-26 15:47:40 +02:00
Michal Privoznik
57eb2936f3 qemu: On attach to live XML check for user alias collision only live XML
When attaching a device to live XML we don't care (well,
shouldn't care) that there's already a device in inactive XML
that has the same user alias.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2019-04-26 15:47:36 +02:00
Michal Privoznik
c4c44b535b qemuDomainAttachDeviceLiveAndConfig: Don't overwrite @ret
If we're attaching a device to both inactive and live XML then
@ret is overwritten which may result in incorrect return value.
For instance, if attaching to inactive XML succeeds, @ret is
assigned value of zero and control proceeds to attaching the
device to live XML. Here, if say
virDomainDeviceValidateAliasForHotplug() fails the control jumps
over to 'cleanup' label and zero is returned indicating success.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2019-04-26 15:16:17 +02:00
Michal Privoznik
08193fdbf5 src: Check for virDomainDiskInsert() retval properly
Our coding style specifies that only negative values are considered as
error. Check for return value of virDomainDiskInsert() properly,
following the style. Not that the function can now return anything other
than 0 or -1, but it just triggers my OCD.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2019-04-26 15:14:17 +02:00
Daniel Henrique Barboza
cc1d1dbbd5 qemuDomainPMSuspendForDuration: check for wake-up support
If the current QEMU guest can't wake up from suspend properly,
and we are able to determine that, avoid suspending the guest
at all. To be able to determine this support, QEMU needs to
implement the 'query-current-machine' QMP call. This is reflected
by the QEMU_CAPS_QUERY_CURRENT_MACHINE cap.

If the cap is enabled, a new function qemuDomainProbeQMPCurrentMachine
is called. This is wrapper for qemuMonitorGetCurrentMachineInfo,
where the 'wakeup-suspend-support' flag is retrieved from
'query-current-machine'. If wakeupSuspendSupport is true,
proceed with the regular flow of qemuDomainPMSuspendForDuration.

The absence of QEMU_CAPS_QUERY_CURRENT_MACHINE indicates that
we're dealing with a QEMU version older than 4.0 (which implements
the required QMP API). In this case, proceed as usual with the
suspend logic of qemuDomainPMSuspendForDuration, since we can't
assume whether the guest has support or not.

Fixes: https://bugs.launchpad.net/ubuntu/+source/qemu/+bug/1759509
Reported-by: Balamuruhan S <bala24@linux.vnet.ibm.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
2019-04-25 11:43:53 +02:00
Daniel P. Berrangé
518026e159 network: use 'bridge' as actual type instead of 'network'
Ports allocated on virtual networks with type=nat|route|open all get
given an actual type of 'network'.

Only ports in networks with type=bridge use an actual type of 'bridge'.

This distinction makes little sense since the virtualization drivers
will treat both actual types in exactly the same way, as they're all
just bridge devices a VM needs to be connected to.

This doesn't affect user visible XML since the "actual" device XML
is internal only, but we need code to convert the data upgrades.

Reviewed-by: Laine Stump <laine@laine.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-04-18 13:10:00 +01:00
Michal Privoznik
4bdce1219f qemu: Simplify interface handling in qemuConnectDomainXMLToNative()
Firstly, VIR_STRDUP() accepts NULL, so there is no need to check
if the string we want to duplicate is not-NULL. Secondly,
virDomainNetSetModelString() also accepts NULL. Thirdly, we have
VIR_AUTOFREE().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-04-17 10:47:23 +02:00
Andrea Bolognani
3c32f9ec42 qemu: Fix uninitialized variable
It has made Clang very unhappy ever since 6bf7c67699.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
2019-04-17 09:33:28 +02:00
Cole Robinson
6bf7c67699 conf: net: Add wrapper functions for <model> value
To ease converting the net->model value to an enum, add
the wrapper functions:

virDomainNetGetModelString
virDomainNetSetModelString
virDomainNetStreqModelString
virDomainNetStrcaseeqModelString

Acked-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
2019-04-16 13:11:08 -04:00
Daniel P. Berrangé
dd52444f23 network: restrict usage of port management APIs
The port allocation APIs are currently called unconditionally for all
types of NIC, but (mostly) only do anything for NICs with type=network.

The exception is the port allocate API which does some validation even
for NICs with type!=network. Relying on this validation is flawed,
however, since the network driver may not even be installed. IOW virt
drivers must not delegate validation to the network driver for NICs
with type != network.

This change allows us to report errors when the virtual network driver
is not registered.

Reviewed-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2019-04-16 14:44:53 +01:00
Jiri Denemark
673c62a3b7 qemu: Don't cache microcode version
My earlier commit be46f61326 was incomplete. It removed caching of
microcode version in the CPU driver, which means the capabilities XML
will see the correct microcode version. But it is also cached in the
QEMU capabilities cache where it is used to detect whether we need to
reprobe QEMU. By missing the second place, the original commit
be46f61326 made the situation even worse since libvirt would report
correct microcode version while still using the old host CPU model
(visible in domain capabilities XML).

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-04-15 14:34:49 +02:00
Cole Robinson
1d31526b52 Always put _LAST enums on second line of VIR_ENUM_IMPL
Standardize on putting the _LAST enum value on the second line
of VIR_ENUM_IMPL invocations. Later patches that add string labels
to VIR_ENUM_IMPL will push most of these to the second line anyways,
so this saves some noise.

Signed-off-by: Cole Robinson <crobinso@redhat.com>
2019-04-11 12:47:23 -04:00
Michal Privoznik
5b9819eedc domain capabilities: Expose firmware auto selection feature
If a management application wants to use firmware auto selection
feature it can't currently know if the libvirtd it's talking to
support is or not. Moreover, it doesn't know which values that
are accepted for the @firmware attribute of <os/> when parsing
will allow successful start of the domain later, i.e. if the mgmt
application wants to use 'bios' whether there exists a FW
descriptor in the system that describes bios.

This commit then adds 'firmware' enum to <os/> element in
<domainCapabilities/> XML like this:

  <enum name='firmware'>
    <value>bios</value>
    <value>efi</value>
  </enum>

We can see both 'bios' and 'efi' listed which means that there
are descriptors for both found in the system (matched with the
machine type and architecture reported in the domain capabilities
earlier and not shown here).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
2019-04-10 13:58:51 +02:00
Peter Krempa
285c5f28c4 util: Move enum convertors into virenum.(c|h)
virutil.(c|h) is a very gross collection of random code. Remove the enum
handlers from there so we can limit the scope where virtutil.h is used.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-04-10 09:12:04 +02:00
Nikolay Shirokovskiy
e3389d830c qemu: Don't duplicate suspend events and state changes
Since the STOP event handler can use the pausedReason as sent to
qemuProcessStopCPUs, we no longer need to send duplicate suspended
lifecycle events because we know what caused the stop along with extra
details. This processing allows us to also remove the duplicated state
change from qemuProcessStopCPUs.

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
2019-04-04 10:36:04 +03:00
Nikolay Shirokovskiy
cae45f2cdd qemu: fix domain unlock/unref in qemuMigrationSrcPerform
qemuMigrationSrcPerform callers expect it to call virDomainObjEndAPI
in any case so on error paths we miss the virDomainObjEndAPI call.
To fix this let's make qemuMigrationSrcPerform callers responsible
for the virDomainObjEndAPI call.

ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
2019-04-03 13:46:26 +03:00
Peter Krempa
c358adc571 qemu: capabilities: Always assume disk snapshot caps
'blockdev-snapshot-sync' is present in QEMU since v0.14.0-rc0 and
'transaction' since v1.1.0 (52e7c241ac766406f05fa)

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-04-03 11:58:10 +02:00
Peter Krempa
72e88ca0a2 qemu: capabilities: Always assume QEMU_CAPS_DRIVE_MIRROR
qemu added the 'drive-mirror' command in v1.3.0 (d9b902db3fb71fdc)

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-04-03 11:58:10 +02:00
Peter Krempa
852afb2dc4 qemu: capabilities: Always assume QEMU_CAPS_BLOCK_COMMIT
qemu added the 'block-commit' command in v1.3.0 (ed61fc10e8c8d2)

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-04-03 11:58:10 +02:00
Peter Krempa
f1a0d2277c qemu: domain: drop qemuDomainSupportsBlockJobs
It always returns true.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-04-03 11:58:10 +02:00
Peter Krempa
d5654a7537 qemu: capabilities: Always assume QEMU_CAPS_BLOCKJOB_ASYNC
This was detected by the presence of 'block-stream' which is present in
qemu since v1.1 (db58f9c0605fa151b8c4)

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-04-03 11:58:09 +02:00
Eric Blake
a487890d37 snapshot: Refactor qemu to utilize virDomainMoment more
Use the common base class virDomainMoment for iterator callbacks
related to snapshots from the qemu code, so that when checkpoint
operations are introduced, they can share the same callbacks.

Simplify the code for qemuDomainSnapshotCurrent by better utilizing
virDomainMoment helpers.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2019-03-27 08:16:10 -05:00
Eric Blake
3d7c683a27 snapshot: Drop pointless function virDomainMomentIsCurrentName
The qemu driver already had a full-blown virDomainMomentObjPtr to
check against, and the test driver ought to have one since we get
better error checking that the user passed in a valid object. Removes
the need for a helper function added in commit commit 4819f54b.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
2019-03-27 08:13:24 -05:00
Laine Stump
b204941865 qemu_hotplug: pull qemuDomainUpdateDeviceList out of qemuDomainDetachDeviceLive
qemuDomainDetachDeviceLive() is called from two places in
qemu_driver.c, and qemuDomainUpdateDeviceList() is called from the
end of qemuDomainDetachDeviceLive(), which is now in qemu_hotplug.c

This patch replaces the single call to qemuDomainUpdateDeviceList()
with two calls to it immediately after return from
qemuDomainDetachDeviceLive(). This is only done if the return from
that function is exactly 0, in order to exactly preserve previous
behavior.

Removing that one call from qemuDomainDetachDeviceList() will permit
us to call it from the test driver hotplug test, replacing the
separate calls to qemuDomainDetachDeviceDiskLive(),
qemuDomainDetachChrDevice(), qemuDomainDetachShmemDevice() and
qemuDomainDetachWatchdog(). We want to do this so that part of the
common functionality of those three functions (and the rest of the
device-specific Detach functions) can be pulled up into
qemuDomainDetachDeviceLive() without breaking the test. (This is done
in the next patch).

NB: Almost certainly this is "not the best place" to call
qemuDomainUpdateDeviceList() (actually, it is provably the *wrong*
place), since it's purpose is to retrieve an "up to date" list of
aliases for all devices from qemu, and if the guest OS hasn't yet
processed the detach request, the now-being-removed device may still
be on that list. It would arguably be better to instead call
qemuDomainUpdateDevicesList() later during the response to the
DEVICE_DELETED event for the device. But removing the call from the
current point in the detach could have some unforeseen ill effect due
to changed timing, so the change to move it into
qemuDomainRemove*Device() will be done in a separate patch (in order
to make it easily revertible in case it causes a regression).

Signed-off-by: Laine Stump <laine@laine.org>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
2019-03-26 11:05:03 -04:00
Laine Stump
6a9c3fbade qemu_hotplug: move qemuDomainDetachDeviceLive() to qemu_hotplug.c
This function is going to take on some of the functionality of its
subordinate functions, which all live in qemu_hotplug.c.

qemuDomainDetachDeviceControllerLive() is only called from
qemuDomainDetachDeviceLive() (and will soon be merged into
qemuDomainDetachControllerDevice(), which is in qemu_hotplug.c), so
it is also moved.

Signed-off-by: Laine Stump <laine@laine.org>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
2019-03-26 11:05:03 -04:00
Eric Blake
e055a816af snapshot: Rename virDomainSnapshotObjPtr
Now that the core of SnapshotObj is agnostic to snapshots and can be
shared with upcoming checkpoint code, it is time to rename the struct
and the functions specific to list operations. A later patch will
shuffle which file holds the common code. This is a fairly mechanical
patch.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2019-03-22 01:18:34 -05:00
Eric Blake
1ab05da228 snapshot: Switch type of virDomainSnapshotObj.def
Another step towards making the object list reusable for both
snapshots and checkpoints: the list code only ever needs items that
are in the common virDomainMomentDef base type. This undoes a lot of
the churn in accessing common members added in the previous patch, and
the bulk of the patch is mechanical. But there was one spot where I
had to unroll a VIR_STEAL_PTR to work around changed types.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2019-03-22 01:18:34 -05:00
Eric Blake
ffc0fbebe2 snapshot: Factor out virDomainMomentDef class
Pull out the common parts of virDomainSnapshotDef that will be reused
for virDomainCheckpointDef into a new base class.  Adjust all callers
that use the direct fields (some of it is churn that disappears when
the next patch refactors virDomainSnapshotObj; oh well...).

Someday, I hope to switch this type to be a subclass of virObject, but
that requires a more thorough audit of cleanup paths, and besides
minimal incremental changes are easier to review.

As for the choice of naming:
I promised my teenage daughter Evelyn that I'd give her credit for her
contribution to this commit. I asked her "What would be a good name
for a base class for DomainSnapshot and DomainCheckpoint". After
explaining what a base class was (using the classic OOB Square and
Circle inherit from Shape), she came up with "DomainMoment", which is
way better than my initial thought of "DomainPointInTime" or
"DomainPIT".

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2019-03-22 01:18:34 -05:00
Eric Blake
02c4e24db7 snapshot: Add accessors for updating snapshot list relations
Rather than allowing a leaky abstraction where multiple drivers have
to open-code operations that update the relations in a
virDomainSnapshotObjList, it is better to add accessor functions so
that updates to relations are maintained closer to the internals.
This patch finishes the job started in the previous patch, by getting
rid of all direct access to nchildren, first_child, or sibling outside
of the lowest level functions, making it easier to refactor later on.

The lone new caller to virDomainSnapshotObjListSize() checks for a
return != 0, because it wants to handles errors (-1, only possible if
the hash table wasn't allocated) and existing snapshots (> 0) in the
same manner; we can drop the check for a current snapshot on the
grounds that there shouldn't be one if there are no snapshots.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2019-03-22 01:18:33 -05:00
Eric Blake
ced0898f86 snapshot: Add accessor for reparenting snapshot children
Rather than allowing a leaky abstraction where multiple drivers have
to open-code operations that update the relations in a
virDomainSnapshotObjList, it is better to add accessor functions so
that updates to relations are maintained closer to the internals.
This patch starts the task with a single new function:
virDomainSnapshotMoveChildren(). The logic might not be immediately
obvious [okay, that's an understatement - the existing code uses black
magic ;-)], so here's an overview: The old code has an implicit for
loop around each call to qemuDomainSnapshotReparentChildren() by using
virDomainSnapshotForEachChild() (you'll need a wider context than
git's default of 3 lines to see that); the new code has a more visible
for loop. Then it helps if you realize that the code is making two
separate changes to each child object: STRDUP of the new parent name
prior to writing XML files (unchanged), and touching up the pointer to
the parent object (refactored); the end result is the same whether a
single pass made both changes (both in driver code), or whether it is
split into two passes making one change each (one in driver code, the
other in the new accessor).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2019-03-22 01:18:25 -05:00
Eric Blake
4819f54bd3 snapshot: Track current snapshot in virDomainSnapshotObjList
It is easier to track the current snapshot as part of the list of
snapshots. In particular, doing so lets us guarantee that the current
snapshot is cleared if that snapshot is removed from the list (rather
than depending on the caller to do so, and risking a use-after-free
problem, such as the one recently patched in 1db9d0efbf).  This
requires the addition of several new accessor functions, as well as a
useful return type for virDomainSnapshotObjListRemove().  A few error
handling sites that were previously setting vm->current_snapshot =
NULL can now be dropped, because the previous function call has now
done it already.  Also, qemuDomainRevertToSnapshot() was setting the
current vm twice, so keep only the one used on the success path.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2019-03-22 01:15:20 -05:00
Eric Blake
40bc98ddaf snapshot: Rework parse logic during libvirt startup
Rework the logic in qemuDomainSnapshotLoad() to set
vm->current_snapshot only once at the end of the loop, rather than
repeatedly querying it during the loop, to make it easier for the next
patch to use accessor functions rather than direct manipulation of
vm->current_snapshot.  When encountering multiple snapshots claiming
to be current (based on the presence of an <active>1</active> element
in the XML, which libvirt only outputs for internal use and not for
any public API), this changes behavior from warning only once and
running with no current snapshot, to instead warning on each duplicate
and selecting the last one encountered (which is arbitrary based on
readdir() ordering, but actually stands a fair chance of being the
most-recently created snapshot whether by timestamp or by the
propensity of humans to name things in ascending order).

Note that the code in question is only run by libvirtd when it first
starts, reading state from disk from the previous run into memory for
this run. Since the data resides somewhere that only libvirt should be
touching (typically /var/lib/libvirt/qemu/snapshot/*), it should be
clean.  So in the common case, the code touched here is unreachable.
But if someone is actually messing with files behind libvirt's back,
they deserve the change in behavior.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2019-03-22 01:15:20 -05:00
Eric Blake
f105627992 snapshot: Drop virDomainSnapshotDef.current
The only use for the 'current' member of virDomainSnapshotDef was with
the PARSE/FORMAT_INTERNAL flag for controlling an internal-use
<active> element marking whether a particular snapshot definition was
current, and even then, only by the qemu driver on output, and by qemu
and test driver on input. But this duplicates vm->snapshot_current,
and gets in the way of potential simplifications to have qemu store a
single file for all snapshots rather than one file per snapshot.  Get
rid of the member by adding a bool* parameter during parse (ignored if
the PARSE_INTERNAL flag is not set), and by adding a new flag during
format (if FORMAT_INTERNAL is set, the value printed in <active>
depends on the new FORMAT_CURRENT).

Then update the qemu driver accordingly, which involves hoisting
assignments to vm->current_snapshot to occur prior to any point where
a snapshot XML file is written (although qemu kept
vm->current_snapshot and snapshot->def_current in sync by the end of
the function, they were not always identical in the middle of
functions, so the shuffling gets a bit interesting). Later patches
will clean up some of that confusing churn to vm->current_snapshot.

Note: even if later patches refactor qemu to no longer use
FORMAT_INTERNAL for output (by storing bulk snapshot XML instead), we
will always need PARSE_INTERNAL for input (because on upgrade, a new
libvirt still has to parse XML left from a previous libvirt).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2019-03-22 01:15:20 -05:00
Cole Robinson
c53acd2ad1 Drop needless virtType validation
This code originates from:

commit d0aa10fdd6
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>
2019-03-18 09:35:00 -04:00
Eric Blake
9b75154c07 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>
2019-03-15 11:43:09 -05:00
Jiri Denemark
1c2a9260e8 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>
2019-03-15 09:39:19 +01:00
Eric Blake
e3e8fa1fb4 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>
2019-03-12 20:46:09 -05:00
Eric Blake
c502955909 snapshot: Give virDomainSnapshotDefFormat its own flags
virDomainSnapshotDefFormat currently takes two sets of knobs:
an 'unsigned int flags' argument that can currently just be
VIR_DOMAIN_DEF_FORMAT_SECURE, and an 'int internal' argument used as
a bool to determine whether to output an additional element.  It
then reuses the 'flags' knob to call into virDomainDefFormatInternal(),
which takes a different set of flags. In fact, prior to commit 0ecd6851
(1.2.12), the 'flags' argument actually took the public
VIR_DOMAIN_XML_SECURE, which was even more confusing.  Let's borrow
from the style of that earlier commit, by introducing a function
for translating from the public flags (VIR_DOMAIN_SNAPSHOT_XML_SECURE
was just recently introduced) into a new enum specific to snapshot
formatting, and adjust all callers to use snapshot-specific enum
values when formatting, and where the formatter now uses a new
variable 'domainflags' to make it obvious when we are translating
from snapshot flags back to domain flags.  We don't even have to
use the conversion function for drivers that don't accept the
public VIR_DOMAIN_SNAPSHOT_XML_SECURE flag.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2019-03-07 17:31:40 -06:00
Eric Blake
1ceb0e9337 qemu: Use virDomainSnapshotState for switch statements
Clean up the previous patch which abused switch on virDomainState
while working with a variable containing virDomainSnapshotState, by
converting the two affected switch statements to now use the right
enum.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2019-03-07 17:31:40 -06:00
Eric Blake
f43eb6807e snapshot: Rework virDomainSnapshotState enum
The existing virDomainSnapshotState is a superset of virDomainState,
adding one more state (disk-snapshot) on top of valid domain states.
But as written, the enum cannot be used for gcc validation that all
enum values are covered in a strongly-typed switch condition, because
the enum does not explicitly include the values it is adding to.

Copy the style used in qemu_blockjob.h of creating new enum names
for every inherited value, and update most clients to use the new
enum names anywhere snapshot state is referenced. The exception is
two switch statements in qemu code, which instead gain a fixme
comment about odd type usage (which will be cleaned up in the next
patch). The rest of the patch is fairly mechanical (I actually did
it by temporarily s/state/xstate/ in snapshot_conf.h to let the
compiler find which spots in the code used the field, did the
obvious search and replace in those functions, then undid the rename).

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
2019-03-07 17:31:40 -06:00
Eric Blake
8532ee4a9c qemu: Refactor snapshot check for _LIVE vs. _REDEFINE
The current qemu code rejects the combination of the two flags
VIR_DOMAIN_SNAPSHOT_CREATE_LIVE in tandem with
VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE, but rather late in the cycle
(after the snapshot was already parsed), and with a rather confusing
message (complaining that live snapshots require external storage,
even if the redefined snapshot already declares external storage).
Hoist the rejection message to occur earlier (before parsing any
XML, which also aids upcoming patches that will implement bulk
redefine), and with a more typical error message about mutually
exclusive flags.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-03-07 14:48:43 -06:00
Michal Privoznik
fc380c2e01 Revert "virStateDriver - Separate AutoStart from Initialize"
https://bugzilla.redhat.com/show_bug.cgi?id=1685151

This reverts commit cefb97fb81.

The stateAutoStart callback will be removed in the next commit.
Therefore move autostarting of domains, networks and storage
pools back into stateInitialize callbacks.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
2019-03-06 10:20:31 +01:00