On Wed, Oct 14, 2009 at 12:33:36PM +0100, Daniel P. Berrange wrote:
> On Wed, Oct 14, 2009 at 12:59:53PM +0200, Daniel Veillard wrote:
> > On Wed, Oct 14, 2009 at 10:57:43AM +0100, Daniel P. Berrange wrote:
> > > On Tue, Oct 13, 2009 at 04:14:00PM +0200, Daniel Veillard wrote:
> > > > +
> > > > + /*
> > > > + * check at least the 2 first IP match i.e on same class C subnet
> > > > + */
> > > > + for (i = 0; i < 2;i++) {
> > > > + if (ip4s[i] != ip4e[i]) {
> > > > + virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR,
> > > > + _("start and end of DHCP range do not match '%s' and '%s'"),
> > > > + start, end);
> > > > + return(-1);
> > > > + }
> > > > + }
> > >
> > > Shouldn't we be comparing each of DHCP addresses against the 'netmask'
> > > field we have in virNetworkDef instead. It'd be nice to have a separate
> > > function for this like
> >
> > As far as I can tell the netmask is optional. but if it's there, sure.
>
> Netmask is actually compulsory, although it shouldn't be since there are
> well defined defaults for both IPv4 & 6 :-)
Then we need to fix the schemas !
docs/schemas/network.rng:
<optional>
<attribute name="netmask"><text/></attribute>
</optional>
> >
> > > virSocketAddrInNetwork(struct sockaddr_storage *address,
> > > struct sockaddr_storage *netmask);
> >
> > Hum, I don't understand what that function would do suppose you have
> > 1.2.3.4 and 255.255.255.0 what kind of thing can you do. Sure if you
> > pass 2 addresses then you can check they pertain to the same netmask
> > but that function signature can't work IMHO
>
> Opps, my mistake this should have had 3 args
>
> virSocketAddrInNetwork(struct sockaddr_storage *address,
> struct sockaddr_storage *netaddr,
> struct sockaddr_storage *netmask);
>
Ah, okay, that makes sense.
> We already have some horrible code in virNetworkDefParseXML()
> which can do the netaddr+netmask bit masking for IPv4, to
> calculate a network address.
if I have an array it's trivial. But the struct sockaddr_storage
opaque is weird.
> > > since there's a couple of other places we ought todo this kind of
> > > validation.
> > >
> > > > + ret = ip4e[3] - ip4s[3] + 256 * (ip4e[2] - ip4s[2]);
> > >
> > > It would be nice to have this in a callable function too
> > >
> > > int virSocketAddrRange(struct sockaddr_storage *start,
> > > struct sockaddr_storage *end);
> >
> > Are you supposed to look struct sockaddr_storage ? As posted in my
> > last mail this seems a completely opaque structure at least in theory
> > and if you want to keep the portability it's supposed to bring.
>
> You cast to one of the address specific structs according
> to the ss_family field.
humpf ... okay it has to be cast to be accessed, that's weird,
definitely.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list