[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 9/9] interface: Convert virInterfaceObj to use virObjectLockable



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

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]