Re: [libvirt] [PATCH 14/28] util: move IP route & address object-related functions to virnetdevip.c
On 06/24/2016 04:23 PM, Laine Stump wrote:
> 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...
>
OK nm then... It was just one of those consistency things.
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 14/28] util: move IP route & address object-related functions to virnetdevip.c
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
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
[...]
> ---
