[libvirt] [PATCH] network: prevent a few invalid configuration combinations

[Guido Günther]

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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>