On Fri, Apr 20, 2012 at 12:26:10PM -0500, Michael Roth wrote: > On Fri, Apr 20, 2012 at 10:36:38AM -0300, Luiz Capitulino wrote: > > On Fri, 20 Apr 2012 14:07:16 +0200 > > Michal Privoznik <mpriv...@redhat.com> wrote: > > > > > >> 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". > > > > > > > > > Just to make this clear: this "report-only-error" behavior concerns only > > > guest-suspend-* and guest-shutdown commands, right? Because otherwise, > > > if we enable such behavior for all commands (e.g. fsfreeze) I think we > > > are entering the world of pain. > > > > Exactly, this would only concern the suspend and shutdown commands. > > > > > From user POV there is a huge difference between those 2 sets of > > > commands (suspend/shutdown on one side, the others on the other side): > > > - the first emits an event on qemu monitor, so libvirt can catch that > > > and confirm suspend/shutdown has succeeded > > > > Oops, this is a different subject but there's a problem here. Events are > > just > > hints, they shouldn't be your definitive source of information. > > > > For shutdown and suspend-disk, I think that the best indication that > > the command has succeeded is that the VM will successfully exit. We could > > also have a special exit status code for suspend-to-disk, because the > > command could run in parallel with the user powering off the VM and libvirt > > wouldn't know that (and would think the VM is suspended, while it's really > > powered-off). > > Do we have a way of differentiating a guest shutting down as a result of > shutdown vs. suspend-to-disk? AFAIK they both have the same ending state. If > so I'm not sure we can say anything other than "the machine may or may not > resume when you start it up, depending on whether or not the suspend-to-disk > completed before the machine shut down". > > So for both shutdown and suspend-to-disk, i think the best we can do it > treat qemu exit as an indicator of success (from the client perspective, > at least). > > And if we can do that, shutdown and suspend-to-disk are pretty > straighforward: we either get an error response, or a timeout. Errors
for the remaining, non-success cases, I mean. process exit being the single success case. > are easy to report, timeouts: you just keep issuing the reset sequence > and attempting to re-establish a connection. If the machine is hung this > can continue indefinitely, which is fine if your reset sequence also > has a timeout and allows periodic execution of libvirt to it can do > whatever it normally does if a guest is hung. > Althrough there are also halt and reboot, which have a different success case... reboot I think will just always been either an error, or a timeout, and since timeout induces the reset sequence loop we should eventually re-establish connection with qemu-ga and be fine. So that's good. Halt I think just falls into the hung guest category... > > > > For suspend-ram and suspend-hybrid, we're missing a 'suspended' RunState. > > The event serve as a good hint and you can use it, but if it's lost for some > > reason (eg. libvirt crashes before it's received) then libvirt can learn the > > VM state by issuing query-status. > > Is it pretty trivial to plumb QEVENT_SUSPEND/QEVENT_WAKEUP to a > RunState? If so, checking query-status should be similar to checking for > qemu process exit. If you don't see a transition after a certain > threshold, you transition back into qemu-ga reset sequence loop, and > treat the machine the same way libvirt would treat any hung guest. > > The one downside to query-status vs. an event is that there's a race > window where the suspend succeeds but gets woken up before we see it in > a suspended state, and event would work around this, but there may as > you said be instances where we miss it. > > > > > Now, going back to the original subject. I have to admit that I'm not sure > > what's the best way to go here. > > > > I'll try to recapitulate (for myself and for those that may be confused) > > I'll be > > verbose a bit. > > > > We have two qemu-ga commands that are special: guest-shutdown and the > > guest-suspend > > family. They are special because they shut down the VM or suspend its > > execution > > (meaning that the world of qemu-ga is gone or gets completely frozen). > > > > Today, shutdown is an asynchronous operation: qemu-ga gets the shutdown > > process > > started and returns to accept new commands. For qemu-ga clients, this > > implies: > > > > 1. errors in the shutdown operation are not reported back > > 2. qemu-ga doesn't block > > > > For qemu-ga this implies having a way to automatically reap terminated > > children > > processes. > > > > The guest-suspend commands do the same when executing the suspend operation, > > but before they do that they need to query the VM for suspend support and > > this is done by executing pm-is-supported (if available). This fact > > shouldn't > > be visible to qemu-ga clients, but it has two internal implications: > > > > 1. The operation is half synchronous and half asynchronous > > 2. In order to bypass the automatic process reaper in qemu-ga when > > executing > > pm-is-supported, we have to play tricks that makes the suspend code > > more complex than it should be > > > > We have two options: > > > > 1. Keep the current behavior (explained above, shutdown is async, suspend > > is half sync half async). For libvirt this means nothing changes, for > > qemu-ga this means more complex code > > > > 2. Change everything to be synchronous (this series). This essentially > > means: > > > > A. errors are going to reported back > > B. qemu-ga will block > > C. we avoid all the dirty tricks, and qemu-ga code becomes simpler > > D. In theory, this should be a compatible change due to the end of > > world > > nature of the commands involved > > > > A third possible option would be to have asynchronous support. But I'm not > > sure whether this would fit well and how complex this would be (specially > > because of fork()). > > > >
