At 01/26/2011 06:49 PM, Daniel P. Berrange Write: > On Wed, Jan 26, 2011 at 09:46:52AM +0800, Wen Congyang wrote: >> At 01/26/2011 01:02 AM, Daniel P. Berrange Write: >>> On Tue, Jan 25, 2011 at 02:57:34PM +0800, Wen Congyang wrote: >>>> When we kill the qemu, the function qemuMonitorSetCapabilities() >>>> failed and then we close monitor. >>>> >>>> In another thread, mon->fd is broken and the function >>>> qemuHandleMonitorEOF() is called. The function qemuHandleMonitorEOF() calls >>>> qemudShutdownVMDaemon() to shutdown vm. The monitor will be >>>> closed in the function qemudShutdownVMDaemon(). >>>> >>>> The monitor close twice and the reference is decreased to 0 unexpectedly. >>>> The memory will be freed when reference is decreased to 0. >>>> >>>> We will remove the watch of mon->fd when the monitor is closed. This >>>> request will be done in the function qemuMonitorUnwatch() in the qemuloop >>>> thread. In the function qemuMonitorUnwatch(), we will lock monitor, but >>>> the lock is destroyed and we will block here, >>>> >>>> In the main thread, we may add some watch or timeout, and will be blocked >>>> because the lock of eventLoop is hold by qemuLoop thread. >>>> >>>> We should close monitor only once. >>> >>> I think the problem actually lies in the qemuConnectMonitor() >>> call. This method calls qemuMonitorSetCapabilities() and >>> if that fails it calls qemuMonitorClose(). >>> >>> The caller of qemuConnectMonitor() will see the error >>> code and also try to kill the QEMU process, by calling >>> qemuShutdownVMDaemon(), which calls qemuMonitorClose() >>> again. >>> >>> So I think we need to remove the call to qemuMonitorClose() >>> from qemuConnectMonitor() and just let the calls cleanup >>> up via normal VM shutdown procedure. >> >> The function qemudWaitForMonitor() calls qemuConnectMonitor(), >> but it does not shutdown VM if qemuConnectMonitor() failed. > > You need to look further up the call chain. If the call > to qemuMonitorSetCapabilities() fails, qemuConnectMOnitor() > returns -1. If qemuConnectMonitor() returns -1, then > qemuWaitForMonitor() also returns -1. If qemuWaitForMonitor > returns -1, then qemudStartVMDaemon will call qemudShutdownVMDaemon > which kills the guest and closes the monitor. Yes, you are right. Another question: I think we should check whether the vm has been closed before calling qemudShutdownVMDaemon() in the function qemudStartVMDaemon() as the vm may be shutdown in the function qemuHandleMonitorEOF() in another thread. > > So, by having qemuConnectMonitor() close the monitor when > SetCapabilities() fails, AFAICT, we get a double-close. > > Regards, > Daniel > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list