Re: [libvirt] [PATCH 13/20] test: Convert testDriver to virObjectLockable

On 03/08/2018 07:20 AM, Marc Hartmayer wrote: > The test driver state (@testDriver) uses it's own reference counting > and locking implementation. Instead of doing that, convert @testDriver > into a virObjectLockable and use the provided functionalities. > > Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com&gt; > Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; > --- > src/test/test_driver.c | 207 ++++++++++++++++++++++--------------------------- > 1 file changed, 94 insertions(+), 113 deletions(-) > > diff --git a/src/test/test_driver.c b/src/test/test_driver.c > index 3b55453efe00..f1dd11867143 100644 > --- a/src/test/test_driver.c > +++ b/src/test/test_driver.c > @@ -94,7 +94,7 @@ typedef struct _testAuth testAuth; > typedef struct _testAuth *testAuthPtr; > > struct _testDriver { > - virMutex lock; > + virObjectLockable parent; > > virNodeInfo nodeInfo; > virInterfaceObjListPtr ifaces; > @@ -127,9 +127,22 @@ typedef struct _testDriver testDriver; > typedef testDriver *testDriverPtr; > > static testDriverPtr defaultPrivconn; Oh and of course I see the pain associated with changing the name (and perhaps initializing to NULL just to be painfully obvious). > -static int defaultConnections; > static virMutex defaultLock = VIR_MUTEX_INITIALIZER; > [...] > @@ -1433,24 +1427,11 @@ testConnectAuthenticate(virConnectPtr conn, > static void > testDriverCloseInternal(testDriverPtr driver) > { > - bool dflt = false; > - > - if (driver == defaultPrivconn) { > - dflt = true; > - virMutexLock(&defaultLock); > - if (--defaultConnections) { > - virMutexUnlock(&defaultLock); > - return; > - } > - } > - > - testDriverLock(driver); > - testDriverFree(driver); > - > - if (dflt) { > + virMutexLock(&defaultLock); > + bool disposed = !virObjectUnref(driver); I know it builds, but it's preferable to not intermingle defs inside code, e.g. change to: bool disposed = false; virMutexLock(&defaultLock); disposed = !virObjectUnref(driver); Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; John (another one of those things I can do)

> + if (disposed && driver == defaultPrivconn) > defaultPrivconn = NULL; > - virMutexUnlock(&defaultLock); > - } > + virMutexUnlock(&defaultLock); > } > > [...]