Re: [libvirt] [PATCH 02/10] qemu: agent: dont set error after agent cleanup
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);
}