On Wed, Dec 05, 2012 at 02:15:17PM -0500, Laine Stump wrote: > This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=767057 > > It was possible to define a network with <forward mode='bridge'> that > had both a bridge device and a forward device defined. These two are > mutually exclusive by definition (if you are using a bridge device, > then this is a host bridge, and if you have a forward dev defined, > this is using macvtap). It was also possible to put <ip>, <dns>, and > <domain> elements in this definition, although those aren't supported > by the current driver (although it's conceivable that some other > driver might support that). > > The items that are invalid by definition, are now checked in the XML > parser (since they will definitely *always* be wrong), and the others > are checked in networkValidate() in the network driver (since, as > mentioned, it's possible that some other network driver, or even this > one, could some day support setting those). I'd be great if the testsuite would check that those invalid combinations don't creep back in. Cheers, -- Guido > --- > src/conf/network_conf.c | 9 +++++++++ > src/libvirt_private.syms | 1 + > src/network/bridge_driver.c | 29 +++++++++++++++++++++++++++++ > 3 files changed, 39 insertions(+) > > diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c > index 6ce2e63..06932d8 100644 > --- a/src/conf/network_conf.c > +++ b/src/conf/network_conf.c > @@ -1577,6 +1577,15 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) > def->name); > goto error; > } > + if (def->bridge && (def->nForwardIfs || nForwardPfs)) { > + virReportError(VIR_ERR_XML_ERROR, > + _("A network with forward mode='%s' can specify " > + "a bridge name or a forward dev, but not " > + "both (network '%s')"), > + virNetworkForwardTypeToString(def->forwardType), > + def->name); > + goto error; > + } > break; > } > } > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms > index 946bb20..bc01fe5 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -857,6 +857,7 @@ virNetworkDefParseString; > virNetworkDeleteConfig; > virNetworkFindByName; > virNetworkFindByUUID; > +virNetworkForwardTypeToString; > virNetworkIpDefNetmask; > virNetworkIpDefPrefix; > virNetworkList; > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c > index e8be00a..0893e9b 100644 > --- a/src/network/bridge_driver.c > +++ b/src/network/bridge_driver.c > @@ -2733,6 +2733,35 @@ networkValidate(struct network_driver *driver, > return -1; > > virNetworkSetBridgeMacAddr(def); > + } else { > + /* They are also the only types that currently support setting > + * an IP address for the host-side device (bridge) > + */ > + if (virNetworkDefGetIpByIndex(def, AF_UNSPEC, 0)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("Unsupported <ip> element in network %s " > + "with forward mode='%s'"), > + def->name, > + virNetworkForwardTypeToString(def->forwardType)); > + return -1; > + } > + if (def->dns && > + (def->dns->ntxtrecords || def->dns->nhosts || def->dns->nsrvrecords)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("Unsupported <dns> element in network %s " > + "with forward mode='%s'"), > + def->name, > + virNetworkForwardTypeToString(def->forwardType)); > + return -1; > + } > + if (def->domain) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("Unsupported <domain> element in network %s " > + "with forward mode='%s'"), > + def->name, > + virNetworkForwardTypeToString(def->forwardType)); > + return -1; > + } > } > > /* We only support dhcp on one IPv4 address per defined network */ > -- > 1.7.11.7 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list