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(a)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
> >
>