5:54 p.m.
Still an RFC, since this claims over 400 tests are impacted:
$ git grep '<driver.*type=\(\(.\)[a-zA-Z0-9]*\2\)' tests/*data/ | wc
Most uses of <driver> are per-disk - they affect the way the hypervisor
exposes storage to the guest, regardless of how many host resources
are involved. But one attribute, the format, is something associated
with each host resource, since it is possible to have a chain of
files of different formats. What's more, the <volume> XML of storage
volume APIs already has a <format type='...'/> element.
This patch adds a <format type='...'/> element to each <disk>'s
<source> within a <domain>. For back-compat, we still output the
format of the top-level resource in the <driver> element; and
accept either spelling on input (if both spellings are given, they
must match); but once the XML is extended to show full backing
chain in <domain>s, the backing files will have only a <format>
element, and no extra <driver> elements.
Along with documenting, accepting, and outputting the new field,
this patch has to update every single test that had a <driver>
format to tolerate the new output. For tests where input differs
from output, I left the input alone to prove the old style still
works; there is also a new test to prove the new style as input
in isolation produces the correct output.
* docs/schemas/storagecommon.rng (storageSourceCommon): Parse new
element.
* docs/formatdomain.html.in: Document it.
* src/conf/domain_conf.c (virDomainDiskDefParseXML): Parse new
format element; ensure sane definition.
(virDomainDiskSourceFormat): Output new element.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
docs/formatdomain.html.in | 19 ++++++++++++++++++-
docs/schemas/storagecommon.rng | 7 +++++++
src/conf/domain_conf.c | 32 ++++++++++++++++++++++++++++++--
3 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 7f90455..53eda93 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1887,6 +1887,15 @@
write I/O operations per second.</dd>
</dl>
</dd>
+ <dt><code>format</code></dt>
+ <dd>
+ The optional format element includes a <code>type</code>
+ attribute that records the storage format used by the host
+ resource mentioned in the <code>source</code>
+ element. This element matches the format shown for the XML
+ of a storage volume, when inspecting a host resource via the
+ storage pool APIs. <span class="since">Since 1.2.4</span>
+ </dd>
<dt><code>driver</code></dt>
<dd>
The optional driver element allows specifying further details
@@ -1901,7 +1910,15 @@
supports a name of "tap", "tap2", "phy", or
"file", with a
type of "aio", while qemu only supports a name of
"qemu",
but multiple types including "raw", "bochs",
"qcow2", and
- "qed".
+ "qed".<br/>
+ <span class="since">Since 1.2.4</span>, the type
attribute
+ is also shown in the <code>format</code> element as its
+ preferred location (since a chain of backing files may
+ have separate types per file, while the majority of
+ the <code>driver</code> element applies to the device as a
+ whole rather than to individual backing files). On input,
+ either location may specify a format; if both locations
+ are used, the format must be identical between the two.
</li>
<li>
The optional <code>cache</code> attribute controls the
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index 4c0b719..68ce5b9 100644
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -162,6 +162,13 @@
<define name='storageSourceCommon'>
<interleave>
<optional>
+ <element name='format'>
+ <attribute name='type'>
+ <ref name='storageFormat'/>
+ </attribute>
+ </element>
+ </optional>
+ <optional>
<ref name="encryption"/>
</optional>
<optional>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 05fa3f9..e226758 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5111,6 +5111,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
char *sgio = NULL;
char *driverName = NULL;
char *driverType = NULL;
+ char *format = NULL;
const char *source = NULL;
char *target = NULL;
char *trans = NULL;
@@ -5819,14 +5820,34 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
def->product = product;
product = NULL;
- if (driverType) {
- def->src.format = virStorageFileFormatTypeFromString(driverType);
+ if (format) {
+ def->src.format = virStorageFileFormatTypeFromString(format);
if (def->src.format <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown driver format value '%s'"),
+ format);
+ goto error;
+ }
+ }
+
+ /* For back-compat, we also accept <driver type='format'> as a
+ * synonym for the newer <format type='format'>. If both are
+ * given, they must match. */
+ if (driverType) {
+ int value = virStorageFileFormatTypeFromString(driverType);
+ if (value <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown driver format value '%s'"),
driverType);
goto error;
+ } else if (def->src.format && def->src.format != value) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("conflicting formats '%s' and
'%s'"),
+ driverType,
+ virStorageFileFormatTypeToString(def->src.format));
+ goto error;
}
+ def->src.format = value;
}
if (mirrorFormat) {
@@ -5858,6 +5879,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
VIR_FREE(usageType);
VIR_FREE(authUUID);
VIR_FREE(authUsage);
+ VIR_FREE(format);
VIR_FREE(driverType);
VIR_FREE(driverName);
VIR_FREE(mirror);
@@ -14799,11 +14821,17 @@ virDomainDiskSourceFormat(virBufferPtr buf,
{
size_t n;
const char *startupPolicy = NULL;
+ const char *format = NULL;
if (policy)
startupPolicy = virDomainStartupPolicyTypeToString(policy);
+ if (src->format)
+ format = virStorageFileFormatTypeToString(src->format);
if (src->path || src->nhosts > 0 || src->srcpool || startupPolicy) {
+ /* No <format> element unless <source> provided. */
+ virBufferEscapeString(buf, "<format type='%s'>\n",
format);
+
switch ((enum virStorageType)src->type) {
case VIR_STORAGE_TYPE_FILE:
virBufferAddLit(buf, "<source");
--
1.9.0