[libvirt] [PATCH 14/28] util: move IP route & address object-related functions to virnetdevip.c
Laine Stump
laine at laine.org
Fri Jun 24 20:23:55 UTC 2016
On 06/24/2016 07:11 AM, John Ferlan wrote:
>
> 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).
But when the structures get below a certain level of complexity (or
maybe it's that they're nested deep enough, dunno), they tend to lose
the Def suffix - virNetDevBandwith, virNetDevVlan virDomainDeviceInfo
virNetDevVPortProfile...
>
> Maybe they'll happen in a later patch, but why not move the Create,
> Parse and Format routines as well?
except that Parse and Format functions are traditionally kept in the
conf directory. As for the Create, I remember thinking about that one,
but on the first glance it just had so much test that it seemed more
like something from the conf directory. Probably not the right choice,
but I can make a patch to move it later.
> That'd remove networkcommon_conf it
> seems. Could really gone for overkill and generated
> virnetdeviproute.{h,c} ~/~
Yeah, there's a few gigantic files, then several tiny ones. It would be
nice if they were all in the middle somewhere.
>
>> 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
>
More information about the libvir-list
mailing list