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

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

 



On 12/20/2010 06:52 PM, Eric Blake wrote:
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)?

If that's the case,

+/* 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.

If the raw numbers have to be in there anyway, I'd rather just do it by hand for all platforms.

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]