[Qemu-devel] Re: Two QMP events issues

[Luiz Capitulino]

On Mon, 8 Feb 2010 14:12:20 +0000
"Daniel P. Berrange" <address@hidden> 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.