[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]