[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/3] Add a "forward family" option



On 15.10.2012 12:27, Benjamin Cama wrote:
> It allows to specify forwarding only for IPv4 or IPv6. Not specifying it
> enables forwarding for both protocols.
> ---
>  src/conf/network_conf.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++-
>  src/conf/network_conf.h |    1 +
>  2 files changed, 58 insertions(+), 1 deletions(-)

When you introduce new feature to XML schema, you must update
docs/schemas/*.rng and docs/format*.html.in

> 
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 891d48c..8c2ae9b 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -1204,6 +1204,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>      xmlNodePtr virtPortNode = NULL;
>      xmlNodePtr forwardNode = NULL;
>      int nIps, nPortGroups, nForwardIfs, nForwardPfs, nForwardAddrs;
> +    char *forwardFamily = NULL;
>      char *forwardDev = NULL;
>      char *forwardManaged = NULL;
>      char *type = NULL;
> @@ -1358,6 +1359,23 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>              def->forwardType = VIR_NETWORK_FORWARD_NAT;
>          }
>  
> +        forwardFamily = virXPathString("string(./@family)", ctxt);
> +        if (forwardFamily) {
> +            if (STREQ(forwardFamily, "ipv4")) {
> +                def->forwardFamily = AF_INET;
> +            } else if (STREQ(forwardFamily, "ipv6")) {
> +                def->forwardFamily = AF_INET6;
> +            } else {
> +                virNetworkReportError(VIR_ERR_XML_ERROR,
> +                                      _("Unknown forward family '%s'"), forwardFamily);

%s/virNetworkReportError/virReportError/

> +                VIR_FREE(forwardFamily);
> +                goto error;
> +            }
> +            VIR_FREE(forwardFamily);
> +        } else {
> +            def->forwardFamily = AF_UNSPEC;
> +        }
> +

Usually, we create an enum for this and use vir.*TypeFromString() and
vir.*TypeToString();

>          forwardDev = virXPathString("string(./@dev)", ctxt);
>          forwardManaged = virXPathString("string(./@managed)", ctxt);
>          if(forwardManaged != NULL) {
> @@ -1515,8 +1533,16 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>          VIR_FREE(forwardIfNodes);
>          VIR_FREE(forwardAddrNodes);
>          switch (def->forwardType) {
> -        case VIR_NETWORK_FORWARD_ROUTE:
>          case VIR_NETWORK_FORWARD_NAT:
> +            if (def->forwardFamily == AF_INET6) {
> +                virNetworkReportError(VIR_ERR_XML_ERROR,
> +                                      _("%s forwarding is not allowed in IPv6 (network '%s')"),
> +                                      virNetworkForwardTypeToString(def->forwardType),
> +                                      def->name);
> +                goto error;
> +            }
> +            /* fall through to next case */
> +        case VIR_NETWORK_FORWARD_ROUTE:
>              /* It's pointless to specify L3 forwarding without specifying
>               * the network we're on.
>               */
> @@ -1527,6 +1553,25 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>                                 def->name);
>                  goto error;
>              }
> +            /* If forwarding for one family only, an address from this family
> +             * must be present
> +             */
> +            if (def->forwardFamily) {
> +                int ii;
> +                for (ii = 0; ii < def->nips; ii++) {
> +                    if (VIR_SOCKET_ADDR_IS_FAMILY(&def->ips[ii].address, def->forwardFamily))
> +                        break;
> +                }
> +                if (ii == def->nips) {

useless 'if'.

> +                    char ipVersion = def->forwardFamily == AF_INET6 ? '6' : '4';
> +                    virNetworkReportError(VIR_ERR_XML_ERROR,
> +                                          _("%s IPv%c forwarding requested, but no IPv%c address provided for network '%s'"),
> +                                          virNetworkForwardTypeToString(def->forwardType),
> +                                          ipVersion, ipVersion,
> +                                          def->name);
> +                    goto error;
> +                }
> +            }
>              if (def->nForwardIfs > 1) {
>                  virReportError(VIR_ERR_XML_ERROR,
>                                 _("multiple forwarding interfaces specified for network '%s', only one is supported"),
> @@ -1555,6 +1600,13 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>                                 def->name);
>                  goto error;
>              }
> +            if (def->forwardFamily) {
> +                virNetworkReportError(VIR_ERR_XML_ERROR,
> +                                      _("bridge forward family option only allowed in route and nat mode, not in %s (network '%s')"),
> +                                      virNetworkForwardTypeToString(def->forwardType),
> +                                      def->name);
> +                goto error;
> +            }
>              break;
>          }
>      }
> @@ -1830,6 +1882,8 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags)
>          if (!def->nForwardPfs)
>              dev = virNetworkDefForwardIf(def, 0);
>          const char *mode = virNetworkForwardTypeToString(def->forwardType);
> +        const char *family = (def->forwardFamily == AF_INET ? "ipv4" :
> +                                def->forwardFamily == AF_INET6 ? "ipv6" : NULL);
>  
>          if (!mode) {
>              virReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -1846,6 +1900,8 @@ char *virNetworkDefFormat(const virNetworkDefPtr def, unsigned int flags)
>              else
>                  virBufferAddLit(&buf, " managed='no'");
>          }
> +        if (family)
> +            virBufferEscapeString(&buf, " family='%s'", family);

Here is the best place for vir.*TypeToString().

>          virBufferAsprintf(&buf, "%s>\n",
>                            (def->nForwardIfs || def->nForwardPfs) ? "" : "/");
>          virBufferAdjustIndent(&buf, 2);
> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> index 55502fb..61ecbc1 100644
> --- a/src/conf/network_conf.h
> +++ b/src/conf/network_conf.h
> @@ -186,6 +186,7 @@ struct _virNetworkDef {
>  
>      int forwardType;    /* One of virNetworkForwardType constants */
>      int managed;        /* managed attribute for hostdev mode */
> +    int forwardFamily;  /* AF_INET or AF_INET6 - AF_UNSPEC for both */
>  
>      /* If there are multiple forward devices (i.e. a pool of
>       * interfaces), they will be listed here.
> 

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]