Re: [Qemu-devel] [RFC 0/2]: qemu-ga: drop automatic reaper

[Luiz Capitulino]

On Thu, 19 Apr 2012 13:57:48 -0500
Michael Roth <address@hidden> wrote:

> On Thu, Apr 19, 2012 at 02:36:53PM -0300, Luiz Capitulino wrote:
> > Michael,
> > 
> > I'm going to revive this topic one more time. I was working on v2 of my 
> > fixes
> > to the suspend race bugs and really thought that the real problem is that 
> > the
> > code shouldn't be that complex.
> > 
> > Basically, this series drops the automatic reaper and adds waitpid() calls &
> > related logic to the suspend and shutdown functions.
> > 
> > My objective with this RFC is to understand why this can't be done.
> 
> Hi Luiz,
> 
> CC'ing Michael as some of these suggestions might affect his work on
> libvirt integration.

I forgot to CC him and Eric.

> Thanks for taking the time to implement what this would look like
> this. I think this is a sensible approach in general, the main issue
> for me is the specific commands currently impacted by such a change:
> suspend and shutdown.
> 
> While we do document that these commands may not return a response,
> the new behavior actually introduces a pathological assurance that
> they *won't* return a response:
> shutdown/pm-suspend/echo mem >/sys/power/state never return, or don't
> return till the guest wakes up.

Yes, but that can be viewed as an implementation detail because this is
also possible with the current code (ie. if the suspend process manages
to suspend the guest before qemu-ga is able to emit a response).

> So *every* suspend/shutdown command will result in a timeout or
> indefinite hang to the user. Honestly I just find the behavior
> completely unexpected/unintuitive and would prefer to reduce it to
> being a corner case. As the code stands currently it's actually
> extremely rare that a response won't be returned before the commands
> are executed.

I don't think we can assume this is a corner case. It all depends on how
the two processes are scheduled. On an idle VM it's more likely that qemu-ga
will emit the response first because the suspend process does more I/O, but
that really depends on the scheduler/system load. This is also true for the
shutdown command.

As we have discussed with Anthony on irc months ago, the client can't expect
a response from this command and should reset the connection before sending
new commands when the VM resumes.

If this turns out to be a problem for libvirt, I'd say it's a libvirt bug.

> So that's why we're jumping through hoops atm. It's more a
> philosophical reason than a technical one.
> 
> But if we were to do things as you proposed and document things as
> "this command will *only* show a response on error", I guess maybe I
> can live with that... in the case of shutdown older clients will
> start seeing timeouts where they previously expected a "nothing"
> response. The nothing response never entailed success or failure so
> this shouldn't break anything that wasn't already broken however...

Yes, and again, depending on the VM load it's possible that the VM vanishes
before qemu-ga emits its response.

> But, I think if we tell users we'll *only* send response on error,
> we should do our part to *not* send the responses, rather than relying
> on them having implemented the reset mechanism to throw them away after
> guest wake-up. What we could do is allow a command to set a flag, after
> it reaps it's child (in the case of suspend this would be after
> wake-up, for shutdown it'd basically be a no-op, but worth adding
> for readability sake), to have qemu-ga not send a response. We'd
> implement it similarly to how we did ga_set_response_delimited().

Fine with me. Stating that "do not wait for an OK response, because none
will be sent" sounds clearer than "an OK response may, or may not be
emitted. Or it may be emitted when the VM resumes".