On 12/22/2010 11:58 AM, Laine Stump wrote: > This commit adds support for IPv6 parsing and formatting to the > virtual network XML parser, including moving around data definitions > to allow for multiple <ip> elements on a single network, but only > changes the consumers of this API to accomodate for the changes in s/accomodate/accommodate/ > API/structure, not to add any actual IPv6 functionality. That will > come in a later patch - this patch attempts to maintain the same final > functionality in both drivers that use the network XML parser - vbox > and "bridge" (the Linux bridge-based driver used by the qemu > hypervisor driver). > > * src/libvirt_private.syms: Add new private API functions. > * src/conf/network_conf.[ch]: Change C data structure and > parsing/formatting. > * src/network/bridge_driver.c: Update to use new parser/formatter. > * src/vbox/vbox_tmpl.c: update to use new parser/formatter > * docs/schemas/network.rng: changes to the schema - > * there can now be more than one <ip> element. > * ip address is now an ip-addr (ipv4 or ipv6) rather than ipv4-addr > * new optional "prefix" attribute that can be used in place of "netmask" > * new optional "family" attribute - "ipv4" or "ipv6" > (will default to ipv4) > * define data types for the above > * tests/networkxml2xml(in|out)/nat-network.xml: add multiple <ip> elements > (including IPv6) to a single network definition to verify they are being > correctly parsed and formatted. > + <!-- Based on http://blog.mes-stats.fr/2008/10/09/regex-ipv4-et-ipv6 --> > + <define name='ipv6-addr'> > + <data type='string'> > + <!-- To understand this better, take apart the toplevel '|'s --> > + <param name="pattern"> (([0-9A-Fa-f]{1,4}:){7}[0-9A-Fa-f]{1,4}) |(([0-9A-Fa-f]{1,4}:){6}:[0-9A-Fa-f]{1,4}) |(([0-9A-Fa-f]{1,4}:){5}:([0-9A-Fa-f]{1,4}:)?[0-9A-Fa-f]{1,4}) |(([0-9A-Fa-f]{1,4}:){4}:([0-9A-Fa-f]{1,4}:){0,2}[0-9A-Fa-f]{1,4}) |(([0-9A-Fa-f]{1,4}:){3}:([0-9A-Fa-f]{1,4}:){0,3}[0-9A-Fa-f]{1,4}) |(([0-9A-Fa-f]{1,4}:){2}:([0-9A-Fa-f]{1,4}:){0,4}[0-9A-Fa-f]{1,4}) |(([0-9A-Fa-f]{1,4}:){6}((((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2})))\.){3}(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2})))) This doesn't properly represent the dotted IPv4 suffix (it allows abcd::00.00.00.00). This should reuse the IPv4 pattern (swap 200 to occur in the pattern before 100, and avoid double 0): (((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9])) |(([0-9A-Fa-f]{1,4}:){0,5}:((((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2})))\.){3}(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2})))) same problem with the IPv4 portion |(::([0-9A-Fa-f]{1,4}:){0,5}((((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2})))\.){3}(((25[0-5])|(1[0-9]{2})|(2[0-4][0-9])|([0-9]{1,2})))) and again |([0-9A-Fa-f]{1,4}::([0-9A-Fa-f]{1,4}:){0,5}[0-9A-Fa-f]{1,4}) |(::([0-9A-Fa-f]{1,4}:){0,6}[0-9A-Fa-f]{1,4}) |(([0-9A-Fa-f]{1,4}:){1,7}:) Wow - that's quite the strict regexp. I probably would have copped out and done the much shorter [:0-9a-fA-F.]+ and filter out the non-IPv6 strings later, but since you already have the regex, we might as well keep it. > void virNetworkDefFree(virNetworkDefPtr def) > { > - int i; > + int ii; This rename looks funny. > + /* parse/validate netmask */ > + if (netmask) { > + if (address == NULL) { > + /* netmask is meaningless without an address */ > + virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("netmask specified without address in network '%s'"), > + networkName); > + goto error; > + } > + > + if (!VIR_SOCKET_IS_FAMILY(&def->address, AF_INET)) { > + virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("netmask not supported for address '%s' in network '%s' (IPv4 only)"), > + address, networkName); > + goto error; > + } > + > + if (def->prefix > 0) { > + /* can't have both netmask and prefix at the same time */ > + virNetworkReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("network '%s' has prefix='%u' but no address"), > + networkName, def->prefix); Should this message be "network '%s' cannot have both prefix='%u' and a netmask"? > diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h > index a922d28..a51794d 100644 > --- a/src/conf/network_conf.h > +++ b/src/conf/network_conf.h > @@ -56,6 +56,33 @@ struct _virNetworkDHCPHostDef { > virSocketAddr ip; > }; > > +typedef struct _virNetworkIpDef virNetworkIpDef; > +typedef virNetworkIpDef *virNetworkIpDefPtr; > +struct _virNetworkIpDef { > + char *family; /* ipv4 or ipv6 - default is ipv4 */ > + virSocketAddr address; /* Bridge IP address */ > + > + /* The first two items below are taken directly from the XML (one > + * or the other is given, but not both) and the 3rd is derived > + * from the first two. When formatting XML, always use netMasktStr Typo in netMasktStr? > + int nips; s/int/size_t/ > + virNetworkIpDefPtr ips; /* ptr to array of IP addresses on this network */ > }; > > typedef struct _virNetworkObj virNetworkObj; > +virNetworkIpDefPtr > +virNetworkDefGetIpByIndex(const virNetworkDefPtr def, > + int family, int n); Should n be size_t? > virNetworkFindByUUID; > virNetworkLoadAllConfigs; > +virNetworkIpDefNetmask; > +virNetworkIpDefPrefix; Sorting. Close call as to whether I pointed out enough things to warrant a v3, or if you should just fix them. -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list