Re: [libvirt] [PATCH v2] qemu_agent: Ignore expected EOFs

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?