On 12/22/2010 11:58 AM, Laine Stump wrote:
> At this point everything is already in place to make IPv6 happen, we just
> need to add a few rules, remove some checks for IPv4-only, and document
> the changes to the XML on the website.
> ---
> No changes from V1.
>
> docs/formatnetwork.html.in | 35 +++++++--
Yeah - a patch series with documentation updates!
> static int
> networkAddGeneralIptablesRules(struct network_driver *driver,
> virNetworkObjPtr network)
> @@ -926,6 +985,11 @@ networkAddGeneralIptablesRules(struct network_driver *driver,
> goto err8;
> }
>
> + /* add IPv6 general rules, if needed */
> + if (networkAddGeneralIp6tablesRules(driver, network) < 0) {
> + goto err8;
Should this be err9, with a step that undoes the previous action when
you get past the err8 failure point?
> + if (virAsprintf(&field, SYSCTL_PATH "/net/ipv6/conf/%s/disable_ipv6",
> + network->def->bridge) < 0) {
...
> + if (virFileWriteStr(field, "1", 0) < 0) {
> + virReportSystemError(errno,
> + _("cannot enable %s"), field);
Misleading message; maybe "cannot write to %s to disable IPv6".
> @@ -1755,7 +1845,8 @@ cleanup:
> static int networkUndefine(virNetworkPtr net) {
> struct network_driver *driver = net->conn->networkPrivateData;
> virNetworkObjPtr network;
> - virNetworkIpDefPtr ipv4def;
> + virNetworkIpDefPtr ipdef;
> + int v4present = 0, v6present = 0;
s/int/bool/
> @@ -1780,12 +1871,17 @@ static int networkUndefine(virNetworkPtr net) {
>
> /* we only support dhcp on one IPv4 address per defined network */
> for (ii = 0;
> - (ipv4def = virNetworkDefGetIpByIndex(network->def, AF_INET, ii));
> + (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii));
> ii++) {
> - if (ipv4def->nranges || ipv4def->nhosts)
> - break;
> + if (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET)) {
> + if (ipdef->nranges || ipdef->nhosts)
> + v4present = 1;
At first glance, this logic didn't sound right. You only set v4present
if you found a dhcp interface, ignoring other ipv4 interfaces. Then
again,...
> + } else if (VIR_SOCKET_IS_FAMILY(&ipdef->address, AF_INET6)) {
> + v6present = 1;
> + }
> }
> - if (ipv4def) {
> +
> + if (v4present) {
> dnsmasqContext *dctx = dnsmasqContextNew(network->def->name, DNSMASQ_STATE_DIR);
you only use it to disable dnsmasq rather than all things related to
IPv4, so maybe it would be better to rename the variable to dhcp_present
instead of v4present.
ACK with those nits addressed.
--
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