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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/