[libvirt] [PATCHv2] storage: Fix logical pool fmt type

[John Ferlan]

On 09/25/2014 10:26 AM, Erik Skultety wrote:
> According to our documentation logical pool supports formats 'auto' and
> 'lvm2'. However, in storage_conf.c we prevously defined storage pool

s/prevously/previously

> formats: unknown, lvm2. Due to backward compatibility reasons
> documentation now refers to pool format type 'unknown' instead of 'auto'.

could be modified depending on how you handle my comment below.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1123767
> ---
>  docs/schemas/storagepool.rng | 2 +-
>  docs/storage.html.in         | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
> index 2d165a3..7234ef3 100644
> --- a/docs/schemas/storagepool.rng
> +++ b/docs/schemas/storagepool.rng
> @@ -465,7 +465,7 @@
>        <element name='format'>
>          <attribute name='type'>
>            <choice>
> -            <value>auto</value>
> +            <value>unknown</value>

Perhaps in order to avoid someone "in the future" getting us back into
this mess - can we add a comment after the </value>:

"<!-- back-compat requires keeping 'unknown' not 'auto' -->"

There's a few other examples of back-compat comments...

>              <value>lvm2</value>
>            </choice>
>          </attribute>
> diff --git a/docs/storage.html.in b/docs/storage.html.in
> index 3d2ffca..49fd862 100644
> --- a/docs/storage.html.in
> +++ b/docs/storage.html.in
> @@ -331,7 +331,7 @@
>        The logical volume pool supports the following formats:
>      </p>
>      <ul>
> -      <li><code>auto</code> - automatically determine format</li>
> +      <li><code>unknown</code> - automatically determine format</li>

I think if you follow what 'virStoragePoolFormatDisk' does (or Disk
volume pools on the webpage) and just don't list 'unknown' that'd
probably be better.  Unless someone else thinks it should be listed.
Yes, a list of 1 element looks strange.  If that's not desired some text
indicating that logical pools only support the 'lvm2' type and if format
is not provided, then libvirt will determine the type.


>        <li>
>          <code>lvm2</code>
>        </li>
> 

ACK

Let's see if anyone else has feelings one way or another - I can modify
based on my review and push so you don't have to send a v3.  Just want
to give others a chance first...

John