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(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org