Re: [PATCH 14/28] util: move IP route & address object-related functions to virnetdevip.c

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

 




On 06/22/2016 01:37 PM, Laine Stump wrote:
> These functions all need to be called from a utility function that
> must be located in the util directory, so we move them all into
> util/virnetdevip.[ch] now that it exists.
> 
> Function and struct names were appropriately changed for the new
> location, but all code is unchanged aside from motion and renaming.
> ---
>  src/conf/domain_conf.c            |  36 ++++++-------
>  src/conf/domain_conf.h            |  16 ++----
>  src/conf/network_conf.c           |  16 +++---
>  src/conf/network_conf.h           |   4 +-
>  src/conf/networkcommon_conf.c     | 107 ++++----------------------------------
>  src/conf/networkcommon_conf.h     |  55 +++++++-------------
>  src/libvirt_private.syms          |  16 +++---
>  src/lxc/lxc_container.c           |  12 ++---
>  src/lxc/lxc_native.c              |  12 ++---
>  src/network/bridge_driver.c       |  14 ++---
>  src/network/bridge_driver_linux.c |   6 +--
>  src/util/virnetdevip.c            |  69 ++++++++++++++++++++++++
>  src/util/virnetdevip.h            |  29 +++++++++++
>  13 files changed, 191 insertions(+), 201 deletions(-)
> 

The one naming thing that "could" have changed as well is to keep the
"Def" portion (virNetDevIPRouteDefFree, virNetDevIPRouteDefParseXML,
virNetDevIPRouteDefFormat, virNetDevIPRouteDefCreate).  Generally I'd
say it's a coin flip, but to be consistent since they're handling the
virNetworkRouteDefPtr, then I guess without "knowing" the API names I'd
start searching "NetworkRouteDef{Parse|Format}" in order to find the
code that dealt with it (the libvirt consistency argument).

Maybe they'll happen in a later patch, but why not move the Create,
Parse and Format routines as well? That'd remove networkcommon_conf it
seems.  Could really gone for overkill and generated
virnetdeviproute.{h,c} ~/~

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 4802e03..f380271 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

[...]
> @@ -6266,9 +6266,9 @@ virDomainHostdevDefParseXMLCaps(xmlNodePtr node ATTRIBUTE_UNUSED,
>          if (nroutenodes) {
>              size_t i;
>              for (i = 0; i < nroutenodes; i++) {
> -                virNetworkRouteDefPtr route = NULL;
> +                virNetDevIPRoutePtr route = NULL;
>  
> -                if (!(route = virNetworkRouteDefParseXML(_("Domain hostdev device"),
> +                if (!(route = virNetDevIPRouteParseXML(_("Domain hostdev device"),
>                                                           routenodes[i],
>                                                           ctxt)))

The arguments need vertical alignment on line 2 and 3 under the _...

[...]

> @@ -8955,9 +8955,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>      int ret, val;
>      size_t i;
>      size_t nips = 0;
> -    virDomainNetIPDefPtr *ips = NULL;
> +    virNetDevIPAddrPtr *ips = NULL;
>      size_t nroutes = 0;
> -    virNetworkRouteDefPtr *routes = NULL;
> +    virNetDevIPRoutePtr *routes = NULL;
>  
>      if (VIR_ALLOC(def) < 0)
>          return NULL;
> @@ -9074,7 +9074,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>                      ctxt->node = tmpnode;
>                  }
>              } else if (xmlStrEqual(cur->name, BAD_CAST "ip")) {
> -                virDomainNetIPDefPtr ip = NULL;
> +                virNetDevIPAddrPtr ip = NULL;
>  
>                  if (!(ip = virDomainNetIPParseXML(cur)))
>                      goto error;
> @@ -9082,13 +9082,13 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
>                  if (VIR_APPEND_ELEMENT(ips, nips, ip) < 0)
>                      goto error;
>              } else if (xmlStrEqual(cur->name, BAD_CAST "route")) {
> -                virNetworkRouteDefPtr route = NULL;
> -                if (!(route = virNetworkRouteDefParseXML(_("Domain interface"),
> +                virNetDevIPRoutePtr route = NULL;
> +                if (!(route = virNetDevIPRouteParseXML(_("Domain interface"),
>                                                           cur, ctxt)))

Vertical alignment...


[...]


>  /* Used for prefix of ifname of any network name generated dynamically
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 5ae2bdf..fb2a48d 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c

[...]

> @@ -2261,9 +2261,9 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>              goto error;
>          /* parse each definition */
>          for (i = 0; i < nRoutes; i++) {
> -            virNetworkRouteDefPtr route = NULL;
> +            virNetDevIPRoutePtr route = NULL;
>  
> -            if (!(route = virNetworkRouteDefParseXML(def->name,
> +            if (!(route = virNetDevIPRouteParseXML(def->name,
>                                                       routeNodes[i],
>                                                       ctxt)))

Vertical alignment

[...]

> --- a/src/conf/networkcommon_conf.c
> +++ b/src/conf/networkcommon_conf.c
> @@ -32,34 +32,8 @@
>  

[...]

> -virNetworkRouteDefPtr
> -virNetworkRouteDefCreate(const char *errorDetail,
> +virNetDevIPRoutePtr
> +virNetDevIPRouteCreate(const char *errorDetail,
>                           char *family,
>                           const char *address,
>                           const char *netmask,
> @@ -69,7 +43,7 @@ virNetworkRouteDefCreate(const char *errorDetail,
>                           unsigned int metric,
>                           bool hasMetric)

Vertical alignment

>  {
> -    virNetworkRouteDefPtr def = NULL;
> +    virNetDevIPRoutePtr def = NULL;
>      virSocketAddr testAddr;
>  
>      if (VIR_ALLOC(def) < 0)
> @@ -242,21 +216,21 @@ virNetworkRouteDefCreate(const char *errorDetail,
>      return def;
>  
>   error:
> -    virNetworkRouteDefFree(def);
> +    virNetDevIPRouteFree(def);
>      return NULL;
>  }
>  
> -virNetworkRouteDefPtr
> -virNetworkRouteDefParseXML(const char *errorDetail,
> +virNetDevIPRoutePtr
> +virNetDevIPRouteParseXML(const char *errorDetail,
>                             xmlNodePtr node,
>                             xmlXPathContextPtr ctxt)

Vertical alignment

>  {
>      /*
> -     * virNetworkRouteDef object is already allocated as part
> +     * virNetDevIPRoute object is already allocated as part
>       * of an array.  On failure clear: it out, but don't free it.
>       */
>  
> -    virNetworkRouteDefPtr def = NULL;
> +    virNetDevIPRoutePtr def = NULL;
>      xmlNodePtr save;
>      char *family = NULL;
>      char *address = NULL, *netmask = NULL;
> @@ -302,7 +276,7 @@ virNetworkRouteDefParseXML(const char *errorDetail,
>          }
>      }
>  
> -    def = virNetworkRouteDefCreate(errorDetail, family, address, netmask,
> +    def = virNetDevIPRouteCreate(errorDetail, family, address, netmask,
>                                     gateway, prefix, hasPrefix, metric,
>                                     hasMetric);

Vertical alignment

>  
> @@ -316,8 +290,8 @@ virNetworkRouteDefParseXML(const char *errorDetail,
>  }
>  
>  int
> -virNetworkRouteDefFormat(virBufferPtr buf,
> -                         const virNetworkRouteDef *def)
> +virNetDevIPRouteFormat(virBufferPtr buf,
> +                         const virNetDevIPRoute *def)

Vertical alignment


[...]


> diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
> index 9ad1b08..8294d29 100644
> --- a/src/lxc/lxc_native.c
> +++ b/src/lxc/lxc_native.c
> @@ -419,7 +419,7 @@ typedef struct {
>      char *macvlanmode;
>      char *vlanid;
>      char *name;
> -    virDomainNetIPDefPtr *ips;
> +    virNetDevIPAddrPtr *ips;
>      size_t nips;
>      char *gateway_ipv4;
>      char *gateway_ipv6;
> @@ -430,10 +430,10 @@ typedef struct {
>  static int
>  lxcAddNetworkRouteDefinition(const char *address,
>                               int family,
> -                             virNetworkRouteDefPtr **routes,
> +                             virNetDevIPRoutePtr **routes,
>                               size_t *nroutes)
>  {
> -    virNetworkRouteDefPtr route = NULL;
> +    virNetDevIPRoutePtr route = NULL;
>      char *familyStr = NULL;
>      char *zero = NULL;
>  
> @@ -444,7 +444,7 @@ lxcAddNetworkRouteDefinition(const char *address,
>      if (VIR_STRDUP(familyStr, family == AF_INET ? "ipv4" : "ipv6") < 0)
>          goto error;
>  
> -    if (!(route = virNetworkRouteDefCreate(_("Domain interface"), familyStr,
> +    if (!(route = virNetDevIPRouteCreate(_("Domain interface"), familyStr,
>                                            zero, NULL, address, 0, false,
>                                            0, false)))

Vertical alignment

>          goto error;
> @@ -460,7 +460,7 @@ lxcAddNetworkRouteDefinition(const char *address,
>   error:

[...]


ACK - I think the Def should be added back in and the vertical alignment
things fixed.  A quick scan from yesterday and I found just one instance
in patch 2, src/conf/interface_conf.c for _virInterfaceDef and the
comment column off by 1 vertical char (no big deal).

John

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]