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.
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list