On 10/25/2012 11:18 AM, Peter Krempa wrote: > When a transient network was created some of the checks weren't run on > the definition allowing to start invalid networks. > > This patch splits out code to the network validation function and > re-uses that code when creating transient networks. Nice cleanup / fix! There are a few other problems with transient networks that I've been meaning to fix in order to bring their level of operation up to parity with transient domains. See https://bugzilla.redhat.com/show_bug.cgi?id=819416 for example. > --- > src/network/bridge_driver.c | 96 +++++++++++++++++++-------------------------- > 1 file changed, 40 insertions(+), 56 deletions(-) > > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c > index 45fca0e..e90444d 100644 > --- a/src/network/bridge_driver.c > +++ b/src/network/bridge_driver.c > @@ -2689,11 +2689,48 @@ cleanup: > > > static int > -networkValidate(virNetworkDefPtr def) > +networkValidate(struct network_driver *driver, > + virNetworkDefPtr def, > + bool check_active) > { > int ii; > bool vlanUsed, vlanAllowed; > const char *defaultPortGroup = NULL; > + virNetworkIpDefPtr ipdef; > + bool ipv4def = false; > + int i; > + > + /* check for duplicate networks */ > + if (virNetworkObjIsDuplicate(&driver->networks, def, check_active) < 0) > + return -1; > + > + /* Only the three L3 network types that are configured by libvirt > + * need to have a bridge device name / mac address provided > + */ > + if (def->forwardType == VIR_NETWORK_FORWARD_NONE || > + def->forwardType == VIR_NETWORK_FORWARD_NAT || > + def->forwardType == VIR_NETWORK_FORWARD_ROUTE) { > + > + if (virNetworkSetBridgeName(&driver->networks, def, 1)) > + return -1; > + > + virNetworkSetBridgeMacAddr(def); Well, those aren't strictly "validating the network", but are rather auto-setting some attributes. But since they are required in both cases, and happen at the same time we are validating that the network config is okay, I guess it's reasonable to put it here. > + } > + > + /* We only support dhcp on one IPv4 address per defined network */ > + for (i = 0; (ipdef = virNetworkDefGetIpByIndex(def, AF_INET, i)); i++) { > + if (ipdef->nranges || ipdef->nhosts) { > + if (ipv4def) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("Multiple dhcp sections found. " > + "dhcp is supported only for a " > + "single IPv4 address on each network")); > + return -1; > + } else { > + ipv4def = true; > + } > + } > + } > > /* The only type of networks that currently support transparent > * vlan configuration are those using hostdev sr-iov devices from > @@ -2747,23 +2784,7 @@ static virNetworkPtr networkCreate(virConnectPtr conn, const char *xml) { > if (!(def = virNetworkDefParseString(xml))) > goto cleanup; > > - if (virNetworkObjIsDuplicate(&driver->networks, def, true) < 0) > - goto cleanup; > - > - /* Only the three L3 network types that are configured by libvirt > - * need to have a bridge device name / mac address provided > - */ > - if (def->forwardType == VIR_NETWORK_FORWARD_NONE || > - def->forwardType == VIR_NETWORK_FORWARD_NAT || > - def->forwardType == VIR_NETWORK_FORWARD_ROUTE) { > - > - if (virNetworkSetBridgeName(&driver->networks, def, 1)) > - goto cleanup; > - > - virNetworkSetBridgeMacAddr(def); > - } > - > - if (networkValidate(def) < 0) > + if (networkValidate(driver, def, true) < 0) > goto cleanup; > > /* NB: "live" is false because this transient network hasn't yet > @@ -2793,54 +2814,17 @@ cleanup: > > static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { > struct network_driver *driver = conn->networkPrivateData; > - virNetworkIpDefPtr ipdef, ipv4def = NULL; > virNetworkDefPtr def; > bool freeDef = true; > virNetworkObjPtr network = NULL; > virNetworkPtr ret = NULL; > - int ii; > > networkDriverLock(driver); > > if (!(def = virNetworkDefParseString(xml))) > goto cleanup; > > - if (virNetworkObjIsDuplicate(&driver->networks, def, false) < 0) > - goto cleanup; > - > - /* Only the three L3 network types that are configured by libvirt > - * need to have a bridge device name / mac address provided > - */ > - if (def->forwardType == VIR_NETWORK_FORWARD_NONE || > - def->forwardType == VIR_NETWORK_FORWARD_NAT || > - def->forwardType == VIR_NETWORK_FORWARD_ROUTE) { > - > - if (virNetworkSetBridgeName(&driver->networks, def, 1)) > - goto cleanup; > - > - virNetworkSetBridgeMacAddr(def); > - } > - > - /* We only support dhcp on one IPv4 address per defined network */ > - for (ii = 0; > - (ipdef = virNetworkDefGetIpByIndex(def, AF_UNSPEC, ii)); > - ii++) { > - if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) { > - if (ipdef->nranges || ipdef->nhosts) { > - if (ipv4def) { > - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > - _("Multiple dhcp sections found. " > - "dhcp is supported only for a " > - "single IPv4 address on each network")); > - goto cleanup; > - } else { > - ipv4def = ipdef; > - } > - } > - } > - } > - > - if (networkValidate(def) < 0) > + if (networkValidate(driver, def, false) < 0) > goto cleanup; > > if (!(network = virNetworkAssignDef(&driver->networks, def, false))) ACK. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list