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.