Re: [libvirt] [PATCH v2 18/20] network: Rename @filter to @aclfilter

2017-08-16 Thread John Ferlan 


On 08/16/2017 04:09 AM, Pavel Hrdina wrote:
> On Wed, Aug 16, 2017 at 09:36:36AM +0200, Michal Privoznik wrote:
>> On 08/15/2017 10:56 PM, John Ferlan wrote:
>>>
>>>
>>> On 08/15/2017 11:32 AM, Michal Privoznik wrote:
 On 07/26/2017 05:05 PM, John Ferlan wrote:
> Rename the virNetworkObjListFilter to be virNetworkObjListACLFilter
> since that's more representative of what it is. Also modify the
> variable @filter to be @aclfilter. In the future adding the ability
> to describe a generic @filter routine for the Export functions
> could be a useful thing.

 Well technically this is a filter. It's only that we use ACL filter
 function for it. But the implementation is generic enough for the cb to
 be called filter IMO. Therefore I'm not a fan of this one.

 Michal

>>>
>>> Understood - I can drop it, but then it's different than what I've
>>> already done in nwfilter, secret, nodedevice, and storage.
>>
>> Darn, did those slip in? Frankly, I'm not a fan of this change.
>> Therefore I'll no longer object to this change, but probably not ACK it
>> either. I'd like others to chime in and express their opinion.
> 
> I agree with Michal and I've already pointed this out for the first
> series [1].  In my second response I gave up :) but I still think
> that it should be made as a generic filter.  Since now I'm not the only
> one against this change we should consider "fixing" the other places
> that already use aclfilter.
> 
> Pavel
> 
> [1] 
> 

 OK To me it just seemed better to admit it was an ACL filter
as opposed to some other kind of filter. As noted that was perhaps a
carryover from my initial efforts where aclfilter and matchfilter were
co-mingled.

I will remove this patch from the series and will send a followup to
undo the others once I push.

John

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

Re: [libvirt] [PATCH v2 18/20] network: Rename @filter to @aclfilter

2017-08-16 Thread Pavel Hrdina 
On Wed, Aug 16, 2017 at 09:36:36AM +0200, Michal Privoznik wrote:
> On 08/15/2017 10:56 PM, John Ferlan wrote:
> > 
> > 
> > On 08/15/2017 11:32 AM, Michal Privoznik wrote:
> >> On 07/26/2017 05:05 PM, John Ferlan wrote:
> >>> Rename the virNetworkObjListFilter to be virNetworkObjListACLFilter
> >>> since that's more representative of what it is. Also modify the
> >>> variable @filter to be @aclfilter. In the future adding the ability
> >>> to describe a generic @filter routine for the Export functions
> >>> could be a useful thing.
> >>
> >> Well technically this is a filter. It's only that we use ACL filter
> >> function for it. But the implementation is generic enough for the cb to
> >> be called filter IMO. Therefore I'm not a fan of this one.
> >>
> >> Michal
> >>
> > 
> > Understood - I can drop it, but then it's different than what I've
> > already done in nwfilter, secret, nodedevice, and storage.
> 
> Darn, did those slip in? Frankly, I'm not a fan of this change.
> Therefore I'll no longer object to this change, but probably not ACK it
> either. I'd like others to chime in and express their opinion.

I agree with Michal and I've already pointed this out for the first
series [1].  In my second response I gave up :) but I still think
that it should be made as a generic filter.  Since now I'm not the only
one against this change we should consider "fixing" the other places
that already use aclfilter.

Pavel

[1] 


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 18/20] network: Rename @filter to @aclfilter

2017-08-16 Thread Michal Privoznik 
On 08/15/2017 10:56 PM, John Ferlan wrote:
> 
> 
> On 08/15/2017 11:32 AM, Michal Privoznik wrote:
>> On 07/26/2017 05:05 PM, John Ferlan wrote:
>>> Rename the virNetworkObjListFilter to be virNetworkObjListACLFilter
>>> since that's more representative of what it is. Also modify the
>>> variable @filter to be @aclfilter. In the future adding the ability
>>> to describe a generic @filter routine for the Export functions
>>> could be a useful thing.
>>
>> Well technically this is a filter. It's only that we use ACL filter
>> function for it. But the implementation is generic enough for the cb to
>> be called filter IMO. Therefore I'm not a fan of this one.
>>
>> Michal
>>
> 
> Understood - I can drop it, but then it's different than what I've
> already done in nwfilter, secret, nodedevice, and storage.

Darn, did those slip in? Frankly, I'm not a fan of this change.
Therefore I'll no longer object to this change, but probably not ACK it
either. I'd like others to chime in and express their opinion.

Michal

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

Re: [libvirt] [PATCH v2 18/20] network: Rename @filter to @aclfilter

2017-08-15 Thread John Ferlan 


On 08/15/2017 11:32 AM, Michal Privoznik wrote:
> On 07/26/2017 05:05 PM, John Ferlan wrote:
>> Rename the virNetworkObjListFilter to be virNetworkObjListACLFilter
>> since that's more representative of what it is. Also modify the
>> variable @filter to be @aclfilter. In the future adding the ability
>> to describe a generic @filter routine for the Export functions
>> could be a useful thing.
> 
> Well technically this is a filter. It's only that we use ACL filter
> function for it. But the implementation is generic enough for the cb to
> be called filter IMO. Therefore I'm not a fan of this one.
> 
> Michal
> 

Understood - I can drop it, but then it's different than what I've
already done in nwfilter, secret, nodedevice, and storage.

"Originally" there was an ACL Filter and a "match" filter - the various
Export API's have a Match API which take 2 params (obj, flags) and
returns a bool. If there was a really common object, then that function
could be added as a filter callback function in a very generic export
function.

John

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

Re: [libvirt] [PATCH v2 18/20] network: Rename @filter to @aclfilter

2017-08-15 Thread Michal Privoznik 
On 07/26/2017 05:05 PM, John Ferlan wrote:
> Rename the virNetworkObjListFilter to be virNetworkObjListACLFilter
> since that's more representative of what it is. Also modify the
> variable @filter to be @aclfilter. In the future adding the ability
> to describe a generic @filter routine for the Export functions
> could be a useful thing.

Well technically this is a filter. It's only that we use ACL filter
function for it. But the implementation is generic enough for the cb to
be called filter IMO. Therefore I'm not a fan of this one.

Michal

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

[libvirt] [PATCH v2 18/20] network: Rename @filter to @aclfilter

2017-07-26 Thread John Ferlan 
Rename the virNetworkObjListFilter to be virNetworkObjListACLFilter
since that's more representative of what it is. Also modify the
variable @filter to be @aclfilter. In the future adding the ability
to describe a generic @filter routine for the Export functions
could be a useful thing.

Signed-off-by: John Ferlan 
---
 src/conf/virnetworkobj.c | 24 
 src/conf/virnetworkobj.h | 10 +-
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index d288dd0..2be48dd 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1307,7 +1307,7 @@ virNetworkMatch(virNetworkObjPtr obj,
 struct virNetworkObjListData {
 virConnectPtr conn;
 virNetworkPtr *nets;
-virNetworkObjListFilter filter;
+virNetworkObjListACLFilter aclfilter;
 unsigned int flags;
 int nnets;
 bool error;
@@ -1327,8 +1327,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))
@@ -1356,11 +1356,11 @@ 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, NULL, aclfilter, flags, 0, 
false};
 
 virObjectLock(netobjs);
 if (nets && VIR_ALLOC_N(data.nets, virHashSize(netobjs->objs) + 1) < 0)
@@ -1436,7 +1436,7 @@ virNetworkObjListForEach(virNetworkObjListPtr nets,
 
 struct virNetworkObjListGetHelperData {
 virConnectPtr conn;
-virNetworkObjListFilter filter;
+virNetworkObjListACLFilter aclfilter;
 char **names;
 int maxnames;
 bool active;
@@ -1461,8 +1461,8 @@ virNetworkObjListGetHelper(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 ((data->active && virNetworkObjIsActive(obj)) ||
@@ -1486,13 +1486,13 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
   bool active,
   char **names,
   int maxnames,
-  virNetworkObjListFilter filter,
+  virNetworkObjListACLFilter aclfilter,
   virConnectPtr conn)
 {
 int ret = -1;
 
 struct virNetworkObjListGetHelperData data = {
-conn, filter, names, maxnames, active, 0, false};
+conn, aclfilter, names, maxnames, active, 0, false};
 
 virObjectLock(nets);
 virHashForEach(nets->objs, virNetworkObjListGetHelper, );
@@ -1514,11 +1514,11 @@ 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, aclfilter, NULL, -1, active, 0, false};
 
 virObjectLock(nets);
 virHashForEach(nets->objs, virNetworkObjListGetHelper, );
diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
index f7ed387..d7199fd 100644
--- a/src/conf/virnetworkobj.h
+++ b/src/conf/virnetworkobj.h
@@ -153,8 +153,8 @@ virNetworkObjTaint(virNetworkObjPtr obj,
virNetworkTaintFlags taint);
 
 typedef bool
-(*virNetworkObjListFilter)(virConnectPtr conn,
-   virNetworkDefPtr def);
+(*virNetworkObjListACLFilter)(virConnectPtr conn,
+  virNetworkDefPtr def);
 
 virNetworkObjPtr
 virNetworkObjAssignDef(virNetworkObjListPtr nets,
@@ -219,7 +219,7 @@ int
 virNetworkObjListExport(virConnectPtr conn,
 virNetworkObjListPtr netobjs,
 virNetworkPtr **nets,
-virNetworkObjListFilter filter,
+virNetworkObjListACLFilter aclfilter,
 unsigned int flags);
 
 typedef int
@@ -236,13 +236,13 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
   bool active,
   char **names,
   int maxnames,
-  virNetworkObjListFilter filter,
+  virNetworkObjListACLFilter aclfilter,