Re: [PATCH libvirt 1/2] storage: add preallocation element

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]