[libvirt] [PATCH 5/6] net: Re-use checks when creating transient networks
Laine Stump
laine at laine.org
Thu Oct 25 21:04:07 UTC 2012
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.
More information about the libvir-list
mailing list