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

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