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

[John Ferlan]

On 09/26/2014 06:56 AM, John Ferlan wrote:
> 
> 
> 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
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

I pushed the following for you

John


diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index 2d165a3..0f05c5c 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> <!-- back-compat requires keeping 'unknown' not 'auto' -->
             <value>lvm2</value>
           </choice>
         </attribute>
diff --git a/docs/storage.html.in b/docs/storage.html.in
index 3d2ffca..9933548 100644
--- a/docs/storage.html.in
+++ b/docs/storage.html.in
@@ -328,14 +328,10 @@
 
     <h3>Valid pool format types</h3>
     <p>
-      The logical volume pool supports the following formats:
+      The logical volume pool supports only the <code>lvm2</code> format,
+      although not supplying a format value will result in automatic
+      selection of the<code>lvm2</code> format.
     </p>
-    <ul>
-      <li><code>auto</code> - automatically determine format</li>
-      <li>
-        <code>lvm2</code>
-      </li>
-    </ul>
 
     <h3>Valid volume format types</h3>
     <p>
-- 
1.9.3