On 08/24/2017 03:08 PM, John Ferlan wrote:
> Create/use a helper to perform object allocation.
>
> Adjust storagevolxml2argvtest.c in order to use the allocator and
> setting of the obj->def.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> src/conf/virstorageobj.c | 31 +++++++++++++++++++++----------
> src/conf/virstorageobj.h | 3 +++
> src/libvirt_private.syms | 1 +
> tests/storagevolxml2argvtest.c | 20 +++++++++++---------
> 4 files changed, 36 insertions(+), 19 deletions(-)
>
> diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
> index ebda9fe..eb7664c 100644
> --- a/src/conf/virstorageobj.c
> +++ b/src/conf/virstorageobj.c
> @@ -37,6 +37,26 @@
> VIR_LOG_INIT("conf.virstorageobj");
>
>
> +virStoragePoolObjPtr
> +virStoragePoolObjNew(void)
> +{
> + virStoragePoolObjPtr obj;
> +
> + if (VIR_ALLOC(obj) < 0)
> + return NULL;
> +
> + if (virMutexInit(&obj->lock) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("cannot initialize mutex"));
> + VIR_FREE(obj);
> + return NULL;
> + }
> + virStoragePoolObjLock(obj);
> + obj->active = 0;
> + return obj;
> +}
> +
> +
> virStoragePoolDefPtr
> virStoragePoolObjGetDef(virStoragePoolObjPtr obj)
> {
> @@ -421,17 +441,8 @@ virStoragePoolObjAssignDef(virStoragePoolObjListPtr pools,
> return obj;
> }
>
> - if (VIR_ALLOC(obj) < 0)
> - return NULL;
> -
> - if (virMutexInit(&obj->lock) < 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("cannot initialize mutex"));
> - VIR_FREE(obj);
> + if (!(obj = virStoragePoolObjNew()))
> return NULL;
> - }
> - virStoragePoolObjLock(obj);
> - obj->active = 0;
>
> if (VIR_APPEND_ELEMENT_COPY(pools->objs, pools->count, obj) < 0) {
> virStoragePoolObjUnlock(obj);
> diff --git a/src/conf/virstorageobj.h b/src/conf/virstorageobj.h
> index a5ed8f8..401c4d5 100644
> --- a/src/conf/virstorageobj.h
> +++ b/src/conf/virstorageobj.h
> @@ -70,6 +70,9 @@ typedef bool
> (*virStoragePoolObjListFilter)(virConnectPtr conn,
> virStoragePoolDefPtr def);
>
> +virStoragePoolObjPtr
> +virStoragePoolObjNew(void);
> +
> virStoragePoolDefPtr
> virStoragePoolObjGetDef(virStoragePoolObjPtr obj);
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 8bb1cbc..fc884fa 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1071,6 +1071,7 @@ virStoragePoolObjListFree;
> virStoragePoolObjLoadAllConfigs;
> virStoragePoolObjLoadAllState;
> virStoragePoolObjLock;
> +virStoragePoolObjNew;
> virStoragePoolObjNumOfStoragePools;
> virStoragePoolObjNumOfVolumes;
> virStoragePoolObjRemove;
> diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c
> index 9e43204..2ee3dba 100644
> --- a/tests/storagevolxml2argvtest.c
> +++ b/tests/storagevolxml2argvtest.c
> @@ -50,18 +50,19 @@ testCompareXMLToArgvFiles(bool shouldFail,
> virConnectPtr conn;
>
> virStorageVolDefPtr vol = NULL, inputvol = NULL;
> - virStoragePoolDefPtr pool = NULL;
> + virStoragePoolDefPtr def = NULL;
> virStoragePoolDefPtr inputpool = NULL;
> - virStoragePoolObj poolobj = {.def = NULL };
> -
> + virStoragePoolObjPtr obj = NULL;
>
> if (!(conn = virGetConnect()))
> goto cleanup;
>
> - if (!(pool = virStoragePoolDefParseFile(poolxml)))
> + if (!(def = virStoragePoolDefParseFile(poolxml)))
> goto cleanup;
>
> - poolobj.def = pool;
> + if (!(obj = virStoragePoolObjNew()))
> + goto cleanup;
> + virStoragePoolObjSetDef(obj, def);
>
> if (inputpoolxml) {
> if (!(inputpool = virStoragePoolDefParseFile(inputpoolxml)))
> @@ -71,17 +72,17 @@ testCompareXMLToArgvFiles(bool shouldFail,
> if (inputvolxml)
> parse_flags |= VIR_VOL_XML_PARSE_NO_CAPACITY;
>
> - if (!(vol = virStorageVolDefParseFile(pool, volxml, parse_flags)))
> + if (!(vol = virStorageVolDefParseFile(def, volxml, parse_flags)))
> goto cleanup;
>
> if (inputvolxml &&
> !(inputvol = virStorageVolDefParseFile(inputpool, inputvolxml, 0)))
> goto cleanup;
>
> - testSetVolumeType(vol, pool);
> + testSetVolumeType(vol, def);
> testSetVolumeType(inputvol, inputpool);
>
> - cmd = virStorageBackendCreateQemuImgCmdFromVol(conn, &poolobj, vol,
> + cmd = virStorageBackendCreateQemuImgCmdFromVol(conn, obj, vol,
> inputvol, flags,
> create_tool, imgformat,
> NULL);
> @@ -102,12 +103,13 @@ testCompareXMLToArgvFiles(bool shouldFail,
> ret = 0;
>
> cleanup:
> - virStoragePoolDefFree(pool);
> + virStoragePoolDefFree(def);
> virStoragePoolDefFree(inputpool);
> virStorageVolDefFree(vol);
> virStorageVolDefFree(inputvol);
> virCommandFree(cmd);
> VIR_FREE(actualCmdline);
> + virStoragePoolObjUnlock(obj);
Almost. Firstly, @def is now part of @obj so it shouldn't be freed
separately. Secondly, @obj is leaked. ACK if you squash this in:
diff --git i/tests/Makefile.am w/tests/Makefile.am
index 1f8c4cd42..813888575 100644
--- i/tests/Makefile.am
+++ w/tests/Makefile.am
@@ -885,7 +885,10 @@ storagevolxml2argvtest_SOURCES = \
testutils.c testutils.h
storagevolxml2argvtest_LDADD = \
$(LIBXML_LIBS) \
- ../src/libvirt_driver_storage_impl.la $(LDADDS)
+ ../src/libvirt_driver_storage_impl.la \
+ ../src/libvirt_conf.la \
+ ../src/libvirt_util.la \
+ $(LDADDS)
else ! WITH_STORAGE
EXTRA_DIST += storagevolxml2argvtest.c
diff --git i/tests/storagevolxml2argvtest.c w/tests/storagevolxml2argvtest.c
index 2ee3dbad9..1b3003216 100644
--- i/tests/storagevolxml2argvtest.c
+++ w/tests/storagevolxml2argvtest.c
@@ -60,8 +60,10 @@ testCompareXMLToArgvFiles(bool shouldFail,
if (!(def = virStoragePoolDefParseFile(poolxml)))
goto cleanup;
- if (!(obj = virStoragePoolObjNew()))
+ if (!(obj = virStoragePoolObjNew())) {
+ virStoragePoolDefFree(def);
goto cleanup;
+ }
virStoragePoolObjSetDef(obj, def);
if (inputpoolxml) {
@@ -103,13 +105,13 @@ testCompareXMLToArgvFiles(bool shouldFail,
ret = 0;
cleanup:
- virStoragePoolDefFree(def);
virStoragePoolDefFree(inputpool);
virStorageVolDefFree(vol);
virStorageVolDefFree(inputvol);
virCommandFree(cmd);
VIR_FREE(actualCmdline);
virStoragePoolObjUnlock(obj);
+ virStoragePoolObjFree(obj);
virObjectUnref(conn);
return ret;
}
Alternatively, you can leave the virStoragePoolDefFree() call as is and
just set @def = NULL right after virStoragePoolObjSetDef(). I'll leave it
up to you which one you prefer.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
