If we pass XML to virDomainDefineXML API with these two elements:
...
<title></title>
<description></description>
...
libvirt correctly ignores these two elements and they will not appear
in the parsed XML.
However, if we use virDomainSetMetadata API and with "" as value for
title or description we will end up with the parsed XML that contains
these empty elements.
Let's fix the behavior of this API to behave the same as
virDomainDefineXML.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1518042
Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Add a new function to make it possible to parse a list of snapshots
at once. This is a counterpart to an earlier patch making it
possible to produce all snapshots in a single XML string, and
intentionally parses the same top-level element <snapshots> with
an optional attribute current='name'.
Note that since we know we started with no relations at all, and
since checking parent relationships per-snapshot is not viable as
we don't control which order the snapshots appear in, that we are
fine with doing a final pass to update all parent/child
relationships among the definitions.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Pull out the portion of virDomainSnapshotRefinePrep() that deals
with definition sanity into a separate helper routine that can
be reused with bulk redefine, leaving behind only the code
specific to loop checking and in-place updates that are only
needed in single-definition handling.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@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>
Add a new function to output all of the domain's snapshots in one
buffer.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
Split out an internal helper that produces format into a
virBuffer, similar to what domain_conf.c does, and making
the next patch easier to write.
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>
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>
Failure would have occurred for @ctxt before in callers' other
virXPath calls and @def derefs.
Found by Coverity due to commit 66a508d2 using VIR_XPATH_NODE_AUTORESTORE
to access @ctxt before the if condition. The @def was noted by review.
Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Capabilities should not duplicate data that are obvious from our
documentation and will not change with different QEMU binaries
or the way how we compile libvirt.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
When dealing with internal paths we don't need to worry about
whether or not suffixes are lowercase since we have full control
over them, which means we can avoid performing case-insensitive
string comparisons.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
Despite its name, this is really just a general-purpose string
manipulation function, so it should be moved to the virstring
module and renamed accordingly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
Despite its name, this is really just a general-purpose string
manipulation function, so it should be moved to the virstring
module and renamed accordingly.
A few trivial whitespace changes are squashed in.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
Despite its name, this is really just a general-purpose string
manipulation function, so it should be moved to the virstring
module and renamed accordingly.
In addition to the obvious s/File/String/, also tweak the name
to make it clear that the presence of the suffix is verified
using case-insensitive comparison.
A few trivial whitespace changes are squashed in.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
In these cases the check that is removed has been done a few
lines above already (as can even be seen in the context). Drop
them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Add support to format the storage pool capabilities using
the virStoragePoolTypeInfoPtr to determine what capabilities
exist for the various pools and the driver capabilities to
determine whether the pool is compiled in and supported.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
https://bugzilla.redhat.com/show_bug.cgi?id=1581670
During storage driver backend initialization, let's save
which backends are available in the storage pool capabilities.
In order to format those, we need add a connectGetCapabilities
processor to the storageHypervisorDriver. This allows a storage
connection, such as "storage:///system" to find the API and
format the results, such as:
virsh -c storage:///system capabilities
<capabilities>
<pool>
<enum name='type'>
<value>dir</value>
<value>fs</value>
<value>netfs</value>
<value>logical</value>
<value>iscsi</value>
<value>iscsi-direct</value>
<value>scsi</value>
<value>mpath</value>
<value>disk</value>
<value>rbd</value>
<value>sheepdog</value>
<value>gluster</value>
<value>zfs</value>
</enum>
</pool>
</capabilities>
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Introduce the bare bones functions to processing capability
data for the storage driver.
Since there will be no need for the <host> output, we need
to filter that data.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
The ZFS pool is documented as not using pool format types, so remove
the defaultFormat value.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
The multipath pool is documented as not using the volume type,
so let's just remove it.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
The iscsi and iscsi-direct pools are documented as not using
the volume type, so let's just remove it. Besides it would
have produced bad output since formatting uses the Disk types.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
The scsi pool is documented as not using the volume type,
so let's just remove it.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
The rbd pool is documented as not using the volume type,
so let's just remove it.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
The sheepdog pool is documented as not using the volume type,
so let's just remove it. Besides it would have produced bad
results since the defaultType is FILE but the formatting used
the Disk types.
Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
Rather than moving the XPath root node in the caller and then still
passing it down, make sure that the callees move the node themselves.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Remove logic necessary to figure out whether to format the 'features'
element by using virXMLFormatElement.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virXMLFormatElement for the formatting which allows us to avoid
looking through the array to see if any feature is enabled.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
If none of the 'capabilities' features are enabled we'd still format the
opening and closing tag for the <capabilities element.
The implementation is suboptimal but will be refactored for a better
approach. This is done prior to the refactor to show that tests are not
impacted.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use VIR_AUTOCLEAN to avoid leaking the buffer on error path and get rid
of resetting mid loop since virXMLFormatElement does the reset
internally.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
'i' is always in range of the enum, thus the name is always populated by
virDomainFeatureTypeToString.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
These buffers are used temporarily for some of the partial formatters
but not globally. Prefix the name with 'tmp' to be explicit.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Pure code motion of code for formatting domain features to a function
called virDomainDefFormatFeatures. Best viewed with the '--patience'
option for git show.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Split out the code into a separate function named
virDomainDefFormatBlkiotune and use virXMLFormatElement.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virXMLFormatElement to format the internals along with simplifying
cleanup code paths.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virXMLFormatElement to format the internals along with simplifying
cleanup code paths.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virXMLFormatElement to format the internals along with simplifying
cleanup code paths.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virXMLFormatElement to format the internals along with simplifying
cleanup code paths.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virXMLFormatElement to format the internals along with simplifying
cleanup code paths.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Use virXMLFormatElement to format the internals along with simplifying
cleanup code paths.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Refactor the function to use the XML formatting aid and use automatic
cleaning to simplify the control flow.
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
After commits e2087c2 and ec0793de older GCC started act very smart and
complain about potentially uninitialized variable, which existed prior
to these patches + even if the affected vars were left uninitialized the
function responsible for filling them in would have failed with NULL
being returned which the caller has always handled carefully.
Although GCC complained only about a single variable, let's initialize
all of them so as to prevent any further potential breakages.
Signed-off-by: Erik Skultety <eskultet@redhat.com>
Add <controller type='scsi' model handling for virtio transitional
devices. Ex:
<controller type='scsi' model='virtio-transitional'/>
* "virtio-transitional" maps to qemu "virtio-scsi-pci-transitional"
* "virtio-non-transitional" maps to qemu "virtio-scsi-non-transitional"
The naming here doesn't match the pre-existing model=virtio-scsi.
The prescence of '-scsi' there seems kind of redundant as we have
type='scsi' already, so I decided to follow the pattern of other
patches and use virtio-transitional etc.
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
