[libvirt] [PATCH v3 9/9] interface: Convert virInterfaceObj to use virObjectLockable
Bjoern Walk
bwalk at linux.vnet.ibm.com
Tue May 30 12:13:19 UTC 2017
John Ferlan <jferlan at redhat.com> [2017-05-30, 06:43AM -0400]:
> [...]
> void
>-virInterfaceObjFree(virInterfaceObjPtr obj)
>+virInterfaceObjEndAPI(virInterfaceObjPtr *obj)
Naming is hard, and I don't have any better suggestion. Just wanted to
say that the name is, maybe, improvable :)
> {
>- if (!obj)
>+ if (!*obj)
> return;
>
>- virInterfaceDefFree(obj->def);
>- virMutexDestroy(&obj->lock);
>- VIR_FREE(obj);
>+ virObjectUnlock(*obj);
>+ virObjectUnref(*obj);
>+ *obj = NULL;
I'm a bit conflicted if this is strictly necessary. In what situation
would this bite a consumer if we don't NULL obj? At least in this patch
I can't find any.
> }
>
>
>@@ -140,18 +150,18 @@ virInterfaceObjListFindByMACString(virInterfaceObjListPtr interfaces,
> virInterfaceObjPtr obj = interfaces->objs[i];
> virInterfaceDefPtr def;
>
>- virInterfaceObjLock(obj);
>+ virObjectLock(obj);
> def = obj->def;
> if (STRCASEEQ(def->mac, mac)) {
> if (matchct < maxmatches) {
> if (VIR_STRDUP(matches[matchct], def->name) < 0) {
>- virInterfaceObjUnlock(obj);
>+ virObjectUnlock(obj);
> goto error;
> }
> matchct++;
> }
> }
>- virInterfaceObjUnlock(obj);
>+ virObjectUnlock(obj);
> }
> return matchct;
>
>@@ -173,11 +183,11 @@ virInterfaceObjListFindByName(virInterfaceObjListPtr interfaces,
> virInterfaceObjPtr obj = interfaces->objs[i];
> virInterfaceDefPtr def;
>
>- virInterfaceObjLock(obj);
>+ virObjectLock(obj);
> def = obj->def;
> if (STREQ(def->name, name))
>- return obj;
>- virInterfaceObjUnlock(obj);
>+ return virObjectRef(obj);
>+ virObjectUnlock(obj);
> }
>
> return NULL;
>@@ -190,7 +200,7 @@ virInterfaceObjListFree(virInterfaceObjListPtr interfaces)
> size_t i;
>
> for (i = 0; i < interfaces->count; i++)
>- virInterfaceObjFree(interfaces->objs[i]);
>+ virObjectUnref(interfaces->objs[i]);
> VIR_FREE(interfaces->objs);
> VIR_FREE(interfaces);
> }
>@@ -227,7 +237,7 @@ virInterfaceObjListClone(virInterfaceObjListPtr interfaces)
> VIR_FREE(xml);
> if (!(obj = virInterfaceObjListAssignDef(dest, backup)))
> goto error;
>- virInterfaceObjUnlock(obj); /* locked by virInterfaceObjListAssignDef */
>+ virInterfaceObjEndAPI(&obj);
> }
>
> return dest;
>@@ -256,13 +266,12 @@ virInterfaceObjListAssignDef(virInterfaceObjListPtr interfaces,
>
> if (VIR_APPEND_ELEMENT_COPY(interfaces->objs,
> interfaces->count, obj) < 0) {
>- virInterfaceObjUnlock(obj);
>- virInterfaceObjFree(obj);
>+ virInterfaceObjEndAPI(&obj);
> return NULL;
> }
>
> obj->def = def;
>- return obj;
>+ return virObjectRef(obj);
>
> }
>
>@@ -273,17 +282,17 @@ virInterfaceObjListRemove(virInterfaceObjListPtr interfaces,
> {
> size_t i;
>
>- virInterfaceObjUnlock(obj);
>+ virObjectUnlock(obj);
> for (i = 0; i < interfaces->count; i++) {
>- virInterfaceObjLock(interfaces->objs[i]);
>+ virObjectLock(interfaces->objs[i]);
> if (interfaces->objs[i] == obj) {
>- virInterfaceObjUnlock(interfaces->objs[i]);
>- virInterfaceObjFree(interfaces->objs[i]);
>+ virObjectUnlock(interfaces->objs[i]);
>+ virObjectUnref(interfaces->objs[i]);
Here you could use virInterfaceObjEndAPI if the nulling would be
dropped. Small advantage, I know.
>
> VIR_DELETE_ELEMENT(interfaces->objs, i, interfaces->count);
> break;
> }
>- virInterfaceObjUnlock(interfaces->objs[i]);
>+ virObjectUnlock(interfaces->objs[i]);
> }
> }
>
>@@ -297,10 +306,10 @@ virInterfaceObjListNumOfInterfaces(virInterfaceObjListPtr interfaces,
>
> for (i = 0; (i < interfaces->count); i++) {
> virInterfaceObjPtr obj = interfaces->objs[i];
>- virInterfaceObjLock(obj);
>+ virObjectLock(obj);
> if (wantActive == virInterfaceObjIsActive(obj))
> ninterfaces++;
>- virInterfaceObjUnlock(obj);
>+ virObjectUnlock(obj);
> }
>
> return ninterfaces;
>@@ -320,16 +329,16 @@ virInterfaceObjListGetNames(virInterfaceObjListPtr interfaces,
> virInterfaceObjPtr obj = interfaces->objs[i];
> virInterfaceDefPtr def;
>
>- virInterfaceObjLock(obj);
>+ virObjectLock(obj);
> def = obj->def;
> if (wantActive == virInterfaceObjIsActive(obj)) {
> if (VIR_STRDUP(names[nnames], def->name) < 0) {
>- virInterfaceObjUnlock(obj);
>+ virObjectUnlock(obj);
> goto failure;
> }
> nnames++;
> }
>- virInterfaceObjUnlock(obj);
>+ virObjectUnlock(obj);
> }
>
> return nnames;
>diff --git a/src/conf/virinterfaceobj.h b/src/conf/virinterfaceobj.h
>index 3934e63..2b9e1b2 100644
>--- a/src/conf/virinterfaceobj.h
>+++ b/src/conf/virinterfaceobj.h
>@@ -28,6 +28,9 @@ typedef virInterfaceObj *virInterfaceObjPtr;
> typedef struct _virInterfaceObjList virInterfaceObjList;
> typedef virInterfaceObjList *virInterfaceObjListPtr;
>
>+void
>+virInterfaceObjEndAPI(virInterfaceObjPtr *obj);
>+
> virInterfaceDefPtr
> virInterfaceObjGetDef(virInterfaceObjPtr obj);
>
>@@ -68,12 +71,6 @@ void
> virInterfaceObjListRemove(virInterfaceObjListPtr interfaces,
> virInterfaceObjPtr obj);
>
>-void
>-virInterfaceObjLock(virInterfaceObjPtr obj);
>-
>-void
>-virInterfaceObjUnlock(virInterfaceObjPtr obj);
>-
> typedef bool
> (*virInterfaceObjListFilter)(virConnectPtr conn,
> virInterfaceDefPtr def);
>diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>index 9be50cb..5d6cb5e 100644
>--- a/src/libvirt_private.syms
>+++ b/src/libvirt_private.syms
>@@ -910,6 +910,7 @@ virDomainObjListRename;
>
>
> # conf/virinterfaceobj.h
>+virInterfaceObjEndAPI;
> virInterfaceObjGetDef;
> virInterfaceObjIsActive;
> virInterfaceObjListAssignDef;
>@@ -921,9 +922,7 @@ virInterfaceObjListGetNames;
> virInterfaceObjListNew;
> virInterfaceObjListNumOfInterfaces;
> virInterfaceObjListRemove;
>-virInterfaceObjLock;
> virInterfaceObjSetActive;
>-virInterfaceObjUnlock;
>
>
> # conf/virnetworkobj.h
>diff --git a/src/test/test_driver.c b/src/test/test_driver.c
>index 511d65f..9a1c8a5 100644
>--- a/src/test/test_driver.c
>+++ b/src/test/test_driver.c
>@@ -1027,7 +1027,7 @@ testParseInterfaces(testDriverPtr privconn,
> }
>
> virInterfaceObjSetActive(obj, true);
>- virInterfaceObjUnlock(obj);
>+ virInterfaceObjEndAPI(&obj);
> }
>
> ret = 0;
>@@ -3718,7 +3718,7 @@ testInterfaceLookupByName(virConnectPtr conn,
>
> ret = virGetInterface(conn, def->name, def->mac);
>
>- virInterfaceObjUnlock(obj);
>+ virInterfaceObjEndAPI(&obj);
> return ret;
> }
>
>@@ -3769,7 +3769,7 @@ testInterfaceIsActive(virInterfacePtr iface)
>
> ret = virInterfaceObjIsActive(obj);
>
>- virInterfaceObjUnlock(obj);
>+ virInterfaceObjEndAPI(&obj);
> return ret;
> }
>
>@@ -3881,7 +3881,7 @@ testInterfaceGetXMLDesc(virInterfacePtr iface,
>
> ret = virInterfaceDefFormat(def);
>
>- virInterfaceObjUnlock(obj);
>+ virInterfaceObjEndAPI(&obj);
> return ret;
> }
>
>@@ -3912,8 +3912,7 @@ testInterfaceDefineXML(virConnectPtr conn,
>
> cleanup:
> virInterfaceDefFree(def);
>- if (obj)
>- virInterfaceObjUnlock(obj);
>+ virInterfaceObjEndAPI(&obj);
> testDriverUnlock(privconn);
> return ret;
> }
>@@ -3956,7 +3955,7 @@ testInterfaceCreate(virInterfacePtr iface,
> ret = 0;
>
> cleanup:
>- virInterfaceObjUnlock(obj);
>+ virInterfaceObjEndAPI(&obj);
> return ret;
> }
>
>@@ -3983,7 +3982,7 @@ testInterfaceDestroy(virInterfacePtr iface,
> ret = 0;
>
> cleanup:
>- virInterfaceObjUnlock(obj);
>+ virInterfaceObjEndAPI(&obj);
> return ret;
> }
>
>--
>2.9.4
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
>
Otherwise, looks good to me.
--
IBM Systems
Linux on z Systems & Virtualization Development
------------------------------------------------------------------------
IBM Deutschland
Schönaicher Str. 220
71032 Böblingen
Phone: +49 7031 16 1819
E-Mail: bwalk at de.ibm.com
------------------------------------------------------------------------
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 896 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170530/1c29ac97/attachment-0001.sig>
More information about the libvir-list
mailing list