Re: [libvirt] [PATCH 07/19] storage: Use consistent variable names in virstorageobj

2017-07-14 Thread Pavel Hrdina 
On Thu, Jul 13, 2017 at 05:49:35PM -0400, John Ferlan wrote:
> 
> 
> On 07/12/2017 04:17 AM, Pavel Hrdina wrote:
> > On Tue, May 09, 2017 at 11:30:14AM -0400, John Ferlan wrote:
> >> A virStoragePoolObjPtr will be an 'obj'.
> >>
> >> A virStoragePoolPtr will be a 'pool'.
> > 
> > There is no such change in this commit.
> > 
> 
> Simple enough to remove.
> 
> >>
> >> NB: Also modify the @matchpool to @matchobj.
> >> Signed-off-by: John Ferlan 
> >> ---
> >>  src/conf/virstorageobj.c | 342 
> >> +++
> >>  src/conf/virstorageobj.h |  20 +--
> >>  2 files changed, 180 insertions(+), 182 deletions(-)
> >>
> >> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
> >> index dd41701..74a9c67 100644
> >> --- a/src/conf/virstorageobj.c
> >> +++ b/src/conf/virstorageobj.c
> >> @@ -70,15 +70,15 @@ virStoragePoolObjListFree(virStoragePoolObjListPtr 
> >> pools)
> >>  
> >>  void
> >>  virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
> >> -virStoragePoolObjPtr pool)
> >> +virStoragePoolObjPtr obj)
> > 
> > This is why I'm not a fan of this change, to make the code consistent
> > we should also rename "pools" to "objs" which I don't like so probably
> > "objList" or something else.
> > 
> > Pavel
> > 
> 
> I understand the objections and I agree @pools could be something
> else... Similarly @devs, @nwfilters, @secrets, @interfaces, etc.
> 
> The difference between @pools and @pool/@obj is the usage of @pool/@obj
> wasn't consistent.  At least there's only one @pools - OK, well two one
> in _virStorageDriverState and one in _testDriver.
> 
> I guess that's the line I didn't cross.

This can be addressed as a followup.

Reviewed-by: Pavel Hrdina 


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

Re: [libvirt] [PATCH 07/19] storage: Use consistent variable names in virstorageobj

2017-07-13 Thread John Ferlan 


On 07/12/2017 04:17 AM, Pavel Hrdina wrote:
> On Tue, May 09, 2017 at 11:30:14AM -0400, John Ferlan wrote:
>> A virStoragePoolObjPtr will be an 'obj'.
>>
>> A virStoragePoolPtr will be a 'pool'.
> 
> There is no such change in this commit.
> 

Simple enough to remove.

>>
>> NB: Also modify the @matchpool to @matchobj.
>> Signed-off-by: John Ferlan 
>> ---
>>  src/conf/virstorageobj.c | 342 
>> +++
>>  src/conf/virstorageobj.h |  20 +--
>>  2 files changed, 180 insertions(+), 182 deletions(-)
>>
>> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
>> index dd41701..74a9c67 100644
>> --- a/src/conf/virstorageobj.c
>> +++ b/src/conf/virstorageobj.c
>> @@ -70,15 +70,15 @@ virStoragePoolObjListFree(virStoragePoolObjListPtr pools)
>>  
>>  void
>>  virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
>> -virStoragePoolObjPtr pool)
>> +virStoragePoolObjPtr obj)
> 
> This is why I'm not a fan of this change, to make the code consistent
> we should also rename "pools" to "objs" which I don't like so probably
> "objList" or something else.
> 
> Pavel
> 

I understand the objections and I agree @pools could be something
else... Similarly @devs, @nwfilters, @secrets, @interfaces, etc.

The difference between @pools and @pool/@obj is the usage of @pool/@obj
wasn't consistent.  At least there's only one @pools - OK, well two one
in _virStorageDriverState and one in _testDriver.

I guess that's the line I didn't cross.

John

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

Re: [libvirt] [PATCH 07/19] storage: Use consistent variable names in virstorageobj

2017-07-12 Thread Pavel Hrdina 
On Tue, May 09, 2017 at 11:30:14AM -0400, John Ferlan wrote:
> A virStoragePoolObjPtr will be an 'obj'.
> 
> A virStoragePoolPtr will be a 'pool'.

There is no such change in this commit.

> 
> NB: Also modify the @matchpool to @matchobj.
> Signed-off-by: John Ferlan 
> ---
>  src/conf/virstorageobj.c | 342 
> +++
>  src/conf/virstorageobj.h |  20 +--
>  2 files changed, 180 insertions(+), 182 deletions(-)
> 
> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
> index dd41701..74a9c67 100644
> --- a/src/conf/virstorageobj.c
> +++ b/src/conf/virstorageobj.c
> @@ -70,15 +70,15 @@ virStoragePoolObjListFree(virStoragePoolObjListPtr pools)
>  
>  void
>  virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
> -virStoragePoolObjPtr pool)
> +virStoragePoolObjPtr obj)

This is why I'm not a fan of this change, to make the code consistent
we should also rename "pools" to "objs" which I don't like so probably
"objList" or something else.

Pavel


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

[libvirt] [PATCH 07/19] storage: Use consistent variable names in virstorageobj

2017-05-09 Thread John Ferlan 
A virStoragePoolObjPtr will be an 'obj'.

A virStoragePoolPtr will be a 'pool'.

NB: Also modify the @matchpool to @matchobj.
Signed-off-by: John Ferlan 
---
 src/conf/virstorageobj.c | 342 +++
 src/conf/virstorageobj.h |  20 +--
 2 files changed, 180 insertions(+), 182 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index dd41701..74a9c67 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -70,15 +70,15 @@ virStoragePoolObjListFree(virStoragePoolObjListPtr pools)
 
 void
 virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
-virStoragePoolObjPtr pool)
+virStoragePoolObjPtr obj)
 {
 size_t i;
 
-virStoragePoolObjUnlock(pool);
+virStoragePoolObjUnlock(obj);
 
 for (i = 0; i < pools->count; i++) {
 virStoragePoolObjLock(pools->objs[i]);
-if (pools->objs[i] == pool) {
+if (pools->objs[i] == obj) {
 virStoragePoolObjUnlock(pools->objs[i]);
 virStoragePoolObjFree(pools->objs[i]);
 
@@ -125,15 +125,15 @@ virStoragePoolObjFindByName(virStoragePoolObjListPtr 
pools,
 
 
 static virStoragePoolObjPtr
-virStoragePoolSourceFindDuplicateDevices(virStoragePoolObjPtr pool,
+virStoragePoolSourceFindDuplicateDevices(virStoragePoolObjPtr obj,
  virStoragePoolDefPtr def)
 {
 size_t i, j;
 
-for (i = 0; i < pool->def->source.ndevice; i++) {
+for (i = 0; i < obj->def->source.ndevice; i++) {
 for (j = 0; j < def->source.ndevice; j++) {
-if (STREQ(pool->def->source.devices[i].path, 
def->source.devices[j].path))
-return pool;
+if (STREQ(obj->def->source.devices[i].path, 
def->source.devices[j].path))
+return obj;
 }
 }
 
@@ -142,54 +142,54 @@ 
virStoragePoolSourceFindDuplicateDevices(virStoragePoolObjPtr pool,
 
 
 void
-virStoragePoolObjClearVols(virStoragePoolObjPtr pool)
+virStoragePoolObjClearVols(virStoragePoolObjPtr obj)
 {
 size_t i;
-for (i = 0; i < pool->volumes.count; i++)
-virStorageVolDefFree(pool->volumes.objs[i]);
+for (i = 0; i < obj->volumes.count; i++)
+virStorageVolDefFree(obj->volumes.objs[i]);
 
-VIR_FREE(pool->volumes.objs);
-pool->volumes.count = 0;
+VIR_FREE(obj->volumes.objs);
+obj->volumes.count = 0;
 }
 
 
 virStorageVolDefPtr
-virStorageVolDefFindByKey(virStoragePoolObjPtr pool,
+virStorageVolDefFindByKey(virStoragePoolObjPtr obj,
   const char *key)
 {
 size_t i;
 
-for (i = 0; i < pool->volumes.count; i++)
-if (STREQ(pool->volumes.objs[i]->key, key))
-return pool->volumes.objs[i];
+for (i = 0; i < obj->volumes.count; i++)
+if (STREQ(obj->volumes.objs[i]->key, key))
+return obj->volumes.objs[i];
 
 return NULL;
 }
 
 
 virStorageVolDefPtr
-virStorageVolDefFindByPath(virStoragePoolObjPtr pool,
+virStorageVolDefFindByPath(virStoragePoolObjPtr obj,
const char *path)
 {
 size_t i;
 
-for (i = 0; i < pool->volumes.count; i++)
-if (STREQ(pool->volumes.objs[i]->target.path, path))
-return pool->volumes.objs[i];
+for (i = 0; i < obj->volumes.count; i++)
+if (STREQ(obj->volumes.objs[i]->target.path, path))
+return obj->volumes.objs[i];
 
 return NULL;
 }
 
 
 virStorageVolDefPtr
-virStorageVolDefFindByName(virStoragePoolObjPtr pool,
+virStorageVolDefFindByName(virStoragePoolObjPtr obj,
const char *name)
 {
 size_t i;
 
-for (i = 0; i < pool->volumes.count; i++)
-if (STREQ(pool->volumes.objs[i]->name, name))
-return pool->volumes.objs[i];
+for (i = 0; i < obj->volumes.count; i++)
+if (STREQ(obj->volumes.objs[i]->name, name))
+return obj->volumes.objs[i];
 
 return NULL;
 }
@@ -296,39 +296,39 @@ virStoragePoolObjPtr
 virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def)
 {
-virStoragePoolObjPtr pool;
+virStoragePoolObjPtr obj;
 
-if ((pool = virStoragePoolObjFindByName(pools, def->name))) {
-if (!virStoragePoolObjIsActive(pool)) {
-virStoragePoolDefFree(pool->def);
-pool->def = def;
+if ((obj = virStoragePoolObjFindByName(pools, def->name))) {
+if (!virStoragePoolObjIsActive(obj)) {
+virStoragePoolDefFree(obj->def);
+obj->def = def;
 } else {
-virStoragePoolDefFree(pool->newDef);
-pool->newDef = def;
+virStoragePoolDefFree(obj->newDef);
+obj->newDef = def;
 }
-return pool;
+return obj;
 }
 
-if (VIR_ALLOC(pool) < 0)
+if (VIR_ALLOC(obj) < 0)
 return NULL;
 
-if (virMutexInit(&pool->lock) < 0) {
+if (virMutexInit(&obj->lock) < 0) {
 virReportErro