[libvirt] [PATCH 1/2] avoid vm to be deleted if qemuConnectMonitor failed

[Daniel P. Berrange]

On Tue, Jan 25, 2011 at 02:43:43PM +0800, Wen Congyang wrote:
> The reason of libvirtd cores dump is that:
> We add vm->refs when we alloc the memory, and decrease it 
> in the function qemuHandleMonitorEOF() in other thread.
> 
> We add vm->refs in the function qemuConnectMonitor() and
> decrease it when the vm is inactive.
> 
> The libvirtd will block in the function qemuMonitorSetCapabilities()
> because the vm is stopped by signal SIGSTOP. Now the vm->refs is 2.
> 
> Then we kill the vm by signal SIGKILL. The function
> qemuMonitorSetCapabilities() failed, and then we will decrease vm->refs
> in the function qemuMonitorClose().
> In another thread, mon->fd is broken and the function
> qemuHandleMonitorEOF() is called. 
> 
> If qemuHandleMonitorEOF() decreases vm->refs before qemuConnectMonitor()
> returns, vm->refs will be decrease to 0 and the memory is freed.
> 
> We will call qemudShutdownVMDaemon() as qemuConnectMonitor() failed.
> The memory has been freed, so qemudShutdownVMDaemon() is too dangerous.
> 
> We will reference NULL pointer in the function virDomainConfVMNWFilterTeardown():
> =============
> void
> virDomainConfVMNWFilterTeardown(virDomainObjPtr vm) {
>     int i;
> 
>     if (nwfilterDriver != NULL) {
>         for (i = 0; i < vm->def->nnets; i++)
>             virDomainConfNWFilterTeardown(vm->def->nets[i]);
>     }
> }
> ============
> vm->def->nnets is not 0 but vm->def->nets is NULL(We don't set vm->def->nnets
> to 0 when we free vm).
> 
> We should add an extra reference of vm to avoid vm to be deleted if
> qemuConnectMonitor() failed.
> 
> Signed-off-by: Wen Congyang <wency at cn.fujitsu.com>

I'm not convinced this is required, if we apply the change I
mention in your other patch, to remove the call to qemuMOnitorClose()
from qemuConnectMonitor(). I think that mistaken call to
qemuMonitorClose() in qemuConnectMonitor is what is breaking
the ref counting everywhere else.

Daniel