Re: [PATCH 02/10] qemu: agent: dont set error after agent cleanup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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);
>  }
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]