On Sat, May 05, 2012 at 02:46:32AM +0200, Marc-André Lureau wrote: /volume/allocation says in its documentation "If set to a value smaller than the capacity, the pool has the option of deciding to sparsely allocate a volume. It does not have to honour requests for sparse allocation though.", ie when trying to use "allocation" to get a sparsely allocated volume, failures to do the sparse allocation are not fatal (if the documentation is to be trusted). I think it would be nicer to be consistent with that for preallocation and not return an error either in this case. Christophe

On Sat, May 05, 2012 at 02:46:32AM +0200, Marc-André Lureau wrote: I'd use VIR_ERR_XML_ERROR or VIR_ERR_CONFIG_UNSUPPORTED here The string returned by virStorageQemuImgOptionsStr is leaked here and in all subsequent uses. Patch looks good otherwise. Christophe

Allow to specify preallocation mode for QCOW2 images. If not specified or not available, it's ignored. This change only modify the schema, doc, parsing and tests. --- docs/formatstorage.html.in | 6 ++++++ docs/schemas/storagevol.rng | 18 ++++++++++++++++++ src/conf/storage_conf.c | 26 ++++++++++++++++++++++++++ src/conf/storage_conf.h | 1 + src/util/storage_file.c | 4 ++++ src/util/storage_file.h | 10 ++++++++++ tests/storagevolxml2xmlin/vol-qcow2.xml | 1 + tests/storagevolxml2xmlout/vol-qcow2.xml | 1 + 8 files changed, 67 insertions(+) diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in index d0e4319..c4faadf 100644 --- a/docs/formatstorage.html.in +++ b/docs/formatstorage.html.in @@ -252,6 +252,12 @@ 1,152,921,504,606,846,976 bytes). <span class="since">Since 0.4.1, multi-character <code>unit</code> since 0.9.11</span></dd> + <dt><code>preallocation</code></dt> + <dd>An image with preallocated metadata is initially larger but + can improve performance when the image needs to grow. This is + supported by QCOW2 image format. + Attribe <code>mode</code> value can be 'off' or 'metadata'. + <span class="since">Since 0.9.13</span></dd> <dt><code>capacity</code></dt> <dd>Providing the logical capacity for the volume. This value is in bytes by default, but a <code>unit</code> attribute can be diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 8edb877..d9a148e 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -40,6 +40,7 @@ <ref name='scaledInteger'/> </element> </optional> + <ref name='preallocation'/> </define> <define name='permissions'> @@ -171,6 +172,23 @@ </optional> </define> + <define name='preallocationmode'> + <choice> + <value>off</value> + <value>metadata</value> + </choice> + </define> + + <define name='preallocation'> + <optional> + <element name='preallocation'> + <attribute name='mode'> + <ref name='preallocationmode'/> + </attribute> + </element> + </optional> + </define> + <define name='name'> <data type='string'> <param name="pattern">[a-zA-Z0-9_\+\-\.]+</param> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 0b34f28..446cc9d 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -991,6 +991,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, char *allocation = NULL; char *capacity = NULL; char *unit = NULL; + char *preallocation = NULL; xmlNodePtr node; options = virStorageVolOptionsForPoolType(pool->type); @@ -1035,6 +1036,19 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, ret->allocation = ret->capacity; } + preallocation = virXPathString("string(./preallocation/@mode)", ctxt); + if (preallocation) { + if ((ret->preallocation = virStoragePreallocationModeTypeFromString(preallocation)) < 0) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("unknown preallocation mode %s"), preallocation); + goto cleanup; + } + + VIR_FREE(preallocation); + } else { + ret->preallocation = VIR_STORAGE_PREALLOCATION_NONE; + } + ret->target.path = virXPathString("string(./target/path)", ctxt); if (options->formatFromString) { char *format = virXPathString("string(./target/format/@type)", ctxt); @@ -1244,6 +1258,18 @@ virStorageVolDefFormat(virStoragePoolDefPtr pool, virBufferAsprintf(&buf," <allocation unit='bytes'>%llu</allocation>\n", def->allocation); + if (def->preallocation != VIR_STORAGE_PREALLOCATION_NONE) { + const char *preallocation; + + preallocation = virStoragePreallocationModeTypeToString(def->preallocation); + if (!preallocation) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("unexpected preallocation mode")); + goto cleanup; + } + virBufferAsprintf(&buf," <preallocation mode='%s'/>\n", preallocation); + } + if (virStorageVolTargetDefFormat(options, &buf, &def->target, "target") < 0) goto cleanup; diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 9222c4a..c7c7af0 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -98,6 +98,7 @@ struct _virStorageVolDef { virStorageVolSource source; virStorageVolTarget target; virStorageVolTarget backingStore; + int preallocation; /* virStoragePreallocationMode enum */ }; typedef struct _virStorageVolDefList virStorageVolDefList; diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 530071e..8d78d36 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -47,6 +47,10 @@ VIR_ENUM_IMPL(virStorageFileFormat, "cloop", "cow", "dmg", "iso", "qcow", "qcow2", "qed", "vmdk", "vpc") +VIR_ENUM_IMPL(virStoragePreallocationMode, + VIR_STORAGE_PREALLOCATION_LAST, + "", "off", "metadata") + enum lv_endian { LV_LITTLE_ENDIAN = 1, /* 1234 */ LV_BIG_ENDIAN /* 4321 */ diff --git a/src/util/storage_file.h b/src/util/storage_file.h index 13d0e87..dfc8719 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -46,6 +46,16 @@ enum virStorageFileFormat { VIR_ENUM_DECL(virStorageFileFormat); +enum virStoragePreallocationMode { + VIR_STORAGE_PREALLOCATION_NONE, + VIR_STORAGE_PREALLOCATION_OFF, + VIR_STORAGE_PREALLOCATION_METADATA, + + VIR_STORAGE_PREALLOCATION_LAST +}; + +VIR_ENUM_DECL(virStoragePreallocationMode); + typedef struct _virStorageFileMetadata { char *backingStore; int backingStoreFormat; diff --git a/tests/storagevolxml2xmlin/vol-qcow2.xml b/tests/storagevolxml2xmlin/vol-qcow2.xml index b4924de..b4c6522 100644 --- a/tests/storagevolxml2xmlin/vol-qcow2.xml +++ b/tests/storagevolxml2xmlin/vol-qcow2.xml @@ -5,6 +5,7 @@ </source> <capacity unit="G">5</capacity> <allocation>294912</allocation> + <preallocation mode='metadata'/> <target> <path>/var/lib/libvirt/images/OtherDemo.img</path> <format type='qcow2'/> diff --git a/tests/storagevolxml2xmlout/vol-qcow2.xml b/tests/storagevolxml2xmlout/vol-qcow2.xml index 4490931..311c52e 100644 --- a/tests/storagevolxml2xmlout/vol-qcow2.xml +++ b/tests/storagevolxml2xmlout/vol-qcow2.xml @@ -5,6 +5,7 @@ </source> <capacity unit='bytes'>5368709120</capacity> <allocation unit='bytes'>294912</allocation> + <preallocation mode='metadata'/> <target> <path>/var/lib/libvirt/images/OtherDemo.img</path> <format type='qcow2'/> -- 1.7.10.1

On Sat, May 05, 2012 at 02:46:31AM +0200, Marc-André Lureau wrote: by _the_ QCOW2 image format? (needs confirmation from a native speaker I think) Attribute I was about to say this should wait until 0.9.13, good that we agree ;) This is a nit, but it seems to usually be written <optional><ref name='preallocation'/></optional> rather than having the 'optional' tag down the ref definition. preallocation is leaked in this codepath first member could be "none" instead of "" "default" would be a better name for the default state the default behaviour change some day. Christophe