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

[Marc Hartmayer]

On Thu, Mar 15, 2018 at 04:05 PM +0100, John Ferlan <jferlan at redhat.com> wrote:
> 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 at linux.vnet.ibm.com>
>> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
>> ---
>>  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 at redhat.com>
>
> John
>
> (another one of those things I can do)

Thanks!

>
>> +    if (disposed && driver == defaultPrivconn)
>>          defaultPrivconn = NULL;
>> -        virMutexUnlock(&defaultLock);
>> -    }
>> +    virMutexUnlock(&defaultLock);
>>  }
>>  
>>  
> [...]
>
-- 
Beste Grüße / Kind regards
   Marc Hartmayer

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