6:48 a.m.
Merge code in storage_driver.c and test_driver into virstorageobj to
count the number of volumes/pools, to return a list of names, return
the collected list of objects for volumes/pools. For volumes, that's
moved code, while for pools that's just changing the export API to
take an address of 'devobjs' to follow other usages. I also added
code to converge the FindBy{UUID|Name}
FWIW: This is part of the common driver objects code I've been working
through. I figured I will post each driver separately rather than one
John Ferlan (7):
storage: Introduce virStoragePoolObjNumOfVolumes
storage: Introduce virStoragePoolObjVolumeGetNames
storage: Introduce virStoragePoolObjVolumeListExport
storage: Introduce virStoragePoolObjNumOfStoragePools
storage: Introduce virStoragePoolObjGetNames
storage: Pass driver arg by ref
storage: Create helpers to perform FindByUUID and FindByName
src/conf/virstorageobj.c | 165 +++++++++++++++++++++++-
src/conf/virstorageobj.h | 45 ++++++-
src/libvirt_private.syms | 5 +
src/storage/storage_driver.c | 297 +++++++++++++------------------------------
src/test/test_driver.c | 131 +++++--------------
5 files changed, 332 insertions(+), 311 deletions(-)
--
2.9.3
6:48 a.m.
New subject: [libvirt] [PATCH 1/7] storage: Introduce virStoragePoolObjNumOfVolumes
Unify the NumOfVolumes API into virstorageobj.c from storage_driver and
test_driver. The only real difference between the two is the test driver
doesn't call using the aclfilter API.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 19 +++++++++++++++++++
src/conf/virstorageobj.h | 11 +++++++++++
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 10 +++-------
src/test/test_driver.c | 3 ++-
5 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 34f2eb7..e57694c 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -195,6 +195,25 @@ virStorageVolDefFindByName(virStoragePoolObjPtr pool,
}
+int
+virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr volumes,
+ virConnectPtr conn,
+ virStoragePoolDefPtr pooldef,
+ virStoragePoolVolumeACLFilter aclfilter)
+{
+ int nvolumes = 0;
+ size_t i;
+
+ for (i = 0; i < volumes->count; i++) {
+ virStorageVolDefPtr def = volumes->objs[i];
+ if (aclfilter && aclfilter(conn, pooldef, def))
+ nvolumes++;
+ }
+
+ return nvolumes;
+}
+
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def)
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index ecba94e..3effe7a 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -108,6 +108,17 @@ virStorageVolDefFindByName(virStoragePoolObjPtr pool,
void
virStoragePoolObjClearVols(virStoragePoolObjPtr pool);
+typedef bool
+(*virStoragePoolVolumeACLFilter)(virConnectPtr conn,
+ virStoragePoolDefPtr pool,
+ virStorageVolDefPtr def);
+
+int
+virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr volumes,
+ virConnectPtr conn,
+ virStoragePoolDefPtr pooldef,
+ virStoragePoolVolumeACLFilter aclfilter);
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 92083e5..9580622 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1002,6 +1002,7 @@ virStoragePoolObjListFree;
virStoragePoolObjLoadAllConfigs;
virStoragePoolObjLoadAllState;
virStoragePoolObjLock;
+virStoragePoolObjNumOfVolumes;
virStoragePoolObjRemove;
virStoragePoolObjSaveDef;
virStoragePoolObjSourceFindDuplicate;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index fea7698..7d2f74d 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1390,7 +1390,6 @@ storagePoolNumOfVolumes(virStoragePoolPtr obj)
{
virStoragePoolObjPtr pool;
int ret = -1;
- size_t i;
if (!(pool = virStoragePoolObjFromStoragePool(obj)))
return -1;
@@ -1403,12 +1402,9 @@ storagePoolNumOfVolumes(virStoragePoolPtr obj)
_("storage pool '%s' is not active"),
pool->def->name);
goto cleanup;
}
- ret = 0;
- for (i = 0; i < pool->volumes.count; i++) {
- if (virStoragePoolNumOfVolumesCheckACL(obj->conn, pool->def,
- pool->volumes.objs[i]))
- ret++;
- }
+
+ ret = virStoragePoolObjNumOfVolumes(&pool->volumes, obj->conn,
pool->def,
+ virStoragePoolNumOfVolumesCheckACL);
cleanup:
virStoragePoolObjUnlock(pool);
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index cce4d2d..a4b5833 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4821,7 +4821,8 @@ testStoragePoolNumOfVolumes(virStoragePoolPtr pool)
goto cleanup;
}
- ret = privpool->volumes.count;
+ ret = virStoragePoolObjNumOfVolumes(&privpool->volumes, pool->conn,
+ privpool->def, NULL);
cleanup:
if (privpool)
--
2.9.3
6:48 a.m.
New subject: [libvirt] [PATCH 2/7] storage: Introduce virStoragePoolObjVolumeGetNames
Mostly code motion to move storagePoolListVolumes code into virstorageobj.c
and rename to virStoragePoolObjVolumeGetNames.
Also includes a couple of variable name adjustments to keep code consistent
with other drivers.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 30 ++++++++++++++++++++++++++++++
src/conf/virstorageobj.h | 8 ++++++++
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 24 ++++++------------------
src/test/test_driver.c | 21 ++++++---------------
5 files changed, 51 insertions(+), 33 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index e57694c..5933618 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -214,6 +214,36 @@ virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr volumes,
}
+int
+virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr volumes,
+ virConnectPtr conn,
+ virStoragePoolDefPtr pooldef,
+ virStoragePoolVolumeACLFilter aclfilter,
+ char **const names,
+ int maxnames)
+{
+ int nnames = 0;
+ size_t i;
+
+ for (i = 0; i < volumes->count && nnames < maxnames; i++) {
+ virStorageVolDefPtr def = volumes->objs[i];
+ if (aclfilter && !aclfilter(conn, pooldef, def))
+ continue;
+ if (VIR_STRDUP(names[nnames++], def->name) < 0)
+ goto failure;
+ }
+
+ return nnames;
+
+ failure:
+ while (--nnames >= 0)
+ VIR_FREE(names[nnames]);
+ memset(names, 0, maxnames * sizeof(*names));
+
+ return -1;
+}
+
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def)
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 3effe7a..bf88094 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -119,6 +119,14 @@ virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr volumes,
virStoragePoolDefPtr pooldef,
virStoragePoolVolumeACLFilter aclfilter);
+int
+virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr volumes,
+ virConnectPtr conn,
+ virStoragePoolDefPtr pooldef,
+ virStoragePoolVolumeACLFilter aclfilter,
+ char **const names,
+ int maxnames);
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9580622..a635cac 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1007,6 +1007,7 @@ virStoragePoolObjRemove;
virStoragePoolObjSaveDef;
virStoragePoolObjSourceFindDuplicate;
virStoragePoolObjUnlock;
+virStoragePoolObjVolumeGetNames;
# cpu/cpu.h
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 7d2f74d..ce77fe1 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1411,14 +1411,14 @@ storagePoolNumOfVolumes(virStoragePoolPtr obj)
return ret;
}
+
static int
storagePoolListVolumes(virStoragePoolPtr obj,
char **const names,
int maxnames)
{
virStoragePoolObjPtr pool;
- size_t i;
- int n = 0;
+ int n = -1;
memset(names, 0, maxnames * sizeof(*names));
@@ -1434,24 +1434,12 @@ storagePoolListVolumes(virStoragePoolPtr obj,
goto cleanup;
}
- for (i = 0; i < pool->volumes.count && n < maxnames; i++) {
- if (!virStoragePoolListVolumesCheckACL(obj->conn, pool->def,
- pool->volumes.objs[i]))
- continue;
- if (VIR_STRDUP(names[n++], pool->volumes.objs[i]->name) < 0)
- goto cleanup;
- }
-
- virStoragePoolObjUnlock(pool);
- return n;
-
+ n = virStoragePoolObjVolumeGetNames(&pool->volumes, obj->conn,
pool->def,
+ virStoragePoolListVolumesCheckACL,
+ names, maxnames);
cleanup:
virStoragePoolObjUnlock(pool);
- for (n = 0; n < maxnames; n++)
- VIR_FREE(names[n]);
-
- memset(names, 0, maxnames * sizeof(*names));
- return -1;
+ return n;
}
static int
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index a4b5833..b2840fa 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4830,6 +4830,7 @@ testStoragePoolNumOfVolumes(virStoragePoolPtr pool)
return ret;
}
+
static int
testStoragePoolListVolumes(virStoragePoolPtr pool,
char **const names,
@@ -4837,8 +4838,7 @@ testStoragePoolListVolumes(virStoragePoolPtr pool,
{
testDriverPtr privconn = pool->conn->privateData;
virStoragePoolObjPtr privpool;
- size_t i = 0;
- int n = 0;
+ int n = -1;
memset(names, 0, maxnames * sizeof(*names));
@@ -4851,24 +4851,15 @@ testStoragePoolListVolumes(virStoragePoolPtr pool,
goto cleanup;
}
- for (i = 0; i < privpool->volumes.count && n < maxnames; i++) {
- if (VIR_STRDUP(names[n++], privpool->volumes.objs[i]->name) < 0)
- goto cleanup;
- }
+ n = virStoragePoolObjVolumeGetNames(&privpool->volumes, pool->conn,
+ privpool->def, NULL, names, maxnames);
+ cleanup:
virStoragePoolObjUnlock(privpool);
return n;
-
- cleanup:
- for (n = 0; n < maxnames; n++)
- VIR_FREE(names[i]);
-
- memset(names, 0, maxnames * sizeof(*names));
- if (privpool)
- virStoragePoolObjUnlock(privpool);
- return -1;
}
+
static int
testStoragePoolListAllVolumes(virStoragePoolPtr obj,
virStorageVolPtr **vols,
--
2.9.3
6:52 a.m.
New subject: [libvirt] [PATCH 2/7] storage: Introduce virStoragePoolObjVolumeGetNames
On 04/06/2017 01:48 PM, John Ferlan wrote:
Mostly code motion to move storagePoolListVolumes code into
virstorageobj.c
and rename to virStoragePoolObjVolumeGetNames.
Also includes a couple of variable name adjustments to keep code consistent
with other drivers.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 30 ++++++++++++++++++++++++++++++
src/conf/virstorageobj.h | 8 ++++++++
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 24 ++++++------------------
src/test/test_driver.c | 21 ++++++---------------
5 files changed, 51 insertions(+), 33 deletions(-)
Again, couple of useless memset()-s here.
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index e57694c..5933618 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -214,6 +214,36 @@ virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr volumes,
}
+int
+virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr volumes,
+ virConnectPtr conn,
+ virStoragePoolDefPtr pooldef,
+ virStoragePoolVolumeACLFilter aclfilter,
+ char **const names,
const? This doesn't feel right. We are allocating/freeing the strings.
They are not const in any way.
+ int maxnames)
+{
+ int nnames = 0;
+ size_t i;
+
+ for (i = 0; i < volumes->count && nnames < maxnames; i++) {
+ virStorageVolDefPtr def = volumes->objs[i];
+ if (aclfilter && !aclfilter(conn, pooldef, def))
+ continue;
+ if (VIR_STRDUP(names[nnames++], def->name) < 0)
Again, this doesn't look right. Should VIR_STRDUP() fail, nnames is
increased regardless. You're probably safe now, as VIR_ALLOC() done by
caller (not direct, but an indirect one) zeroes out the memory, so
subsequent VIR_FREE() at failure label won't crash. But we should be not
relying on it. @nnames incrementation needs to be done only after
VIR_STRDUP() succeeded.
+ goto failure;
+ }
+
+ return nnames;
+
+ failure:
+ while (--nnames >= 0)
+ VIR_FREE(names[nnames]);
+ memset(names, 0, maxnames * sizeof(*names));
+
+ return -1;
+}
+
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def)
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 3effe7a..bf88094 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -119,6 +119,14 @@ virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr volumes,
virStoragePoolDefPtr pooldef,
virStoragePoolVolumeACLFilter aclfilter);
+int
+virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr volumes,
+ virConnectPtr conn,
+ virStoragePoolDefPtr pooldef,
+ virStoragePoolVolumeACLFilter aclfilter,
+ char **const names,
+ int maxnames);
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 9580622..a635cac 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1007,6 +1007,7 @@ virStoragePoolObjRemove;
virStoragePoolObjSaveDef;
virStoragePoolObjSourceFindDuplicate;
virStoragePoolObjUnlock;
+virStoragePoolObjVolumeGetNames;
# cpu/cpu.h
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 7d2f74d..ce77fe1 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1411,14 +1411,14 @@ storagePoolNumOfVolumes(virStoragePoolPtr obj)
return ret;
}
+
static int
storagePoolListVolumes(virStoragePoolPtr obj,
char **const names,
int maxnames)
{
virStoragePoolObjPtr pool;
- size_t i;
- int n = 0;
+ int n = -1;
memset(names, 0, maxnames * sizeof(*names));
@@ -1434,24 +1434,12 @@ storagePoolListVolumes(virStoragePoolPtr obj,
goto cleanup;
}
- for (i = 0; i < pool->volumes.count && n < maxnames; i++) {
- if (!virStoragePoolListVolumesCheckACL(obj->conn, pool->def,
- pool->volumes.objs[i]))
- continue;
- if (VIR_STRDUP(names[n++], pool->volumes.objs[i]->name) < 0)
- goto cleanup;
- }
-
- virStoragePoolObjUnlock(pool);
- return n;
-
+ n = virStoragePoolObjVolumeGetNames(&pool->volumes, obj->conn,
pool->def,
+ virStoragePoolListVolumesCheckACL,
+ names, maxnames);
cleanup:
virStoragePoolObjUnlock(pool);
- for (n = 0; n < maxnames; n++)
- VIR_FREE(names[n]);
-
- memset(names, 0, maxnames * sizeof(*names));
- return -1;
+ return n;
}
static int
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index a4b5833..b2840fa 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4830,6 +4830,7 @@ testStoragePoolNumOfVolumes(virStoragePoolPtr pool)
return ret;
}
+
static int
testStoragePoolListVolumes(virStoragePoolPtr pool,
char **const names,
@@ -4837,8 +4838,7 @@ testStoragePoolListVolumes(virStoragePoolPtr pool,
{
testDriverPtr privconn = pool->conn->privateData;
virStoragePoolObjPtr privpool;
- size_t i = 0;
- int n = 0;
+ int n = -1;
memset(names, 0, maxnames * sizeof(*names));
@@ -4851,24 +4851,15 @@ testStoragePoolListVolumes(virStoragePoolPtr pool,
goto cleanup;
}
- for (i = 0; i < privpool->volumes.count && n < maxnames; i++) {
- if (VIR_STRDUP(names[n++], privpool->volumes.objs[i]->name) < 0)
- goto cleanup;
- }
+ n = virStoragePoolObjVolumeGetNames(&privpool->volumes, pool->conn,
+ privpool->def, NULL, names, maxnames);
+ cleanup:
virStoragePoolObjUnlock(privpool);
return n;
This will not fly. I mean, it will as long as privpool != NULL at this
point. You need to keep the check for privpool before calling Unlock()
over it because Unlock() does not handle NULL well. Alternatively, you
can return directly if testStoragePoolObjFindByName() fails (instead of
goto cleanup).
-
- cleanup:
- for (n = 0; n < maxnames; n++)
- VIR_FREE(names[i]);
-
- memset(names, 0, maxnames * sizeof(*names));
- if (privpool)
- virStoragePoolObjUnlock(privpool);
- return -1;
Michal
7:35 a.m.
New subject: [libvirt] [PATCH 2/7] storage: Introduce virStoragePoolObjVolumeGetNames
On 04/10/2017 07:52 AM, Michal Privoznik wrote:
On 04/06/2017 01:48 PM, John Ferlan wrote:
> Mostly code motion to move storagePoolListVolumes code into
> virstorageobj.c
> and rename to virStoragePoolObjVolumeGetNames.
>
> Also includes a couple of variable name adjustments to keep code
> consistent
> with other drivers.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/conf/virstorageobj.c | 30 ++++++++++++++++++++++++++++++
> src/conf/virstorageobj.h | 8 ++++++++
> src/libvirt_private.syms | 1 +
> src/storage/storage_driver.c | 24 ++++++------------------
> src/test/test_driver.c | 21 ++++++---------------
> 5 files changed, 51 insertions(+), 33 deletions(-)
Again, couple of useless memset()-s here.
>
> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
> index e57694c..5933618 100644
> --- a/src/conf/virstorageobj.c
> +++ b/src/conf/virstorageobj.c
> @@ -214,6 +214,36 @@
> virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr volumes,
> }
>
>
> +int
> +virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr volumes,
> + virConnectPtr conn,
> + virStoragePoolDefPtr pooldef,
> + virStoragePoolVolumeACLFilter aclfilter,
> + char **const names,
const? This doesn't feel right. We are allocating/freeing the strings.
They are not const in any way.
"char **const names" is how this has been defined since before I touched
it! It'd need to be something changed in many more places.
> + int maxnames)
> +{
> + int nnames = 0;
> + size_t i;
> +
> + for (i = 0; i < volumes->count && nnames < maxnames; i++) {
> + virStorageVolDefPtr def = volumes->objs[i];
> + if (aclfilter && !aclfilter(conn, pooldef, def))
> + continue;
> + if (VIR_STRDUP(names[nnames++], def->name) < 0)
Again, this doesn't look right. Should VIR_STRDUP() fail, nnames is
increased regardless. You're probably safe now, as VIR_ALLOC() done by
caller (not direct, but an indirect one) zeroes out the memory, so
subsequent VIR_FREE() at failure label won't crash. But we should be not
relying on it. @nnames incrementation needs to be done only after
VIR_STRDUP() succeeded.
True, but it does follow from whence it came w/ variable name changes. I
will change it though.
Guess this also trivially ;-) needs to be adjusted in the (now pushed,
<sigh>) nodedev changes as well...
> + goto failure;
> + }
> +
> + return nnames;
> +
> + failure:
> + while (--nnames >= 0)
> + VIR_FREE(names[nnames]);
> + memset(names, 0, maxnames * sizeof(*names));
> +
> + return -1;
> +}
> +
> +
> virStoragePoolObjPtr
> virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
> virStoragePoolDefPtr def)
> diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
> index 3effe7a..bf88094 100644
> --- a/src/conf/virstorageobj.h
> +++ b/src/conf/virstorageobj.h
> @@ -119,6 +119,14 @@
> virStoragePoolObjNumOfVolumes(virStorageVolDefListPtr volumes,
> virStoragePoolDefPtr pooldef,
> virStoragePoolVolumeACLFilter aclfilter);
>
> +int
> +virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr volumes,
> + virConnectPtr conn,
> + virStoragePoolDefPtr pooldef,
> + virStoragePoolVolumeACLFilter aclfilter,
> + char **const names,
> + int maxnames);
> +
> virStoragePoolObjPtr
> virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
> virStoragePoolDefPtr def);
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 9580622..a635cac 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1007,6 +1007,7 @@ virStoragePoolObjRemove;
> virStoragePoolObjSaveDef;
> virStoragePoolObjSourceFindDuplicate;
> virStoragePoolObjUnlock;
> +virStoragePoolObjVolumeGetNames;
>
>
> # cpu/cpu.h
> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index 7d2f74d..ce77fe1 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -1411,14 +1411,14 @@ storagePoolNumOfVolumes(virStoragePoolPtr obj)
> return ret;
> }
>
> +
> static int
> storagePoolListVolumes(virStoragePoolPtr obj,
> char **const names,
> int maxnames)
> {
> virStoragePoolObjPtr pool;
> - size_t i;
> - int n = 0;
> + int n = -1;
>
> memset(names, 0, maxnames * sizeof(*names));
>
> @@ -1434,24 +1434,12 @@ storagePoolListVolumes(virStoragePoolPtr obj,
> goto cleanup;
> }
>
> - for (i = 0; i < pool->volumes.count && n < maxnames; i++) {
> - if (!virStoragePoolListVolumesCheckACL(obj->conn, pool->def,
> - pool->volumes.objs[i]))
> - continue;
> - if (VIR_STRDUP(names[n++], pool->volumes.objs[i]->name) < 0)
> - goto cleanup;
> - }
> -
> - virStoragePoolObjUnlock(pool);
> - return n;
> -
> + n = virStoragePoolObjVolumeGetNames(&pool->volumes, obj->conn,
> pool->def,
> +
> virStoragePoolListVolumesCheckACL,
> + names, maxnames);
> cleanup:
> virStoragePoolObjUnlock(pool);
> - for (n = 0; n < maxnames; n++)
> - VIR_FREE(names[n]);
> -
> - memset(names, 0, maxnames * sizeof(*names));
> - return -1;
> + return n;
> }
>
> static int
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index a4b5833..b2840fa 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -4830,6 +4830,7 @@ testStoragePoolNumOfVolumes(virStoragePoolPtr pool)
> return ret;
> }
>
> +
> static int
> testStoragePoolListVolumes(virStoragePoolPtr pool,
> char **const names,
> @@ -4837,8 +4838,7 @@ testStoragePoolListVolumes(virStoragePoolPtr pool,
> {
> testDriverPtr privconn = pool->conn->privateData;
> virStoragePoolObjPtr privpool;
> - size_t i = 0;
> - int n = 0;
> + int n = -1;
>
> memset(names, 0, maxnames * sizeof(*names));
>
> @@ -4851,24 +4851,15 @@ testStoragePoolListVolumes(virStoragePoolPtr
> pool,
> goto cleanup;
> }
>
> - for (i = 0; i < privpool->volumes.count && n < maxnames; i++)
{
> - if (VIR_STRDUP(names[n++], privpool->volumes.objs[i]->name) < 0)
> - goto cleanup;
> - }
> + n = virStoragePoolObjVolumeGetNames(&privpool->volumes, pool->conn,
> + privpool->def, NULL, names,
> maxnames);
>
> + cleanup:
> virStoragePoolObjUnlock(privpool);
> return n;
This will not fly. I mean, it will as long as privpool != NULL at this
point. You need to keep the check for privpool before calling Unlock()
over it because Unlock() does not handle NULL well. Alternatively, you
can return directly if testStoragePoolObjFindByName() fails (instead of
goto cleanup).
Oh right - good catch. I'll return -1 if the ObjFindByName fails
instead (similar to how the storage_driver code does things)
Tks
John
> -
> - cleanup:
> - for (n = 0; n < maxnames; n++)
> - VIR_FREE(names[i]);
> -
> - memset(names, 0, maxnames * sizeof(*names));
> - if (privpool)
> - virStoragePoolObjUnlock(privpool);
> - return -1;
Michal
6:48 a.m.
New subject: [libvirt] [PATCH 3/7] storage: Introduce virStoragePoolObjVolumeListExport
Essentially code motion to move the storage/test driver ListAllVolumes
logic into virstorageobj.c
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 47 ++++++++++++++++++++++++++++++++++++++++++++
src/conf/virstorageobj.h | 7 +++++++
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 37 ++++------------------------------
src/test/test_driver.c | 40 ++++---------------------------------
5 files changed, 63 insertions(+), 69 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 5933618..2484517 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -244,6 +244,53 @@ virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr volumes,
}
+int
+virStoragePoolObjVolumeListExport(virConnectPtr conn,
+ virStorageVolDefListPtr volumes,
+ virStoragePoolDefPtr pooldef,
+ virStorageVolPtr **vols,
+ virStoragePoolVolumeACLFilter aclfilter)
+{
+ int ret = -1;
+ size_t i;
+ virStorageVolPtr *tmp_vols = NULL;
+ virStorageVolPtr vol = NULL;
+ int nvols = 0;
+
+ /* Just returns the volumes count */
+ if (!vols) {
+ ret = volumes->count;
+ goto cleanup;
+ }
+
+ if (VIR_ALLOC_N(tmp_vols, volumes->count + 1) < 0)
+ goto cleanup;
+
+ for (i = 0; i < volumes->count; i++) {
+ virStorageVolDefPtr def = volumes->objs[i];
+ if (aclfilter && !aclfilter(conn, pooldef, def))
+ continue;
+ if (!(vol = virGetStorageVol(conn, pooldef->name, def->name, def->key,
+ NULL, NULL)))
+ goto cleanup;
+ tmp_vols[nvols++] = vol;
+ }
+
+ *vols = tmp_vols;
+ tmp_vols = NULL;
+ ret = nvols;
+
+ cleanup:
+ if (tmp_vols) {
+ for (i = 0; i < nvols; i++)
+ virObjectUnref(tmp_vols[i]);
+ VIR_FREE(tmp_vols);
+ }
+
+ return ret;
+}
+
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def)
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index bf88094..72daa5a 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -127,6 +127,13 @@ virStoragePoolObjVolumeGetNames(virStorageVolDefListPtr volumes,
char **const names,
int maxnames);
+int
+virStoragePoolObjVolumeListExport(virConnectPtr conn,
+ virStorageVolDefListPtr volumes,
+ virStoragePoolDefPtr pooldef,
+ virStorageVolPtr **vols,
+ virStoragePoolVolumeACLFilter aclfilter);
+
virStoragePoolObjPtr
virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
virStoragePoolDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a635cac..6298019 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1008,6 +1008,7 @@ virStoragePoolObjSaveDef;
virStoragePoolObjSourceFindDuplicate;
virStoragePoolObjUnlock;
virStoragePoolObjVolumeGetNames;
+virStoragePoolObjVolumeListExport;
# cpu/cpu.h
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index ce77fe1..475e332 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1442,16 +1442,13 @@ storagePoolListVolumes(virStoragePoolPtr obj,
return n;
}
+
static int
storagePoolListAllVolumes(virStoragePoolPtr pool,
virStorageVolPtr **vols,
unsigned int flags)
{
virStoragePoolObjPtr obj;
- size_t i;
- virStorageVolPtr *tmp_vols = NULL;
- virStorageVolPtr vol = NULL;
- int nvols = 0;
int ret = -1;
virCheckFlags(0, -1);
@@ -1468,38 +1465,12 @@ storagePoolListAllVolumes(virStoragePoolPtr pool,
goto cleanup;
}
- /* Just returns the volumes count */
- if (!vols) {
- ret = obj->volumes.count;
- goto cleanup;
- }
+ ret = virStoragePoolObjVolumeListExport(pool->conn, &obj->volumes,
+ obj->def, vols,
+ virStoragePoolListAllVolumesCheckACL);
- if (VIR_ALLOC_N(tmp_vols, obj->volumes.count + 1) < 0)
- goto cleanup;
-
- for (i = 0; i < obj->volumes.count; i++) {
- if (!virStoragePoolListAllVolumesCheckACL(pool->conn, obj->def,
- obj->volumes.objs[i]))
- continue;
- if (!(vol = virGetStorageVol(pool->conn, obj->def->name,
- obj->volumes.objs[i]->name,
- obj->volumes.objs[i]->key,
- NULL, NULL)))
- goto cleanup;
- tmp_vols[nvols++] = vol;
- }
-
- *vols = tmp_vols;
- tmp_vols = NULL;
- ret = nvols;
cleanup:
- if (tmp_vols) {
- for (i = 0; i < nvols; i++)
- virObjectUnref(tmp_vols[i]);
- VIR_FREE(tmp_vols);
- }
-
virStoragePoolObjUnlock(obj);
return ret;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index b2840fa..44bd47c 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4867,16 +4867,12 @@ testStoragePoolListAllVolumes(virStoragePoolPtr obj,
{
testDriverPtr privconn = obj->conn->privateData;
virStoragePoolObjPtr pool;
- size_t i;
- virStorageVolPtr *tmp_vols = NULL;
- virStorageVolPtr vol = NULL;
- int nvols = 0;
int ret = -1;
virCheckFlags(0, -1);
if (!(pool = testStoragePoolObjFindByUUID(privconn, obj->uuid)))
- goto cleanup;
+ return -1;
if (!virStoragePoolObjIsActive(pool)) {
virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -4884,39 +4880,11 @@ testStoragePoolListAllVolumes(virStoragePoolPtr obj,
goto cleanup;
}
- /* Just returns the volumes count */
- if (!vols) {
- ret = pool->volumes.count;
- goto cleanup;
- }
-
- if (VIR_ALLOC_N(tmp_vols, pool->volumes.count + 1) < 0)
- goto cleanup;
-
- for (i = 0; i < pool->volumes.count; i++) {
- if (!(vol = virGetStorageVol(obj->conn, pool->def->name,
- pool->volumes.objs[i]->name,
- pool->volumes.objs[i]->key,
- NULL, NULL)))
- goto cleanup;
- tmp_vols[nvols++] = vol;
- }
-
- *vols = tmp_vols;
- tmp_vols = NULL;
- ret = nvols;
+ ret = virStoragePoolObjVolumeListExport(obj->conn, &pool->volumes,
+ pool->def, vols, NULL);
cleanup:
- if (tmp_vols) {
- for (i = 0; i < nvols; i++) {
- if (tmp_vols[i])
- virStorageVolFree(tmp_vols[i]);
- }
- VIR_FREE(tmp_vols);
- }
-
- if (pool)
- virStoragePoolObjUnlock(pool);
+ virStoragePoolObjUnlock(pool);
return ret;
}
--
2.9.3
6:48 a.m.
New subject: [libvirt] [PATCH 4/7] storage: Introduce virStoragePoolObjNumOfStoragePools
Unify the NumOf[Defined]StoragePools API into virstorageobj.c from
storage_driver and test_driver. The only real difference between the
two is the test driver doesn't call using the aclfilter API.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 24 ++++++++++++++++++++++++
src/conf/virstorageobj.h | 9 +++++++++
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 22 ++++------------------
src/test/test_driver.c | 17 ++++++-----------
5 files changed, 44 insertions(+), 29 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 2484517..f0201aa 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -556,6 +556,30 @@ virStoragePoolObjDeleteDef(virStoragePoolObjPtr pool)
}
+int
+virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools,
+ virConnectPtr conn,
+ bool wantActive,
+ virStoragePoolObjListACLFilter aclfilter)
+{
+ int npools = 0;
+ size_t i;
+
+ for (i = 0; i < pools->count; i++) {
+ virStoragePoolObjPtr obj = pools->objs[i];
+ virStoragePoolObjLock(obj);
+ if (!aclfilter || aclfilter(conn, obj->def)) {
+ if ((wantActive && virStoragePoolObjIsActive(obj)) ||
+ (!wantActive && !virStoragePoolObjIsActive(obj)))
+ npools++;
+ }
+ virStoragePoolObjUnlock(obj);
+ }
+
+ return npools;
+}
+
+
/*
* virStoragePoolObjIsDuplicate:
* @doms : virStoragePoolObjListPtr to search
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index 72daa5a..5eeda1e 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -146,6 +146,15 @@ virStoragePoolObjSaveDef(virStorageDriverStatePtr driver,
int
virStoragePoolObjDeleteDef(virStoragePoolObjPtr pool);
+typedef bool (*virStoragePoolObjListACLFilter)(virConnectPtr conn,
+ virStoragePoolDefPtr def);
+
+int
+virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools,
+ virConnectPtr conn,
+ bool wantActive,
+ virStoragePoolObjListACLFilter aclfilter);
+
void
virStoragePoolObjFree(virStoragePoolObjPtr pool);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6298019..7bc3bc4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1002,6 +1002,7 @@ virStoragePoolObjListFree;
virStoragePoolObjLoadAllConfigs;
virStoragePoolObjLoadAllState;
virStoragePoolObjLock;
+virStoragePoolObjNumOfStoragePools;
virStoragePoolObjNumOfVolumes;
virStoragePoolObjRemove;
virStoragePoolObjSaveDef;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 475e332..065380b 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -477,21 +477,14 @@ storagePoolLookupByVolume(virStorageVolPtr vol)
static int
storageConnectNumOfStoragePools(virConnectPtr conn)
{
- size_t i;
int nactive = 0;
if (virConnectNumOfStoragePoolsEnsureACL(conn) < 0)
return -1;
storageDriverLock();
- for (i = 0; i < driver->pools.count; i++) {
- virStoragePoolObjPtr obj = driver->pools.objs[i];
- virStoragePoolObjLock(obj);
- if (virConnectNumOfStoragePoolsCheckACL(conn, obj->def) &&
- virStoragePoolObjIsActive(obj))
- nactive++;
- virStoragePoolObjUnlock(obj);
- }
+ nactive = virStoragePoolObjNumOfStoragePools(&driver->pools, conn, true,
+ virConnectNumOfStoragePoolsCheckACL);
storageDriverUnlock();
return nactive;
@@ -536,21 +529,14 @@ storageConnectListStoragePools(virConnectPtr conn,
static int
storageConnectNumOfDefinedStoragePools(virConnectPtr conn)
{
- size_t i;
int nactive = 0;
if (virConnectNumOfDefinedStoragePoolsEnsureACL(conn) < 0)
return -1;
storageDriverLock();
- for (i = 0; i < driver->pools.count; i++) {
- virStoragePoolObjPtr obj = driver->pools.objs[i];
- virStoragePoolObjLock(obj);
- if (virConnectNumOfDefinedStoragePoolsCheckACL(conn, obj->def) &&
- !virStoragePoolObjIsActive(obj))
- nactive++;
- virStoragePoolObjUnlock(obj);
- }
+ nactive = virStoragePoolObjNumOfStoragePools(&driver->pools, conn, false,
+
virConnectNumOfDefinedStoragePoolsCheckACL);
storageDriverUnlock();
return nactive;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 44bd47c..4fb9915 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4115,17 +4115,16 @@ testStoragePoolLookupByVolume(virStorageVolPtr vol)
return testStoragePoolLookupByName(vol->conn, vol->pool);
}
+
static int
testConnectNumOfStoragePools(virConnectPtr conn)
{
testDriverPtr privconn = conn->privateData;
int numActive = 0;
- size_t i;
testDriverLock(privconn);
- for (i = 0; i < privconn->pools.count; i++)
- if (virStoragePoolObjIsActive(privconn->pools.objs[i]))
- numActive++;
+ numActive = virStoragePoolObjNumOfStoragePools(&privconn->pools, conn,
+ true, NULL);
testDriverUnlock(privconn);
return numActive;
@@ -4162,20 +4161,16 @@ testConnectListStoragePools(virConnectPtr conn,
return -1;
}
+
static int
testConnectNumOfDefinedStoragePools(virConnectPtr conn)
{
testDriverPtr privconn = conn->privateData;
int numInactive = 0;
- size_t i;
testDriverLock(privconn);
- for (i = 0; i < privconn->pools.count; i++) {
- virStoragePoolObjLock(privconn->pools.objs[i]);
- if (!virStoragePoolObjIsActive(privconn->pools.objs[i]))
- numInactive++;
- virStoragePoolObjUnlock(privconn->pools.objs[i]);
- }
+ numInactive = virStoragePoolObjNumOfStoragePools(&privconn->pools, conn,
+ false, NULL);
testDriverUnlock(privconn);
return numInactive;
--
2.9.3
6:52 a.m.
New subject: [libvirt] [PATCH 4/7] storage: Introduce virStoragePoolObjNumOfStoragePools
On 04/06/2017 01:48 PM, John Ferlan wrote:
Unify the NumOf[Defined]StoragePools API into virstorageobj.c from
storage_driver and test_driver. The only real difference between the
two is the test driver doesn't call using the aclfilter API.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 24 ++++++++++++++++++++++++
src/conf/virstorageobj.h | 9 +++++++++
src/libvirt_private.syms | 1 +
src/storage/storage_driver.c | 22 ++++------------------
src/test/test_driver.c | 17 ++++++-----------
5 files changed, 44 insertions(+), 29 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 2484517..f0201aa 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -556,6 +556,30 @@ virStoragePoolObjDeleteDef(virStoragePoolObjPtr pool)
}
+int
+virStoragePoolObjNumOfStoragePools(virStoragePoolObjListPtr pools,
+ virConnectPtr conn,
+ bool wantActive,
+ virStoragePoolObjListACLFilter aclfilter)
+{
+ int npools = 0;
+ size_t i;
+
+ for (i = 0; i < pools->count; i++) {
+ virStoragePoolObjPtr obj = pools->objs[i];
+ virStoragePoolObjLock(obj);
+ if (!aclfilter || aclfilter(conn, obj->def)) {
+ if ((wantActive && virStoragePoolObjIsActive(obj)) ||
+ (!wantActive && !virStoragePoolObjIsActive(obj)))
Again, if (wantActive == virStoragePoolObjIsActive()) ...
+ npools++;
+ }
+ virStoragePoolObjUnlock(obj);
+ }
+
+ return npools;
+}
+
+
Michal
6:48 a.m.
New subject: [libvirt] [PATCH 5/7] storage: Introduce virStoragePoolObjGetNames
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(a)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
6:52 a.m.
New subject: [libvirt] [PATCH 5/7] storage: Introduce virStoragePoolObjGetNames
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(a)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
6:48 a.m.
New subject: [libvirt] [PATCH 6/7] storage: Pass driver arg by ref
Alter virStoragePoolObjListExport in order to pass the drivers->pools
by reference
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 8 ++++----
src/conf/virstorageobj.h | 2 +-
src/storage/storage_driver.c | 2 +-
src/test/test_driver.c | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index a9367a2..34a3225 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -1100,7 +1100,7 @@ virStoragePoolMatch(virStoragePoolObjPtr poolobj,
int
virStoragePoolObjListExport(virConnectPtr conn,
- virStoragePoolObjList poolobjs,
+ virStoragePoolObjListPtr poolobjs,
virStoragePoolPtr **pools,
virStoragePoolObjListFilter filter,
unsigned int flags)
@@ -1111,11 +1111,11 @@ virStoragePoolObjListExport(virConnectPtr conn,
int ret = -1;
size_t i;
- if (pools && VIR_ALLOC_N(tmp_pools, poolobjs.count + 1) < 0)
+ if (pools && VIR_ALLOC_N(tmp_pools, poolobjs->count + 1) < 0)
goto cleanup;
- for (i = 0; i < poolobjs.count; i++) {
- virStoragePoolObjPtr poolobj = poolobjs.objs[i];
+ for (i = 0; i < poolobjs->count; i++) {
+ virStoragePoolObjPtr poolobj = poolobjs->objs[i];
virStoragePoolObjLock(poolobj);
if ((!filter || filter(conn, poolobj->def)) &&
virStoragePoolMatch(poolobj, flags)) {
diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
index b651865..7c11910 100644
--- a/src/conf/virstorageobj.h
+++ b/src/conf/virstorageobj.h
@@ -191,7 +191,7 @@ virStoragePoolObjUnlock(virStoragePoolObjPtr obj);
int
virStoragePoolObjListExport(virConnectPtr conn,
- virStoragePoolObjList poolobjs,
+ virStoragePoolObjListPtr poolobjs,
virStoragePoolPtr **pools,
virStoragePoolObjListFilter filter,
unsigned int flags);
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 9493da8..6b8bbb5 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2653,7 +2653,7 @@ storageConnectListAllStoragePools(virConnectPtr conn,
goto cleanup;
storageDriverLock();
- ret = virStoragePoolObjListExport(conn, driver->pools, pools,
+ ret = virStoragePoolObjListExport(conn, &driver->pools, pools,
virConnectListAllStoragePoolsCheckACL,
flags);
storageDriverUnlock();
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 1a6601e..a2a17ef 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4194,7 +4194,7 @@ testConnectListAllStoragePools(virConnectPtr conn,
virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ALL, -1);
testDriverLock(privconn);
- ret = virStoragePoolObjListExport(conn, privconn->pools, pools,
+ ret = virStoragePoolObjListExport(conn, &privconn->pools, pools,
NULL, flags);
testDriverUnlock(privconn);
--
2.9.3
6:48 a.m.
New subject: [libvirt] [PATCH 7/7] storage: Create helpers to perform FindByUUID and FindByName
Create a couple of helpers that will perform the same call sequence.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_driver.c | 144 ++++++++++++++++++-------------------------
1 file changed, 59 insertions(+), 85 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 6b8bbb5..8b55acc 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -387,6 +387,56 @@ storageStateCleanup(void)
}
+static virStoragePoolObjPtr
+storagePoolObjFindByUUID(const unsigned char *uuid,
+ const char *name)
+{
+ virStoragePoolObjPtr pool;
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+ if (!(pool = virStoragePoolObjFindByUUID(&driver->pools, uuid))) {
+ virUUIDFormat(uuid, uuidstr);
+ if (name)
+ virReportError(VIR_ERR_NO_STORAGE_POOL,
+ _("no storage pool with matching uuid '%s'
(%s)"),
+ uuidstr, name);
+ else
+ virReportError(VIR_ERR_NO_STORAGE_POOL,
+ _("no storage pool with matching uuid
'%s'"),
+ uuidstr);
+ }
+
+ return pool;
+}
+
+
+static virStoragePoolObjPtr
+virStoragePoolObjFromStoragePool(virStoragePoolPtr pool)
+{
+ virStoragePoolObjPtr ret;
+
+ storageDriverLock();
+ ret = storagePoolObjFindByUUID(pool->uuid, pool->name);
+ storageDriverUnlock();
+
+ return ret;
+}
+
+
+static virStoragePoolObjPtr
+storagePoolObjFindByName(const char *name)
+{
+ virStoragePoolObjPtr pool;
+
+ storageDriverLock();
+ if (!(pool = virStoragePoolObjFindByName(&driver->pools, name)))
+ virReportError(VIR_ERR_NO_STORAGE_POOL,
+ _("no storage pool with matching name '%s'"),
name);
+ storageDriverUnlock();
+
+ return pool;
+}
+
static virStoragePoolPtr
storagePoolLookupByUUID(virConnectPtr conn,
@@ -396,16 +446,10 @@ storagePoolLookupByUUID(virConnectPtr conn,
virStoragePoolPtr ret = NULL;
storageDriverLock();
- pool = virStoragePoolObjFindByUUID(&driver->pools, uuid);
+ pool = storagePoolObjFindByUUID(uuid, NULL);
storageDriverUnlock();
-
- if (!pool) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(uuid, uuidstr);
- virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching uuid '%s'"),
uuidstr);
+ if (!pool)
return NULL;
- }
if (virStoragePoolLookupByUUIDEnsureACL(conn, pool->def) < 0)
goto cleanup;
@@ -425,15 +469,8 @@ storagePoolLookupByName(virConnectPtr conn,
virStoragePoolObjPtr pool;
virStoragePoolPtr ret = NULL;
- storageDriverLock();
- pool = virStoragePoolObjFindByName(&driver->pools, name);
- storageDriverUnlock();
-
- if (!pool) {
- virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching name '%s'"),
name);
+ if (!(pool = storagePoolObjFindByName(name)))
return NULL;
- }
if (virStoragePoolLookupByNameEnsureACL(conn, pool->def) < 0)
goto cleanup;
@@ -452,16 +489,8 @@ storagePoolLookupByVolume(virStorageVolPtr vol)
virStoragePoolObjPtr pool;
virStoragePoolPtr ret = NULL;
- storageDriverLock();
- pool = virStoragePoolObjFindByName(&driver->pools, vol->pool);
- storageDriverUnlock();
-
- if (!pool) {
- virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching name '%s'"),
- vol->pool);
+ if (!(pool = storagePoolObjFindByName(vol->pool)))
return NULL;
- }
if (virStoragePoolLookupByVolumeEnsureACL(vol->conn, pool->def) < 0)
goto cleanup;
@@ -588,25 +617,6 @@ storageConnectFindStoragePoolSources(virConnectPtr conn,
}
-static virStoragePoolObjPtr
-virStoragePoolObjFromStoragePool(virStoragePoolPtr pool)
-{
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virStoragePoolObjPtr ret;
-
- storageDriverLock();
- if (!(ret = virStoragePoolObjFindByUUID(&driver->pools, pool->uuid))) {
- virUUIDFormat(pool->uuid, uuidstr);
- virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching uuid '%s'
(%s)"),
- uuidstr, pool->name);
- }
- storageDriverUnlock();
-
- return ret;
-}
-
-
static int storagePoolIsActive(virStoragePoolPtr pool)
{
virStoragePoolObjPtr obj;
@@ -813,14 +823,8 @@ storagePoolUndefine(virStoragePoolPtr obj)
int ret = -1;
storageDriverLock();
- if (!(pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid))) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(obj->uuid, uuidstr);
- virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching uuid '%s'
(%s)"),
- uuidstr, obj->name);
+ if (!(pool = storagePoolObjFindByUUID(obj->uuid, obj->name)))
goto cleanup;
- }
if (virStoragePoolUndefineEnsureACL(obj->conn, pool->def) < 0)
goto cleanup;
@@ -999,14 +1003,8 @@ storagePoolDestroy(virStoragePoolPtr obj)
int ret = -1;
storageDriverLock();
- if (!(pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid))) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(obj->uuid, uuidstr);
- virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching uuid '%s'
(%s)"),
- uuidstr, obj->name);
+ if (!(pool = storagePoolObjFindByUUID(obj->uuid, obj->name)))
goto cleanup;
- }
if (virStoragePoolDestroyEnsureACL(obj->conn, pool->def) < 0)
goto cleanup;
@@ -1133,14 +1131,8 @@ storagePoolRefresh(virStoragePoolPtr obj,
virCheckFlags(0, -1);
storageDriverLock();
- if (!(pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid))) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(obj->uuid, uuidstr);
- virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching uuid '%s'
(%s)"),
- uuidstr, obj->name);
+ if (!(pool = storagePoolObjFindByUUID(obj->uuid, obj->name)))
goto cleanup;
- }
if (virStoragePoolRefreshEnsureACL(obj->conn, pool->def) < 0)
goto cleanup;
@@ -1283,16 +1275,8 @@ storagePoolSetAutostart(virStoragePoolPtr obj,
int ret = -1;
storageDriverLock();
- pool = virStoragePoolObjFindByUUID(&driver->pools, obj->uuid);
-
- if (!pool) {
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virUUIDFormat(obj->uuid, uuidstr);
- virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching uuid '%s'
(%s)"),
- uuidstr, obj->name);
+ if (!(pool = storagePoolObjFindByUUID(obj->uuid, obj->name)))
goto cleanup;
- }
if (virStoragePoolSetAutostartEnsureACL(obj->conn, pool->def) < 0)
goto cleanup;
@@ -1713,18 +1697,8 @@ virStorageVolDefFromVol(virStorageVolPtr obj,
{
virStorageVolDefPtr vol = NULL;
- *pool = NULL;
-
- storageDriverLock();
- *pool = virStoragePoolObjFindByName(&driver->pools, obj->pool);
- storageDriverUnlock();
-
- if (!*pool) {
- virReportError(VIR_ERR_NO_STORAGE_POOL,
- _("no storage pool with matching name '%s'"),
- obj->pool);
+ if (!(*pool = storagePoolObjFindByName(obj->pool)))
return NULL;
- }
if (!virStoragePoolObjIsActive(*pool)) {
virReportError(VIR_ERR_OPERATION_INVALID,
--
2.9.3
6:52 a.m.
On 04/06/2017 01:48 PM, John Ferlan wrote:
Merge code in storage_driver.c and test_driver into virstorageobj to
count the number of volumes/pools, to return a list of names, return
the collected list of objects for volumes/pools. For volumes, that's
moved code, while for pools that's just changing the export API to
take an address of 'devobjs' to follow other usages. I also added
code to converge the FindBy{UUID|Name}
FWIW: This is part of the common driver objects code I've been working
through. I figured I will post each driver separately rather than one
John Ferlan (7):
storage: Introduce virStoragePoolObjNumOfVolumes
storage: Introduce virStoragePoolObjVolumeGetNames
storage: Introduce virStoragePoolObjVolumeListExport
storage: Introduce virStoragePoolObjNumOfStoragePools
storage: Introduce virStoragePoolObjGetNames
storage: Pass driver arg by ref
storage: Create helpers to perform FindByUUID and FindByName
src/conf/virstorageobj.c | 165 +++++++++++++++++++++++-
src/conf/virstorageobj.h | 45 ++++++-
src/libvirt_private.syms | 5 +
src/storage/storage_driver.c | 297 +++++++++++++------------------------------
src/test/test_driver.c | 131 +++++--------------
5 files changed, 332 insertions(+), 311 deletions(-)
ACK series, but please do see my comments before pushing - you'll need
to fix some nits.
Michal
2817
days inactive
2821
days old
12 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik