Re: [libvirt] [PATCH 01/11] storage: Refactor the rng schema for storage pool auth
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list