On 23.01.2013 14:25, Peter Krempa wrote: > On 01/22/13 15:11, Michal Privoznik wrote: >> https://bugzilla.redhat.com/show_bug.cgi?id=892079 >> >> One of my previous patches (f2a4e5f176c408) tried to fix crashing >> libvirtd on domain detroy. However, we need to copy pattern from >> qemuProcessHandleMonitorEOF() instead of decrementing reference >> counter. The rationale for this is, if qemu process is dying due >> to domain being destroyed, we obtain EOF on both the monitor and >> agent sockets. However, if the exit is expected, qemuProcessStop >> is called, which cleans both agent and monitor sockets up. We >> want qemuAgentClose() to be called iff the EOF is not expected, >> so we don't leak an FD and memory. >> --- >> >> diff to v1: >> -handle race with qemuProcessHandleMonitorEOF correctly >> >> src/qemu/qemu_process.c | 20 ++++++++++++++++++-- >> 1 file changed, 18 insertions(+), 2 deletions(-) >> >> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c >> index 2f08215..013b2ca 100644 >> --- a/src/qemu/qemu_process.c >> +++ b/src/qemu/qemu_process.c >> @@ -133,14 +133,30 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent, >> virObjectLock(vm); >> >> priv = vm->privateData; >> - if (priv->agent == agent && >> - !virObjectUnref(priv->agent)) >> + >> + if (!priv->agent) { >> + VIR_DEBUG("Agent freed already"); >> + goto unlock; >> + } > > You shouls add to the commit message why this condition is needed. > >> + >> + if (priv->beingDestroyed) { >> + VIR_DEBUG("Domain is being destroyed, agent EOF is expected"); >> + goto unlock; >> + } >> + >> + if (priv->agent == agent) >> priv->agent = NULL; > > Is there a possibility for the agent to be different from the saved one > here? Actually no. It's a leftover from previous implementation. I have removed the condition, extended the commit message and pushed. Thanks! Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list