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

[Harsh Bora]

On 10/05/2012 01:13 AM, Eric Blake wrote:
> On 10/04/2012 01:09 PM, Harsh Bora wrote:
> > On 10/04/2012 09:17 PM, Deepak C Shetty wrote:
> > > On 10/04/2012 07:01 PM, Harsh Prateek Bora wrote:
> > > > Qemu accepts gluster protocol as supported storage backend beside
> > > > others.
> > > > This patch allows users to specify disks on gluster backends like this:
> > > >
> > > >       <disk type='network' device='disk'>
> > > >         <driver name='qemu' type='raw'/>
> > > >         <source protocol='gluster' name='volume/image'>
> > > >           <host name='example.org' port='6000' transport='tcp'/>
>
> > > > +                    <attribute name="transport">
> > > > +                      <choice>
> > > > +                        <value>tcp</value>
> > > > +                        <value>unix</value>
> > > > +                        <value>rdma</value>
> > > > +                      </choice>
> > > > +                    </attribute>
> > > 'transport' attribute is optional, so it should be placed inside
> > > <optional> </optional> ?
> >
> > Not necessarily, <zeroOrMore> works for it. You may want to test the
> > patch without specifying 'transport' attribute!
>
> 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.

> > 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 ?

regards,
Harsh

>