[libvirt] [PATCHv2 09/13] Change virtual network XML parsing/formatting to support IPv6
Eric Blake
eblake at redhat.com
Thu Dec 23 00:16:18 UTC 2010
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 at redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20101222/eeddb959/attachment-0001.sig>
More information about the libvir-list
mailing list