Use the structure names in the @data setup - makes it easier that going
back to find the struct.
Use the @maxnames instead of @nnames since that's what it is.
Modify the @filter to be @aclfilter and change the typedef from
virNetworkObjListFilter to virNetworkObjListACLFilter.
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
src/conf/virnetworkobj.c | 39 ++++++++++++++++++++++-----------------
src/conf/virnetworkobj.h | 12 ++++++------
src/network/bridge_driver.c | 8 ++++----
src/test/test_driver.c | 8 ++++----
4 files changed, 36 insertions(+), 31 deletions(-)
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index fa1e396..cc45a00 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1370,7 +1370,7 @@ virNetworkMatch(virNetworkObjPtr obj,
struct virNetworkObjListData {
virConnectPtr conn;
virNetworkPtr *nets;
- virNetworkObjListFilter filter;
+ virNetworkObjListACLFilter aclfilter;
unsigned int flags;
int nnets;
bool error;
@@ -1390,8 +1390,8 @@ virNetworkObjListPopulate(void *payload,
virObjectLock(obj);
- if (data->filter &&
- !data->filter(data->conn, obj->def))
+ if (data->aclfilter &&
+ !data->aclfilter(data->conn, obj->def))
goto cleanup;
if (!virNetworkMatch(obj, data->flags))
@@ -1419,11 +1419,13 @@ int
virNetworkObjListExport(virConnectPtr conn,
virNetworkObjListPtr netobjs,
virNetworkPtr **nets,
- virNetworkObjListFilter filter,
+ virNetworkObjListACLFilter aclfilter,
unsigned int flags)
{
int ret = -1;
- struct virNetworkObjListData data = { conn, NULL, filter, flags, 0, false};
+ struct virNetworkObjListData data = {
+ .conn = conn, .nets = NULL, .aclfilter = aclfilter, .flags = flags,
+ .nnets = 0, .error = false };
virObjectLock(netobjs);
if (nets && VIR_ALLOC_N(data.nets, virHashSize(netobjs->objs) + 1) < 0)
@@ -1489,7 +1491,8 @@ virNetworkObjListForEach(virNetworkObjListPtr nets,
virNetworkObjListIterator callback,
void *opaque)
{
- struct virNetworkObjListForEachHelperData data = {callback, opaque, 0};
+ struct virNetworkObjListForEachHelperData data = {
+ .callback = callback, .opaque = opaque, .ret = 0};
virObjectLock(nets);
virHashForEach(nets->objs, virNetworkObjListForEachHelper, &data);
virObjectUnlock(nets);
@@ -1499,9 +1502,9 @@ virNetworkObjListForEach(virNetworkObjListPtr nets,
struct virNetworkObjListGetHelperData {
virConnectPtr conn;
- virNetworkObjListFilter filter;
+ virNetworkObjListACLFilter aclfilter;
char **names;
- int nnames;
+ int maxnames;
bool active;
int got;
bool error;
@@ -1518,14 +1521,14 @@ virNetworkObjListGetHelper(void *payload,
if (data->error)
return 0;
- if (data->nnames >= 0 &&
- data->got == data->nnames)
+ if (data->maxnames >= 0 &&
+ data->got == data->maxnames)
return 0;
virObjectLock(obj);
- if (data->filter &&
- !data->filter(data->conn, obj->def))
+ if (data->aclfilter &&
+ !data->aclfilter(data->conn, obj->def))
goto cleanup;
if ((data->active && virNetworkObjIsActive(obj)) ||
@@ -1548,14 +1551,15 @@ int
virNetworkObjListGetNames(virNetworkObjListPtr nets,
bool active,
char **names,
- int nnames,
- virNetworkObjListFilter filter,
+ int maxnames,
+ virNetworkObjListACLFilter aclfilter,
virConnectPtr conn)
{
int ret = -1;
struct virNetworkObjListGetHelperData data = {
- conn, filter, names, nnames, active, 0, false};
+ .conn = conn, .aclfilter = aclfilter, .names = names,
+ .maxnames = maxnames, .active = active, .got = 0, .error = false};
virObjectLock(nets);
virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
@@ -1577,11 +1581,12 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
int
virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
bool active,
- virNetworkObjListFilter filter,
+ virNetworkObjListACLFilter aclfilter,
virConnectPtr conn)
{
struct virNetworkObjListGetHelperData data = {
- conn, filter, NULL, -1, active, 0, false};
+ .conn = conn, .aclfilter = aclfilter, .names = NULL,
+ .maxnames = -1, .active = active, .got = 0, .error = false};
virObjectLock(nets);
virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index b81ffa8..72afbcd 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -155,8 +155,8 @@ virNetworkObjTaint(virNetworkObjPtr obj,
virNetworkTaintFlags taint);
typedef bool
-(*virNetworkObjListFilter)(virConnectPtr conn,
- virNetworkDefPtr def);
+(*virNetworkObjListACLFilter)(virConnectPtr conn,
+ virNetworkDefPtr def);
virNetworkObjPtr
virNetworkObjAssignDef(virNetworkObjListPtr nets,
@@ -221,7 +221,7 @@ int
virNetworkObjListExport(virConnectPtr conn,
virNetworkObjListPtr netobjs,
virNetworkPtr **nets,
- virNetworkObjListFilter filter,
+ virNetworkObjListACLFilter aclfilter,
unsigned int flags);
typedef int
@@ -237,14 +237,14 @@ int
virNetworkObjListGetNames(virNetworkObjListPtr nets,
bool active,
char **names,
- int nnames,
- virNetworkObjListFilter filter,
+ int maxnames,
+ virNetworkObjListACLFilter aclfilter,
virConnectPtr conn);
int
virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
bool active,
- virNetworkObjListFilter filter,
+ virNetworkObjListACLFilter aclfilter,
virConnectPtr conn);
void
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index e2e9d13..47709ef 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2945,7 +2945,7 @@ networkConnectNumOfNetworks(virConnectPtr conn)
static int
networkConnectListNetworks(virConnectPtr conn,
char **const names,
- int nnames)
+ int maxnames)
{
virNetworkDriverStatePtr driver = networkGetDriver();
int got = 0;
@@ -2954,7 +2954,7 @@ networkConnectListNetworks(virConnectPtr conn,
return -1;
got = virNetworkObjListGetNames(driver->networks,
- true, names, nnames,
+ true, names, maxnames,
virConnectListNetworksCheckACL,
conn);
@@ -2983,7 +2983,7 @@ networkConnectNumOfDefinedNetworks(virConnectPtr conn)
static int
networkConnectListDefinedNetworks(virConnectPtr conn,
char **const names,
- int nnames)
+ int maxnames)
{
virNetworkDriverStatePtr driver = networkGetDriver();
int got = 0;
@@ -2992,7 +2992,7 @@ networkConnectListDefinedNetworks(virConnectPtr conn,
return -1;
got = virNetworkObjListGetNames(driver->networks,
- false, names, nnames,
+ false, names, maxnames,
virConnectListDefinedNetworksCheckACL,
conn);
return got;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index dd78b52..4dd469a 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3296,13 +3296,13 @@ testConnectNumOfNetworks(virConnectPtr conn)
static int
testConnectListNetworks(virConnectPtr conn,
char **const names,
- int nnames)
+ int maxnames)
{
testDriverPtr privconn = conn->privateData;
int n;
n = virNetworkObjListGetNames(privconn->networks,
- true, names, nnames, NULL, conn);
+ true, names, maxnames, NULL, conn);
return n;
}
@@ -3322,13 +3322,13 @@ testConnectNumOfDefinedNetworks(virConnectPtr conn)
static int
testConnectListDefinedNetworks(virConnectPtr conn,
char **const names,
- int nnames)
+ int maxnames)
{
testDriverPtr privconn = conn->privateData;
int n;
n = virNetworkObjListGetNames(privconn->networks,
- false, names, nnames, NULL, conn);
+ false, names, maxnames, NULL, conn);
return n;
}
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, May 19, 2017 at 09:03:23AM -0400, John Ferlan wrote: > Use the structure names in the @data setup - makes it easier that going > back to find the struct. > > Use the @maxnames instead of @nnames since that's what it is. Please use camelCase -> @maxNames. > > Modify the @filter to be @aclfilter and change the typedef from > virNetworkObjListFilter to virNetworkObjListACLFilter. NACK to this change, even though it's used only to filter by ACLs, it can be used to filter by anything. This patch does three things in one, so it should be three separate patches. Since the last change is not correct split the remaining changes into two patches. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 07/24/2017 07:49 AM, Pavel Hrdina wrote: > On Fri, May 19, 2017 at 09:03:23AM -0400, John Ferlan wrote: >> Use the structure names in the @data setup - makes it easier that going >> back to find the struct. >> >> Use the @maxnames instead of @nnames since that's what it is. > > Please use camelCase -> @maxNames. > This I disagree with as maxnames is used *liberally* throughout many places in libvirt and in particular as arguments to functions. In particular, follow this back to : virDrvConnectListNetworks virDrvConnectListDefinedNetworks and virConnectListNetworks virConnectListDefinedNetworks which all use @maxnames. But I will separate and describe appropriately. >> >> Modify the @filter to be @aclfilter and change the typedef from >> virNetworkObjListFilter to virNetworkObjListACLFilter. > > NACK to this change, even though it's used only to filter by ACLs, it > can be used to filter by anything. Again, I disagree. I've been using @aclfilter in other drivers and taking this route makes things consistent. Besides, look at a few of the vir*ObjListExport* type functions where there's actually a second filter that would take an @obj and a @flags argument and could be defined "generically" as "@filter". Now if there was a "generic" ObjListExport routine, that @filter could be an element to a common structure too... I will though separate it out. Tks - John > > This patch does three things in one, so it should be three separate > patches. Since the last change is not correct split the remaining > changes into two patches. > > Pavel > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Jul 26, 2017 at 10:57:30AM -0400, John Ferlan wrote: > > > On 07/24/2017 07:49 AM, Pavel Hrdina wrote: > > On Fri, May 19, 2017 at 09:03:23AM -0400, John Ferlan wrote: > >> Use the structure names in the @data setup - makes it easier that going > >> back to find the struct. > >> > >> Use the @maxnames instead of @nnames since that's what it is. > > > > Please use camelCase -> @maxNames. > > > > This I disagree with as maxnames is used *liberally* throughout many > places in libvirt and in particular as arguments to functions. In > particular, follow this back to : > > > virDrvConnectListNetworks > virDrvConnectListDefinedNetworks > > and > > virConnectListNetworks > virConnectListDefinedNetworks > > which all use @maxnames. > > But I will separate and describe appropriately. On the other hand there is for example maxMemory which uses camelCase. I have to disagree with your disagreement :) but argument that something is used in a certain way is not a good argument because the old usage may be wrong. > > >> > >> Modify the @filter to be @aclfilter and change the typedef from > >> virNetworkObjListFilter to virNetworkObjListACLFilter. > > > > NACK to this change, even though it's used only to filter by ACLs, it > > can be used to filter by anything. > > Again, I disagree. I've been using @aclfilter in other drivers and > taking this route makes things consistent. Now that I've checked it, even virDomainObjListExport has the type with ACL in it. So I take my NACK back :). Anyway, for the name I would prefer to use aclFilter but since the ship has sailed I give up. It can be done as a followup cleanup. But please keep this in mind that the camleCase is preferred. Having no separation between words in the variable name reduces readability. Pavel > Besides, look at a few of the vir*ObjListExport* type functions where > there's actually a second filter that would take an @obj and a @flags > argument and could be defined "generically" as "@filter". Now if there > was a "generic" ObjListExport routine, that @filter could be an element > to a common structure too... > > I will though separate it out. > > Tks - > > John > > > > > This patch does three things in one, so it should be three separate > > patches. Since the last change is not correct split the remaining > > changes into two patches. > > > > Pavel > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.