Re: [libvirt] [PATCH 02/13] New virNetworkDef utility functions

On 12/20/2010 01:03 AM, Laine Stump wrote: > Later patches will add the possibility to define a network's netmask > as a prefix (0-32, or 0-128 in the case of IPv6). To make it easier to > deal with definition of both kinds (prefix or netmask), add two new > functions: > > virNetworkDefNetmask: return a copy of the netmask into a > virSocketAddr. If no netmask was specified in the XML, create a > default netmask based on the network class of the virNetworkDef's IP > address. > > virNetworkDefPrefix: return the netmask as numeric prefix (or the > default prefix for the network class of the virNetworkDef's IP > address, if no netmask was specified in the XML) What happens if the user specifies a netmask in the XML that is non-contiguous (bad practice, but some routers do allow it)?

> > +/* return number of 1 bits in netmask for the network's ipAddress, > + * or -1 on error > + */ > +int virNetworkDefPrefix(const virNetworkDefPtr def) Should this return different values, such as -1 if network class not determined, and -2 if netmask was specified but non-contiguous? Or do callers not care? > +{ > + if (VIR_SOCKET_HAS_ADDR(&def->netmask)) { > + return virSocketGetNumNetmaskBits(&def->netmask); [hmm, back to my theme of preferring direct operations over bitwise loops, I notice that the existing GetNumNetmaksBits implementation could be independently optimized] > + } else if (VIR_SOCKET_IS_FAMILY(&def->ipAddress, AF_INET)) { > + /* return the natural prefix for the network's ip address */ > + int addr = ntohl(def->ipAddress.data.inet4.sin_addr.s_addr); > + if (IN_CLASSA(addr)) Where is this defined? Oh, I found it - in<netinet/in.h>, but only as a Linux extension. Since POSIX doesn't require it to exist, you'll need to take care to provide fallback definitions for this macro and its friends.