On Mon, 8 Feb 2010 14:12:20 +0000 "Daniel P. Berrange" <berra...@redhat.com> wrote:
> On Mon, Feb 08, 2010 at 11:41:45AM -0200, Luiz Capitulino wrote: > > > > Hi there, > > > > I have two not so related QMP events issues two discuss, but I will talk > > about > > them in the same email to avoid starting two threads. > > > > The first problem is wrt the STOP event. Right now it's only emitted if > > it's > > triggered through qemu_system_vmstop_request(), which afaik will only be > > called if CONFIG_IOTHREAD is enabled (nonsense, yes). > > > > The best fix I can think of is to move the STOP event down to do_vm_stop(). > > We could even have a 'reason' data member with the string representation of > > the EXCP_ macros. Looks like this is the right thing do to. > > > > There's a problem, though. Migration and block subsystems also do > > vm_stop(0). > > The former's reason seems to be 'stop to be loaded' and the latter is 'can't > > continue' on disk errors. Note that the block subsystem already has its own > > event for disk errors. > > > > So, my solution is to not generate the STOP event on vm_stop(0). If any > > vm_stop(0) user (eg. migration) wants to generate events they should create > > the appropriate EXCP_ macro for that. > > > > Does this look good? > > > > The second problem is about the watchdog device. I have been asked to > > add events for the watchdog's device actions (see > > hw/watchdog.c:watchdog_perform_action()). > > > > Issue is: most of those events directly map to QEMU's events already > > generated by QMP, such as RESET, SHUTDOWN, POWEROFF etc. > > > > We have two solutions: > > > > 1. Introduce watchdog's own events. This is easy to do, but will > > generate two QMP events for most actions. Eg. the watchdog's WDT_RESET > > action will generate a QMP event for WDT_RESET and will generate > > another RESET event when this action takes place in QEMU > > > > 2. Add a 'source' data member to all events requested via the > > qemu_system_* functions, so that we can have a 'wachtdog' source and > > only one event is triggered. This will require a more complex change > > and maybe some hacks will be needed (eg. for vm_stop()) > > > > Opinions? > > For further backgrou, the key end goal here is that in a QMP client, upon > receipt of the 'RESET' event, we need to reliably & immediately determine > why it occurred. eg, triggered by watchdog, or by guest OS request. There > are actually 3 possible sequences > > - WATCHDOG + action=reset, followed by RESET. Assuming no intervening > event can occurr, the client can merely record 'WATCHDOG' and interpret > it when it gets the immediately following 'RESET' event I don't think we can guarantee that, as there a number of events that can happen between the two (eg. VNC events, disk errors etc). > - RESET, followed by WATCHDOG + action=reset. The client doesn't know > the reason for the RESET and can't wait arbitrarily for WATCHDOG since > there might never be one arriving. This is possible :( > - RESET + source=watchdog. Client directly sees the reason > > The second scenario is the one I'd like us to avoid at all costs, since it > will require the client to introduce arbitrary delays in processing events > to determine cause. The first is slightly inconvenient, but doable if we > can assume no intervening events will occur, between WATCHDOG and the > RESET events. The last is obviously simplest for the clients. > > This question is also pretty relevant for Luiz's previous posting of disk > block I/O errors, since one of those actions can result in a PAUSE event Not exactly. The first part my original email describes the low-level part of this problem. First, the block I/O error event may not stop the VM, so I think even if we implement the third scenario, we should keep the block's event separated. Second, the block layer uses vm_stop(0) to stop the VM, according to the description in this email I plan not to generate the STOP event when vm_stop()'s argument is 0.