On Sat, May 05, 2012 at 02:46:31AM +0200, Marc-André Lureau wrote:
> 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.
by _the_ QCOW2 image format? (needs confirmation from a native speaker I
think)
> + Attribe <code>mode</code> value can be 'off' or 'metadata'.
Attribute
> + <span class="since">Since 0.9.13</span></dd>
I was about to say this should wait until 0.9.13, good that we agree ;)
> <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'/>
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.
> </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..95849ec 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);
preallocation is leaked in this codepath
> + 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 pool type"));
> + 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")
first member could be "none" instead of ""
"default" would be a better name for the default state the default
behaviour change some day.
Christophe
> +
> 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
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
pgpm0IVf_Y1Lm.pgp
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list