[PATCH 1/2] qemu: agent: sync once if qemu has serial port event

Michal Privoznik mprivozn at redhat.com
Wed Mar 11 09:38:21 UTC 2020


On 3/5/20 3:47 PM, Nikolay Shirokovskiy wrote:
> Sync was introduced in [1] to check for ga presence. This
> check is racy but in the era before serial events are available
> there was not better solution I guess.

Can you elaborate more on the raciness? There should never be more than 
one thread talking on the agent monitor.

> 
> In case we have the events the sync function is different. It allows us
> to flush stateless ga channel from remnants of previous communications.
> But we need to do it only once. Until we get timeout on issued command
> channel state is ok.
> 
> [1] qemu_agent: Issue guest-sync prior to every command
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
> ---
>   src/qemu/qemu_agent.c        | 13 ++++++++++++-
>   src/qemu/qemu_agent.h        |  3 ++-
>   src/qemu/qemu_process.c      |  3 ++-
>   tests/qemumonitortestutils.c |  3 ++-
>   4 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index cd25ef6cd3..2de53efb7a 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -104,6 +104,8 @@ struct _qemuAgent {
>       int watch;
>   
>       bool running;
> +    bool singleSync;
> +    bool inSync;
>   

I wonder if we can do this with just inSync and not have singleSync. I 
mean, it looks like at all places where singleSync is used we have 
access to qemuCaps so it should be as easy as:

qemuAgentOpen();
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VSERPORT_CHANGE)) {
   if *qemuAgentGuestSync(mon) < 0)
     goto error;
    mon->inSync = true;


and then qemuagentGuestSync() would be a NOP if inSync is set. But it 
would also not set it. But maybe I'm pushing it too far, it's just that 
I'm confused by the name 'singleSync'. Probably 'hasSingleSync' would be 
better? The boolean reflect whether qemu has the VSERPORT_CHANGE 
capability and thus libvirt knows when GA connects/disconnects. 
'singleSync' just doesn't ring that bell at first.

Michal




More information about the libvir-list mailing list