[libvirt] [PATCH 01/12] qemu: qemuDomainPMSuspendForDuration: Check availability of agent

[Erik Skultety]

On Fri, Sep 11, 2020 at 01:39:32PM +0200, Ján Tomko wrote:
> On a Friday in 2020, Erik Skultety wrote:
> > On Fri, Sep 11, 2020 at 03:06:08PM +0800, Lin Ma wrote:
> > > It requires a guest agent configured and running in the domain's guest
> > > OS, So check qemu agent during qemuDomainPMSuspendForDuration().
> > >
> > > Signed-off-by: Lin Ma <lma at suse.de>
> > > ---
> >
> > qemuDomainPMSuspendAgent later in that same function already checks it, we
> > don't need to check it twice.
> >
>
> It only does so after calling qemuDomainQueryWakeupSuspendSupport, which
> requires grabbing a MODIFY job and executing a QMP command.

Why does a query grab a MODIFY job anyway?

>
> I think doing this check upfront is reasonable. We will error out as
> soon as we know it, just for the price for two extra duplicit lines.

True, it will save a fraction of time, I guess I could live with that. More
importantly though, this patch uncovered a bug in the code where we report a
successful suspend even when the agent is not connected and we logged an error
accordingly.

Erik

>
> Jano
>
> > NACK
> >
> > >  src/qemu/qemu_driver.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > > index 2e46931c71..bd287f259e 100644
> > > --- a/src/qemu/qemu_driver.c
> > > +++ b/src/qemu/qemu_driver.c
> > > @@ -16820,6 +16820,9 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
> > >      if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0)
> > >          goto cleanup;
> > >
> > > +    if (!qemuDomainAgentAvailable(vm, true))
> > > +        goto cleanup;
> > > +
> > >      /*
> > >       * The case we want to handle here is when QEMU has the API (i.e.
> > >       * QEMU_CAPS_QUERY_CURRENT_MACHINE is set). Otherwise, do not interfere
> > > --
> > > 2.26.0
> > >
> >