[libvirt] [PATCH] storage: expose volume meta-type in XML

[Osier Yang]

On 23/11/13 06:26, Eric Blake wrote:
> I got annoyed at having to use both 'virsh vol-list $pool --details'
> AND 'virsh vol-dumpxml $vol $pool' to learn if I had populated
> the volume correctly.  Since two-thirds of the data present in
> virStorageVolGetInfo() already appears in virStorageVolGetXMLDesc(),
> this just adds the remaining piece of information.
>
> * docs/formatstorage.html.in: Document new <target type=...>.

I didn't see it relates with "target".

> * docs/schemas/storagevol.rng (target, backingStore): Add it to
> RelaxNG.

I thought "(target, backingStore)" means add "type" to both
of them.  Finally see it means "between"  :-)

> * src/conf/storage_conf.h (virStorageVolTypeToString): Declare.
> * src/conf/storage_conf.c (virStorageVolTargetDefFormat): Output
> the metatype.
> * tests/storagevolxml2xmlout/vol-*.xml: Update tests to match.
>
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>
> Depends on:
> https://www.redhat.com/archives/libvir-list/2013-November/msg00948.html
>
>   docs/formatstorage.html.in                         |  5 +++++
>   docs/schemas/storagevol.rng                        | 15 +++++++++++++++
>   src/conf/storage_conf.c                            | 18 ++++++++++++++++++
>   src/conf/storage_conf.h                            |  1 +
>   tests/storagevolxml2xmlin/vol-logical-backing.xml  |  1 +
>   tests/storagevolxml2xmlin/vol-logical.xml          |  1 +
>   tests/storagevolxml2xmlin/vol-partition.xml        |  1 +
>   tests/storagevolxml2xmlin/vol-sheepdog.xml         |  1 +
>   tests/storagevolxml2xmlout/vol-file-backing.xml    |  1 +
>   tests/storagevolxml2xmlout/vol-file-naming.xml     |  1 +
>   tests/storagevolxml2xmlout/vol-file.xml            |  1 +
>   tests/storagevolxml2xmlout/vol-logical-backing.xml |  1 +
>   tests/storagevolxml2xmlout/vol-logical.xml         |  1 +
>   tests/storagevolxml2xmlout/vol-partition.xml       |  1 +
>   tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml |  1 +
>   tests/storagevolxml2xmlout/vol-qcow2-1.1.xml       |  1 +
>   tests/storagevolxml2xmlout/vol-qcow2-lazy.xml      |  1 +
>   tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml |  1 +
>   tests/storagevolxml2xmlout/vol-qcow2.xml           |  1 +
>   tests/storagevolxml2xmlout/vol-sheepdog.xml        |  1 +
>   20 files changed, 55 insertions(+)
>
> diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
> index 90eeaa3..5f277b4 100644
> --- a/docs/formatstorage.html.in
> +++ b/docs/formatstorage.html.in
> @@ -293,6 +293,7 @@
>         <volume>
>           <name>sparse.img</name>
>           <key>/var/lib/xen/images/sparse.img</key>
> +        <type>file</type>
>           <allocation>0</allocation>
>           <capacity unit="T">1</capacity>
>           ...</pre>
> @@ -305,6 +306,10 @@
>         <dd>Providing an identifier for the volume which is globally unique.
>             This cannot be set when creating a volume: it is always generated.
>           <span class="since">Since 0.4.1</span></dd>
> +      <dt><code>type</code></dt>
> +      <dd>Output-only; provides the volume type that is also available
> +        from <code>virStorageVolGetInfo()</code>.  <span class="since">Since

I think it's better to mention "virsh vol-list $pool --details" instead 
of the
API name here, as we did across the documents.  I'm fine if you keep it
though.

> +        1.1.5</span></dd>
>         <dt><code>allocation</code></dt>
>         <dd>Providing the total storage allocation for the volume. This
>           may be smaller than the logical capacity if the volume is sparsely
> diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
> index e79bc35..96572c5 100644
> --- a/docs/schemas/storagevol.rng
> +++ b/docs/schemas/storagevol.rng
> @@ -25,6 +25,9 @@
>           <optional>
>             <ref name='source'/>
>           </optional>
> +        <optional>
> +          <ref name='voltype'/>
> +        </optional>
>           <ref name='sizing'/>
>           <ref name='target'/>
>           <optional>
> @@ -34,6 +37,18 @@
>       </element>
>     </define>
>
> +  <define name='voltype'>
> +    <element name='type'>
> +      <choice>
> +        <value>file</value>
> +        <value>block</value>
> +        <value>dir</value>
> +        <value>network</value>
> +        <value>network-dir</value>

What's "network-dir" type? the type you will introduce in the glusterfs 
series?
It's not in the git head yet.  So either you will need to remove it, or 
push this
patch after the glusterfs series.

> +      </choice>
> +    </element>
> +  </define>
> +
>     <define name='sizing'>
>       <interleave>
>         <optional>
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 8b378c2..0d2932b 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -51,6 +51,10 @@
>   #define DEFAULT_POOL_PERM_MODE 0755
>   #define DEFAULT_VOL_PERM_MODE  0600
>
> +VIR_ENUM_IMPL(virStorageVol,
> +              VIR_STORAGE_VOL_LAST,
> +              "file", "block", "dir", "network")

Here the "network-dir" type is not included though. So I guess you want
to push this patch before the glusterfs series.

ACK if the "network-dir" is removed.