[Gluster-devel] [libvirt] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.

[Harsh Bora]

On 10/05/2012 02:07 AM, Eric Blake wrote:
> On 10/04/2012 01:56 PM, Harsh Bora wrote:
>
>>> Deepak is right - the <attribute name="transport"> block must be
>>> embedded in an <optional> block.  <zeroOrMore> only applies to elements,
>>> but as written, your zeroOrMore says that it is okay to omit the overall
>>> <host> element, but that if you provide a <host> element, it MUST have a
>>> transport='...' attribute, and that is not correct.  For back-compat
>>> reasons, ALL new attributes must be in an <optional> block since older
>>> clients of the XML did not provide the new attributes.
>>
>> Looking at the grammar, it appeared to me like what you said, however,
>> when I tested the patch, zeroOrMore worked successfully for the
>> transport attribute as well. However, I am willing to use <optional>
>> block for consistency.
>
> Hmm, I guess I'd have to read up on the actual grammar of RNG notation
> to see if using <zeroOrMore> as an alternate spelling of <optional> for
> fixed-name attributes is kosher or just something that our particular
> RNG validation engine permits, but it's certainly not typical usage,
> when you realize that there will never be more than one of that
> attribute.  At any rate, we've hashed this topic to death :)

Hm, I think its better to have <optional> even if the xml works 
otherwise, since this is how one would expect the grammar to be written 
for an optional attribute.

>
>>
>>>
>>>>
>>>> I chose to check for only ':' to decide if its a IPv6 addr because it
>>>> doesnt make sense to be partial towards '.' What if someone specifies a
>>>> host name like 12:12;12,12 or 23:23,23,23 ? A '.' in an IPv6 addr is as
>>>> bad as any other invalid char.
>>>
>>> '.' is valid in IPv6 addr.  But yes, ':' is mandatory in IPv6, and
>>> forbidden in IPv4, so it makes a good distinguishing test between the
>>> two families.
>>
>> So, are you suggesting to validate IPv4 only and that too based on the
>> absence of ':' and presence of '.'? Does that really suffice to validate
>> an IPv4 since any other special character is also an invalid separator
>> for IPv4 ?
>
> Ultimately, you want to accept both types of IP addresses.  I was just
> replying to your quote about '.' not being valid in IPv6 as being an
> incorrect statement; I don't really know what the best separator
> character is that you want to be using in this context, because I
> haven't been following the original conversation closely enough.  You
> should also realize that hostnames cannot contain ':' or ';' (I'm not
> sure about ','), so your question about someone setting a hostname to
> '12:12;12,12' to confuse the parser is not worth worrying about.
>

Ok, I understand that IPv6 can have a '.' though ':' is mandatory in 
IPv6, and the current code should work for IPv6 addresses with a . as 
well. However, I am not sure if we need to validate the user provided 
hostname/IPv4/IPv6 addresses if it contains an invalid separator.

Also, the parsing code for IPv4 should work for hostnames as well. By 
talking about IP address with diff separators, I just meant to say that 
presence of . and absence of : doesnt necessarily mean a valid IPv4 
address since presence of other invalid separator would also mean an 
invalid IPv4 address.

regards,
Harsh