[libvirt] [PATCH 01/11] storage: Refactor the rng schema for storage pool auth

[Osier Yang]

On 20/06/13 14:18, Osier Yang wrote:
> On 06/06/13 22:14, John Ferlan wrote:
>> On 05/28/2013 02:39 AM, Osier Yang wrote:
>>> The attributes/elements for auth type "chap" and "ceph" are complete
>>> different, this separates them into groups.
>> s/complete/completely/
>> s/this separates/these patches separate/
>>
>>> And add "interleave" for "login" and "passwd" attributes of "chap"
>>> type auth.
>> s/And add/Added
>>
>> or
>> Changed "chap" type "login" and "passwd" attributes to be be 
>> interleaved.
>>
>>
>> The only question/comment below is the 'intention' of removing the
>> "optional" attribute from 'passwd' and 'sourceinfoauthsecret'.
>>
>>
>>> ---
>>>   docs/schemas/storagepool.rng | 42 
>>> ++++++++++++++++++++++--------------------
>>>   1 file changed, 22 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/docs/schemas/storagepool.rng 
>>> b/docs/schemas/storagepool.rng
>>> index 3c2158a..2595e37 100644
>>> --- a/docs/schemas/storagepool.rng
>>> +++ b/docs/schemas/storagepool.rng
>>> @@ -280,28 +280,30 @@
>>>       <define name='sourceinfoauth'>
>>>       <element name='auth'>
>>> -      <attribute name='type'>
>>> -        <choice>
>>> -          <value>chap</value>
>>> -          <value>ceph</value>
>>> -        </choice>
>>> -      </attribute>
>>>         <choice>
>>> -        <attribute name='login'>
>>> -          <text/>
>>> -        </attribute>
>>> -        <attribute name='username'>
>>> -          <text/>
>>> -        </attribute>
>>> +        <group>
>>> +          <attribute name='type'>
>>> +            <value>chap</value>
>>> +          </attribute>
>>> +          <interleave>
>>> +            <attribute name='login'>
>>> +              <text/>
>>> +            </attribute>
>>> +            <attribute name='passwd'>
>>> +              <text/>
>>> +            </attribute>
>>> +          </interleave>
>>> +        </group>
>>> +        <group>
>>> +          <attribute name='type'>
>>> +            <value>ceph</value>
>>> +          </attribute>
>>> +          <attribute name='username'>
>>> +            <text/>
>>> +          </attribute>
>>> +          <ref name='sourceinfoauthsecret'/>
>>> +       </group>
>>>         </choice>
>>> -      <optional>
>>> -        <attribute name='passwd'>
>>> -          <text/>
>>> -        </attribute>
>>> -      </optional>
>>> -      <optional>
>>> -        <ref name='sourceinfoauthsecret'/>
>>> -      </optional>
>> Both of these changed to have to be non-optional... Reading the
>> "formatdomain.html" page is "confusing" at best since 'passwd' isn't
>> mentioned.
>
> "passwd" is actually mandatory, see virStoragePoolDefParseAuthChap

I misunderstood you a bit, "passwd" is never supported by domain, it's
the right thing, plain password is always not good for security.

<...>
the domain XML intentionally does not expose the password, only the 
reference to the object that does manage the password
</...>

>
> same for "sourceinfoauthsecret", see virStoragePoolDefParseAuthCephx

domain doesn't do the checking for requirement of either "uuid" or "usage",
but it's the thing it should do.  I don't see any reason why it doesn't 
require
it for a "ceph" type auth.

>
>>
>> It would seem to me that the formatdomain page should also be updated
>> based on what I see here as part of this change.
>
> You should read formatstorage.html.in, unfortunately, it's a history 
> problem,
> we lack of documents for most of the storage stuffs, we should do it 
> later,
> but it will waste lots of time to figure out the right documents, 
> which I don't
> want to touch at this stage..
>
> Osier
>
> -- 
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list