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

Re: [libvirt] [PATCH 2/2] avoid closing monitor twice



At 01/26/2011 12:46 AM, Eric Blake Write:
> On 01/24/2011 11:57 PM, Wen Congyang wrote:
>> +    if (mon->closed) {
>> +        /* We have closed monitor in other thread. */
>> +        qemuMonitorUnlock(mon);
>> +        return;
>> +    }
>> +
>> +    mon->closed = true;
>> +
>>      if (mon->fd >= 0) {
> 
> Why can't mon->fd >= 0 be the flag of whether close has previously been
> attempted?  After all, once the handle gets removed, VIR_FORCE_CLOSE
> sets mon->fd to -1.

We may call qemuMonitorClose() in the function qemuMonitorOpen() to do
the cleanup when it failed. So we still need to close the monitor if
mon->fd is -1.

> 
>>          if (mon->watch)
>>              virEventRemoveHandle(mon->watch);
> 
> Making qemuMonitorClose() do a short-circuit no-op if we already detect
> that another thread has requested close worries me that we might leak
> the resource (because we are bypassing the reference counter).  Is there
> instead somewhere that we should be temporarily increasing the reference
> counter?

If we open the monitor successfully, we hold 2 refs: one for open, and another
one for watch. So I think that that we only close monitor once may not leak the
resource.

> 
> This patch may be correct as-is, but I'm not sure of it yet.
> 


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