On 10/04/2016 02:56 AM, Nikolay Shirokovskiy wrote:
If there is an error event after eof event then after agent is cleaned up on eof error flag will be set back and remains set after next domain start up making agent unavailable. Thus let's check before set this flag on error event. --- src/qemu/qemu_process.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a581f96..3637f4b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -180,7 +180,8 @@ qemuProcessHandleAgentError(qemuAgentPtr agent ATTRIBUTE_UNUSED,
priv = vm->privateData;
- priv->agentError = true; + if (priv->agent) + priv->agentError = true;
A NULL priv->agent happens for the following reasons: 1. qemuDomainObjExitAgent - when the last reference is removed... I would hope we're not falling into this path for what you've seen... 2. processSerialChangedEvent - after a qemuAgentClose for VSERPORT callback and agentError is set to false... (so that seems to be happening properly) 3. qemuProcessHandleAgentEOF (discussed in patch 1) - where I don't believe you should be clearing agentError 4. qemuConnectAgent has a failure after qemuAgentOpen creates the 'agent', but before sets priv->agent = agent. 5. qemuProcessStop - stops a started agent *and* clears agentError flag (so that seems to be happening properly). w/r/t #4... there are windows in qemuConnectAgent where some error after qemuAgentOpen *succeeds* could trigger this function to be called before priv->agent is filled in. Thus by only clearing it when priv->agent is set, we could miss some error that occurred. While the "next" call could also fail and eventually set the error, I'm not convinced that only clearing when priv->agent is set is right. Long way of saying NAK on this one. John
virObjectUnlock(vm); }