When support for hotplug/unplug of SCSI controllers was added way back
in December 2009 (commit da9d937b), unplug was handled by calling the
now-extinct function qemuMonitorRemovePCIDevice(), which required a
PCI address as an argument. At the same time, the idea of every device
in the config having a PCI address apparently was not yet fully
implemented, because the author of the patch including a check for a
valid PCI address in the device object.
These days, all PCI devices are guaranteed to have a valid PCI
address. But more important than that, we no longer detach devices by
PCI address, but instead use qemuDomainDeleteDevice(), which
identifies the device by its alias. So checking for a valid PCI
address is just pointless extra code that obscures the high level of
similarity between all the individual qemuDomainDetach*Device()
functions.
Signed-off-by: Laine Stump <laine@laine.org>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
qemuDomainRemoveRNGDevice() calls qemuDomainDetachExtensionDevice().
According to commit 1d1e264f1 that added this code, it should not be
necessary to explicitly remove the zPCI extension device for a PCI
device during unplug, because "QEMU implements an unplug callback
which will unplug both PCI and zPCI device in a cascaded way". In
fact, no other devices call qemuDomainDetachExtensionDevice() during
their qemuDomainRemove*Device() function, so it should be removed from
qemuDomainRemoveRNGDevice as well.
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
qemuDomainDetachControllerDevice() calls
qemuDomainDetachExtensionDevice() when the controller type is
PCI. This is incorrect in multiple ways:
* Any code that tears down a device should be in the
qemuDomainRemove*Device() function (which is called after libvirt
gets a DEVICE_DELETED event from qemu indicating that the guest is
finished with the device on its end. The qemuDomainDetach*Device()
functions should only contain code that ensures the requested
operation is valid, and sends the command to qemu to initiate the
unplug.
* qemuDomainDetachExtensionDevice() is a function that applies to
devices that plug into a PCI slot, *not* necessarily PCI controllers
(which is what's being checked in the offending code). The proper
way to check for this would be to see if the DeviceInfo for the
controller device had a PCI address, not to check if the controller
is a PCI controller (the code being removed was doing the latter).
* According to commit 1d1e264f1 that added this code (and other
support for hotplugging zPCI devices on s390), it's not necessary to
explicitly detach the zPCI device when unplugging a PCI device. To
quote:
There's no need to implement hot unplug for zPCI as QEMU
implements an unplug callback which will unplug both PCI and
zPCI device in a cascaded way.
and the evidence bears this out - all the other uses of
qemuDomainDetachExtensionDevice() (except one, which I believe is
also in error, and is being removed in a separate patch) are only to
remove the zPCI extension device in cases where it was successfully
added, but there was some other failure later in the hotplug process
(so there was no regular PCI device to remove and trigger removal of
the zPCI extension device).
* PCI controllers are not hot pluggable, so this is dead code
anyway. (The only controllers that can currently be
hotplugged/unplugged are SCSI controllers).
Signed-off-by: Laine Stump <laine@laine.org>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>
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>
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>
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>
An upcoming patch will rework virDomainSnapshotObjList to be generic
for both snapshots and checkpoints; reduce the churn by adding a new
accessor virDomainSnapshotObjGetDef() which returns the
snapshot-specific definition even when the list is rewritten to
operate only on a base class, then using it at sites that that are
specific to snapshots. Use VIR_STEAL_PTR when appropriate in the
affected lines.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
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>
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>
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>
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>
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>
And adjust virQEMUCapsSetList to use it. It will also be used in future
patches.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
Storage source private data can be parsed along with other components of
private data rather than a separate function which is called from
multiple places.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
The existing behavior for ppc64 guests is to always add a USB
keyboard and mouse combo if graphics are present; unfortunately,
this means any attempt to use a USB tablet will cause both pointing
devices to show up in the guest, which in turn will result in poor
user experience.
We can't just stop adding the USB mouse or start adding a USB tablet
instead, because existing applications and users might rely on the
current behavior; however, we can avoid adding the USB mouse if a USB
tablet is already present, thus allowing users and applications to
create guests that contain a single pointing device.
https://bugzilla.redhat.com/show_bug.cgi?id=1683681
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
While the parser and schema have to accept all possible models,
virtio-(non-)transitional models are only applicable to
type=passthrough and should be otherwise rejected.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Right now, the only callers of qemuDomainSnapshotDiscardAllMetadata()
are right before freeing the virDomainSnapshotObjList, so it did not
matter if the list's metaroot (which points to all the defined root
snapshots) is left inconsistent. But an upcoming patch will want to
clear all snapshots if a bulk redefine fails partway through, in
which case things must be reset. Make this work by teaching the
existing virDomainSnapshotUpdateRelations() to be safe regardless of
the incoming state of the metaroot (since we don't want to leak that
internal detail into qemu code), then fixing the qemu code to use
it after deleting all snapshots. Additionally, the qemu code must
reset vm->current_snapshot if the current snapshot was removed,
regardless of whether the overall removal succeeded or failed later.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
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>
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>
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>
