Mostly code motion to move storageConnectList[Defined]StoragePools
and similar test driver code into virstorageobj.c and rename to
virStoragePoolObjGetNames.

Also includes a couple of variable name adjustments to keep code consistent
with other drivers.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/conf/virstorageobj.c     | 37 ++++++++++++++++++++++++++++
 src/conf/virstorageobj.h     |  8 ++++++
 src/libvirt_private.syms     |  1 +
 src/storage/storage_driver.c | 58 +++++++++++---------------------------------
 src/test/test_driver.c       | 48 +++++++++---------------------------
 5 files changed, 72 insertions(+), 80 deletions(-)

diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index f0201aa..a9367a2 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -580,6 +580,43 @@ virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools,
 }
 
 
+int
+virStoragePoolObjGetNames(virStoragePoolObjListPtr pools,
+                          virConnectPtr conn,
+                          bool wantActive,
+                          virStoragePoolObjListACLFilter aclfilter,
+                          char **const names,
+                          int maxnames)
+{
+    int nnames = 0;
+    size_t i;
+
+    for (i = 0; i < pools->count && nnames < maxnames; i++) {
+        virStoragePoolObjPtr obj = pools->objs[i];
+        virStoragePoolObjLock(obj);
+        if (!aclfilter || aclfilter(conn, obj->def)) {
+            if ((wantActive && virStoragePoolObjIsActive(obj)) ||
+                (!wantActive && !virStoragePoolObjIsActive(obj))) {
+                if (VIR_STRDUP(names[nnames++], obj->def->name) < 0) {
+                    virStoragePoolObjUnlock(obj);
+                    goto failure;
+                }
+            }
+        }
+        virStoragePoolObjUnlock(obj);
+    }
+
+    return nnames;
+
+ failure:
+    while (--nnames >= 0)
+        VIR_FREE(names[nnames]);
+    memset(names, 0, maxnames * sizeof(*names));
+
+    return -1;
+}
+
+
 /*
  * virStoragePoolObjIsDuplicate:
  * @doms : virStoragePoolObjListPtr to search
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 5eeda1e..b651865 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -155,6 +155,14 @@ virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools,
                                    bool wantActive,
                                    virStoragePoolObjListACLFilter aclfilter);
 
+int
+virStoragePoolObjGetNames(virStoragePoolObjListPtr pools,
+                          virConnectPtr conn,
+                          bool wantActive,
+                          virStoragePoolObjListACLFilter aclfilter,
+                          char **const names,
+                          int maxnames);
+
 void
 virStoragePoolObjFree(virStoragePoolObjPtr pool);
 
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7bc3bc4..233110a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -996,6 +996,7 @@ virStoragePoolObjClearVols;
 virStoragePoolObjDeleteDef;
 virStoragePoolObjFindByName;
 virStoragePoolObjFindByUUID;
+virStoragePoolObjGetNames;
 virStoragePoolObjIsDuplicate;
 virStoragePoolObjListExport;
 virStoragePoolObjListFree;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 065380b..9493da8 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -490,40 +490,25 @@ storageConnectNumOfStoragePools(virConnectPtr conn)
     return nactive;
 }
 
+
 static int
 storageConnectListStoragePools(virConnectPtr conn,
                                char **const names,
-                               int nnames)
+                               int maxnames)
 {
     int got = 0;
-    size_t i;
+
+    memset(names, 0, maxnames * sizeof(*names));
 
     if (virConnectListStoragePoolsEnsureACL(conn) < 0)
         return -1;
 
     storageDriverLock();
-    for (i = 0; i < driver->pools.count && got < nnames; i++) {
-        virStoragePoolObjPtr obj = driver->pools.objs[i];
-        virStoragePoolObjLock(obj);
-        if (virConnectListStoragePoolsCheckACL(conn, obj->def) &&
-            virStoragePoolObjIsActive(obj)) {
-            if (VIR_STRDUP(names[got], obj->def->name) < 0) {
-                virStoragePoolObjUnlock(obj);
-                goto cleanup;
-            }
-            got++;
-        }
-        virStoragePoolObjUnlock(obj);
-    }
+    got = virStoragePoolObjGetNames(&driver->pools, conn, true,
+                                    virConnectListStoragePoolsCheckACL,
+                                    names, maxnames);
     storageDriverUnlock();
     return got;
-
- cleanup:
-    storageDriverUnlock();
-    for (i = 0; i < got; i++)
-        VIR_FREE(names[i]);
-    memset(names, 0, nnames * sizeof(*names));
-    return -1;
 }
 
 static int
@@ -542,40 +527,25 @@ storageConnectNumOfDefinedStoragePools(virConnectPtr conn)
     return nactive;
 }
 
+
 static int
 storageConnectListDefinedStoragePools(virConnectPtr conn,
                                       char **const names,
-                                      int nnames)
+                                      int maxnames)
 {
     int got = 0;
-    size_t i;
+
+    memset(names, 0, maxnames * sizeof(*names));
 
     if (virConnectListDefinedStoragePoolsEnsureACL(conn) < 0)
         return -1;
 
     storageDriverLock();
-    for (i = 0; i < driver->pools.count && got < nnames; i++) {
-        virStoragePoolObjPtr obj = driver->pools.objs[i];
-        virStoragePoolObjLock(obj);
-        if (virConnectListDefinedStoragePoolsCheckACL(conn, obj->def) &&
-            !virStoragePoolObjIsActive(obj)) {
-            if (VIR_STRDUP(names[got], obj->def->name) < 0) {
-                virStoragePoolObjUnlock(obj);
-                goto cleanup;
-            }
-            got++;
-        }
-        virStoragePoolObjUnlock(obj);
-    }
+    got = virStoragePoolObjGetNames(&driver->pools, conn, false,
+                                    virConnectListDefinedStoragePoolsCheckACL,
+                                    names, maxnames);
     storageDriverUnlock();
     return got;
-
- cleanup:
-    storageDriverUnlock();
-    for (i = 0; i < got; i++)
-        VIR_FREE(names[i]);
-    memset(names, 0, nnames * sizeof(*names));
-    return -1;
 }
 
 /* This method is required to be re-entrant / thread safe, so
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 4fb9915..1a6601e 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4130,35 +4130,23 @@ testConnectNumOfStoragePools(virConnectPtr conn)
     return numActive;
 }
 
+
 static int
 testConnectListStoragePools(virConnectPtr conn,
                             char **const names,
-                            int nnames)
+                            int maxnames)
 {
     testDriverPtr privconn = conn->privateData;
     int n = 0;
-    size_t i;
+
+    memset(names, 0, maxnames * sizeof(*names));
 
     testDriverLock(privconn);
-    memset(names, 0, sizeof(*names)*nnames);
-    for (i = 0; i < privconn->pools.count && n < nnames; i++) {
-        virStoragePoolObjLock(privconn->pools.objs[i]);
-        if (virStoragePoolObjIsActive(privconn->pools.objs[i]) &&
-            VIR_STRDUP(names[n++], privconn->pools.objs[i]->def->name) < 0) {
-            virStoragePoolObjUnlock(privconn->pools.objs[i]);
-            goto error;
-        }
-        virStoragePoolObjUnlock(privconn->pools.objs[i]);
-    }
+    n = virStoragePoolObjGetNames(&privconn->pools, conn, true, NULL,
+                                  names, maxnames);
     testDriverUnlock(privconn);
 
     return n;
-
- error:
-    for (n = 0; n < nnames; n++)
-        VIR_FREE(names[n]);
-    testDriverUnlock(privconn);
-    return -1;
 }
 
 
@@ -4176,35 +4164,23 @@ testConnectNumOfDefinedStoragePools(virConnectPtr conn)
     return numInactive;
 }
 
+
 static int
 testConnectListDefinedStoragePools(virConnectPtr conn,
                                    char **const names,
-                                   int nnames)
+                                   int maxnames)
 {
     testDriverPtr privconn = conn->privateData;
     int n = 0;
-    size_t i;
+
+    memset(names, 0, maxnames * sizeof(*names));
 
     testDriverLock(privconn);
-    memset(names, 0, sizeof(*names)*nnames);
-    for (i = 0; i < privconn->pools.count && n < nnames; i++) {
-        virStoragePoolObjLock(privconn->pools.objs[i]);
-        if (!virStoragePoolObjIsActive(privconn->pools.objs[i]) &&
-            VIR_STRDUP(names[n++], privconn->pools.objs[i]->def->name) < 0) {
-            virStoragePoolObjUnlock(privconn->pools.objs[i]);
-            goto error;
-        }
-        virStoragePoolObjUnlock(privconn->pools.objs[i]);
-    }
+    n = virStoragePoolObjGetNames(&privconn->pools, conn, false, NULL,
+                                  names, maxnames);
     testDriverUnlock(privconn);
 
     return n;
-
- error:
-    for (n = 0; n < nnames; n++)
-        VIR_FREE(names[n]);
-    testDriverUnlock(privconn);
-    return -1;
 }
 
 static int
-- 
2.9.3

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

On 04/06/2017 01:48 PM, John Ferlan wrote:
> Mostly code motion to move storageConnectList[Defined]StoragePools
> and similar test driver code into virstorageobj.c and rename to
> virStoragePoolObjGetNames.
>
> Also includes a couple of variable name adjustments to keep code consistent
> with other drivers.
>
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/conf/virstorageobj.c     | 37 ++++++++++++++++++++++++++++
>  src/conf/virstorageobj.h     |  8 ++++++
>  src/libvirt_private.syms     |  1 +
>  src/storage/storage_driver.c | 58 +++++++++++---------------------------------
>  src/test/test_driver.c       | 48 +++++++++---------------------------
>  5 files changed, 72 insertions(+), 80 deletions(-)
>
> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
> index f0201aa..a9367a2 100644
> --- a/src/conf/virstorageobj.c
> +++ b/src/conf/virstorageobj.c
> @@ -580,6 +580,43 @@ virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools,
>  }
>
>
> +int
> +virStoragePoolObjGetNames(virStoragePoolObjListPtr pools,
> +                          virConnectPtr conn,
> +                          bool wantActive,
> +                          virStoragePoolObjListACLFilter aclfilter,
> +                          char **const names,

const?

> +                          int maxnames)
> +{
> +    int nnames = 0;
> +    size_t i;
> +
> +    for (i = 0; i < pools->count && nnames < maxnames; i++) {
> +        virStoragePoolObjPtr obj = pools->objs[i];
> +        virStoragePoolObjLock(obj);
> +        if (!aclfilter || aclfilter(conn, obj->def)) {
> +            if ((wantActive && virStoragePoolObjIsActive(obj)) ||
> +                (!wantActive && !virStoragePoolObjIsActive(obj))) {

;-)

> +                if (VIR_STRDUP(names[nnames++], obj->def->name) < 0) {

Again, put nnames++ after the if() block.

> +                    virStoragePoolObjUnlock(obj);
> +                    goto failure;
> +                }
> +            }
> +        }
> +        virStoragePoolObjUnlock(obj);
> +    }
> +
> +    return nnames;
> +
> + failure:
> +    while (--nnames >= 0)
> +        VIR_FREE(names[nnames]);
> +    memset(names, 0, maxnames * sizeof(*names));

;-)

> +
> +    return -1;
> +}
> +
> +
>  /*
>   * virStoragePoolObjIsDuplicate:
>   * @doms : virStoragePoolObjListPtr to search
> diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
> index 5eeda1e..b651865 100644
> --- a/src/conf/virstorageobj.h
> +++ b/src/conf/virstorageobj.h
> @@ -155,6 +155,14 @@ virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools,
>                                     bool wantActive,
>                                     virStoragePoolObjListACLFilter aclfilter);
>
> +int
> +virStoragePoolObjGetNames(virStoragePoolObjListPtr pools,
> +                          virConnectPtr conn,
> +                          bool wantActive,
> +                          virStoragePoolObjListACLFilter aclfilter,
> +                          char **const names,
> +                          int maxnames);
> +
>  void
>  virStoragePoolObjFree(virStoragePoolObjPtr pool);
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 7bc3bc4..233110a 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -996,6 +996,7 @@ virStoragePoolObjClearVols;
>  virStoragePoolObjDeleteDef;
>  virStoragePoolObjFindByName;
>  virStoragePoolObjFindByUUID;
> +virStoragePoolObjGetNames;
>  virStoragePoolObjIsDuplicate;
>  virStoragePoolObjListExport;
>  virStoragePoolObjListFree;
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 065380b..9493da8 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -490,40 +490,25 @@ storageConnectNumOfStoragePools(virConnectPtr conn)
>      return nactive;
>  }
>
> +
>  static int
>  storageConnectListStoragePools(virConnectPtr conn,
>                                 char **const names,
> -                               int nnames)
> +                               int maxnames)
>  {
>      int got = 0;
> -    size_t i;
> +
> +    memset(names, 0, maxnames * sizeof(*names));

;-)

>
>      if (virConnectListStoragePoolsEnsureACL(conn) < 0)
>          return -1;
>
>      storageDriverLock();
> -    for (i = 0; i < driver->pools.count && got < nnames; i++) {
> -        virStoragePoolObjPtr obj = driver->pools.objs[i];
> -        virStoragePoolObjLock(obj);
> -        if (virConnectListStoragePoolsCheckACL(conn, obj->def) &&
> -            virStoragePoolObjIsActive(obj)) {
> -            if (VIR_STRDUP(names[got], obj->def->name) < 0) {
> -                virStoragePoolObjUnlock(obj);
> -                goto cleanup;
> -            }
> -            got++;
> -        }
> -        virStoragePoolObjUnlock(obj);
> -    }
> +    got = virStoragePoolObjGetNames(&driver->pools, conn, true,
> +                                    virConnectListStoragePoolsCheckACL,
> +                                    names, maxnames);
>      storageDriverUnlock();
>      return got;
> -
> - cleanup:
> -    storageDriverUnlock();
> -    for (i = 0; i < got; i++)
> -        VIR_FREE(names[i]);
> -    memset(names, 0, nnames * sizeof(*names));
> -    return -1;
>  }
>
>  static int
> @@ -542,40 +527,25 @@ storageConnectNumOfDefinedStoragePools(virConnectPtr conn)
>      return nactive;
>  }
>
> +
>  static int
>  storageConnectListDefinedStoragePools(virConnectPtr conn,
>                                        char **const names,
> -                                      int nnames)
> +                                      int maxnames)
>  {
>      int got = 0;
> -    size_t i;
> +
> +    memset(names, 0, maxnames * sizeof(*names));

;-)

>
>      if (virConnectListDefinedStoragePoolsEnsureACL(conn) < 0)
>          return -1;
>
>      storageDriverLock();
> -    for (i = 0; i < driver->pools.count && got < nnames; i++) {
> -        virStoragePoolObjPtr obj = driver->pools.objs[i];
> -        virStoragePoolObjLock(obj);
> -        if (virConnectListDefinedStoragePoolsCheckACL(conn, obj->def) &&
> -            !virStoragePoolObjIsActive(obj)) {
> -            if (VIR_STRDUP(names[got], obj->def->name) < 0) {
> -                virStoragePoolObjUnlock(obj);
> -                goto cleanup;
> -            }
> -            got++;
> -        }
> -        virStoragePoolObjUnlock(obj);
> -    }
> +    got = virStoragePoolObjGetNames(&driver->pools, conn, false,
> +                                    virConnectListDefinedStoragePoolsCheckACL,
> +                                    names, maxnames);
>      storageDriverUnlock();
>      return got;
> -
> - cleanup:
> -    storageDriverUnlock();
> -    for (i = 0; i < got; i++)
> -        VIR_FREE(names[i]);
> -    memset(names, 0, nnames * sizeof(*names));
> -    return -1;
>  }
>
>  /* This method is required to be re-entrant / thread safe, so
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 4fb9915..1a6601e 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -4130,35 +4130,23 @@ testConnectNumOfStoragePools(virConnectPtr conn)
>      return numActive;
>  }
>
> +
>  static int
>  testConnectListStoragePools(virConnectPtr conn,
>                              char **const names,
> -                            int nnames)
> +                            int maxnames)
>  {
>      testDriverPtr privconn = conn->privateData;
>      int n = 0;
> -    size_t i;
> +
> +    memset(names, 0, maxnames * sizeof(*names));

;-)

>
>      testDriverLock(privconn);
> -    memset(names, 0, sizeof(*names)*nnames);
> -    for (i = 0; i < privconn->pools.count && n < nnames; i++) {
> -        virStoragePoolObjLock(privconn->pools.objs[i]);
> -        if (virStoragePoolObjIsActive(privconn->pools.objs[i]) &&
> -            VIR_STRDUP(names[n++], privconn->pools.objs[i]->def->name) < 0) {
> -            virStoragePoolObjUnlock(privconn->pools.objs[i]);
> -            goto error;
> -        }
> -        virStoragePoolObjUnlock(privconn->pools.objs[i]);
> -    }
> +    n = virStoragePoolObjGetNames(&privconn->pools, conn, true, NULL,
> +                                  names, maxnames);
>      testDriverUnlock(privconn);
>
>      return n;
> -
> - error:
> -    for (n = 0; n < nnames; n++)
> -        VIR_FREE(names[n]);
> -    testDriverUnlock(privconn);
> -    return -1;
>  }
>
>
> @@ -4176,35 +4164,23 @@ testConnectNumOfDefinedStoragePools(virConnectPtr conn)
>      return numInactive;
>  }
>
> +
>  static int
>  testConnectListDefinedStoragePools(virConnectPtr conn,
>                                     char **const names,
> -                                   int nnames)
> +                                   int maxnames)
>  {
>      testDriverPtr privconn = conn->privateData;
>      int n = 0;
> -    size_t i;
> +
> +    memset(names, 0, maxnames * sizeof(*names));

;-)

>
>      testDriverLock(privconn);
> -    memset(names, 0, sizeof(*names)*nnames);
> -    for (i = 0; i < privconn->pools.count && n < nnames; i++) {
> -        virStoragePoolObjLock(privconn->pools.objs[i]);
> -        if (!virStoragePoolObjIsActive(privconn->pools.objs[i]) &&
> -            VIR_STRDUP(names[n++], privconn->pools.objs[i]->def->name) < 0) {
> -            virStoragePoolObjUnlock(privconn->pools.objs[i]);
> -            goto error;
> -        }
> -        virStoragePoolObjUnlock(privconn->pools.objs[i]);
> -    }
> +    n = virStoragePoolObjGetNames(&privconn->pools, conn, false, NULL,
> +                                  names, maxnames);
>      testDriverUnlock(privconn);
>
>      return n;
> -
> - error:
> -    for (n = 0; n < nnames; n++)
> -        VIR_FREE(names[n]);
> -    testDriverUnlock(privconn);
> -    return -1;
>  }
>
>  static int
>

Michal

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