[libvirt] [PATCH] qemu_agent: Ignore expected EOFs
Peter Krempa
pkrempa at redhat.com
Tue Jan 22 10:49:05 UTC 2013
On 01/22/13 10:35, 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.
> ---
> src/qemu/qemu_process.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 2f08215..b212752 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -133,8 +133,15 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent,
> virObjectLock(vm);
>
> priv = vm->privateData;
> - if (priv->agent == agent &&
> - !virObjectUnref(priv->agent))
Yes, this code isn't necessary here as qemuProcessStop frees the agent.
> +
> + if (priv->beingDestroyed) {
> + VIR_DEBUG("Domain is being destroyed, agent EOF is expected");
> + virObjectUnlock(vm);
> + qemuDriverUnlock(driver);
> + return;
> + }
> +
> + if (priv->agent == agent)
> priv->agent = NULL;
>
> virObjectUnlock(vm);
>
Hm, I think this patch is not complete. You should check if the agent
wasn't freed previously in qemuProcessHandleMonitorEOF (in
qemuProcessStop called there) while this function was waiting for the
lock as it doesn't set the beingDestroyed flag. In that case you'd free
the agent twice.
Peter
More information about the libvir-list
mailing list