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

[Michael Roth]

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.

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.

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.

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...

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

Thoughts?

> 
> Thanks.
> 
>  qemu-ga.c            |   17 +------
>  qga/commands-posix.c |  136 
> ++++++++++++++++++++++----------------------------
>  2 files changed, 60 insertions(+), 93 deletions(-)
>