Now that we have a bit more control, let's convert our object into
a lockable object and let that magic handle the create and lock/unlock.
This commit also introduces virInterfaceObjEndAPI in order to handle the
lock unlock and object unref in one call for consumers returning a NULL
obj upon return. This removes the need for virInterfaceObj{Lock|Unlock}
external API's.
Signed-off-by: John Ferlan <jferlan@redhat.com>
---
po/POTFILES.in | 1 -
src/conf/virinterfaceobj.c | 105 ++++++++++++++++++++++++---------------------
src/conf/virinterfaceobj.h | 9 ++--
src/libvirt_private.syms | 3 +-
src/test/test_driver.c | 15 +++----
5 files changed, 68 insertions(+), 65 deletions(-)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 142b4d3..923d647 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -44,7 +44,6 @@ src/conf/storage_adapter_conf.c
src/conf/storage_conf.c
src/conf/virchrdev.c
src/conf/virdomainobjlist.c
-src/conf/virinterfaceobj.c
src/conf/virnetworkobj.c
src/conf/virnodedeviceobj.c
src/conf/virnwfilterobj.c
diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index 1e3f25c..51c3c82 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -33,7 +33,7 @@
VIR_LOG_INIT("conf.virinterfaceobj");
struct _virInterfaceObj {
- virMutex lock;
+ virObjectLockable parent;
bool active; /* true if interface is active (up) */
virInterfaceDefPtr def; /* The interface definition */
@@ -46,50 +46,60 @@ struct _virInterfaceObjList {
/* virInterfaceObj manipulation */
-static virInterfaceObjPtr
-virInterfaceObjNew(void)
-{
- virInterfaceObjPtr obj;
+static virClassPtr virInterfaceObjClass;
+static void virInterfaceObjDispose(void *obj);
- if (VIR_ALLOC(obj) < 0)
- return NULL;
+static int
+virInterfaceObjOnceInit(void)
+{
+ if (!(virInterfaceObjClass = virClassNew(virClassForObjectLockable(),
+ "virInterfaceObj",
+ sizeof(virInterfaceObj),
+ virInterfaceObjDispose)))
+ return -1;
- if (virMutexInit(&obj->lock) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("cannot initialize mutex"));
- VIR_FREE(obj);
- return NULL;
- }
+ return 0;
+}
- virInterfaceObjLock(obj);
- return obj;
-}
+VIR_ONCE_GLOBAL_INIT(virInterfaceObj)
-void
-virInterfaceObjLock(virInterfaceObjPtr obj)
+static void
+virInterfaceObjDispose(void *opaque)
{
- virMutexLock(&obj->lock);
+ virInterfaceObjPtr obj = opaque;
+
+ virInterfaceDefFree(obj->def);
}
-void
-virInterfaceObjUnlock(virInterfaceObjPtr obj)
+static virInterfaceObjPtr
+virInterfaceObjNew(void)
{
- virMutexUnlock(&obj->lock);
+ virInterfaceObjPtr obj;
+
+ if (virInterfaceObjInitialize() < 0)
+ return NULL;
+
+ if (!(obj = virObjectLockableNew(virInterfaceObjClass)))
+ return NULL;
+
+ virObjectLock(obj);
+
+ return obj;
}
void
-virInterfaceObjFree(virInterfaceObjPtr obj)
+virInterfaceObjEndAPI(virInterfaceObjPtr *obj)
{
- if (!obj)
+ if (!*obj)
return;
- virInterfaceDefFree(obj->def);
- virMutexDestroy(&obj->lock);
- VIR_FREE(obj);
+ virObjectUnlock(*obj);
+ virObjectUnref(*obj);
+ *obj = NULL;
}
@@ -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]);
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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
John Ferlan <jferlan@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@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@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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 05/30/2017 08:13 AM, Bjoern Walk wrote:
> John Ferlan <jferlan@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 :)
>
It's a common nomenclature for libvirt... virDomainObjEndAPI,
virNetworkObjEndAPI, and virSecretObjEndAPI.
>> {
>> - 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.
>
Although perhaps true - it's the common way this happens for other
vir*ObjEndAPI source code. Since it's theoretically possible an Unref
could cause the object to be Disposed (if refcnt == 0), setting *obj =
NULL at least "ensures" the caller misusing the object would be a NULL
deref rather than referencing something it no longer owned.
>> }
>>
>>
>> @@ -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.
Understood, I suppose this could also have taken the
"virInterfaceObjEndAPI(&obj);" option...
Still eventually this is changing from a forward linked list removal to
a removal from a hash table.
For the sake of my local branches, I'd prefer to keep as is, although if
there's a strong desire for something different I'm not opposed to
adjusting.
Thanks for taking a look at the last two!
John
>>
>> 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@redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>>
>
> Otherwise, looks good to me.
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.