[libvirt] [PATCHv2 1/3] util: unlock closeCallbacks if get callbacks for connect fail

[John Ferlan]

On 01/18/2017 06:34 PM, John Ferlan wrote:
> 
> 
> On 01/10/2017 01:23 AM, Wang King wrote:
>> Avoid return with the closeCallbacks locked when get callbacks list for connect fail.
>>
>> Signed-off-by: Wang King <king.wang at huawei.com>
>> ---
>>  src/util/virclosecallbacks.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
> 
> ACK,
> 
> John
> 

I ended up pushing this change (a few days ago) and then realizing
almost immediately that the !list option was to Lock not Unlock - so I
fixed that...  I guess the power of my original suggestion to just have
the Unlock on the !list outweighed the reality of what was written the
patch and I just assumed.  In any case, that part is resolved.

As for patches 2 & 3, I've proposed something slightly different with
attribution to you, see:

http://www.redhat.com/archives/libvir-list/2017-January/msg01025.html

I decided that using your patch 3 concept of taking the extra ref would
be wise, although perhaps unnecessary since the callers each would
essentially return NULL when the vm object was removed from the list, so
as long as we removed the reference before the call the result would be
the same. Still for a better look and feel with respect to consistency,
using EndAPI is the better solution IMO.

John


>> diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c
>> index 891a92b..1fa9596 100644
>> --- a/src/util/virclosecallbacks.c
>> +++ b/src/util/virclosecallbacks.c
>> @@ -331,8 +331,10 @@ virCloseCallbacksRun(virCloseCallbacksPtr closeCallbacks,
>>  
>>      virObjectLock(closeCallbacks);
>>      list = virCloseCallbacksGetForConn(closeCallbacks, conn);
>> -    if (!list)
>> +    if (!list) {
>> +        virObjectLock(closeCallbacks);
>>          return;
>> +    }
>>  
>>      for (i = 0; i < list->nentries; i++) {
>>          char uuidstr[VIR_UUID_STRING_BUFLEN];
>>
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>