On Thu, Jun 02, 2011 at 03:01:24PM -0300, Luiz Capitulino wrote: > On Thu, 02 Jun 2011 09:02:30 -0500 > Anthony Liguori <anth...@codemonkey.ws> wrote: > > > On 06/02/2011 08:24 AM, Jiri Denemark wrote: > > > On Thu, Jun 02, 2011 at 08:08:35 -0500, Anthony Liguori wrote: > > >> On 06/02/2011 04:06 AM, Daniel P. Berrange wrote: > > >>>>> B. query-stop-reason > > >>>>> -------------------- > > >>>>> > > >>>>> I also have a simple solution for item 2. The vm_stop() accepts a > > >>>>> reason > > >>>>> argument, so we could store it somewhere and return it as a string, > > >>>>> like: > > >>>>> > > >>>>> -> { "execute": "query-stop-reason" } > > >>>>> <- { "return": { "reason": "user" } } > > >>>>> > > >>>>> Valid reasons could be: "user", "debug", "shutdown", "diskfull" (hey, > > >>>>> this should be "ioerror", no?), "watchdog", "panic", "savevm", > > >>>>> "loadvm", > > >>>>> "migrate". > > >>>>> > > >>>>> Also note that we have a STOP event. It should be extended with the > > >>>>> stop reason too, for completeness. > > >>>> > > >>>> > > >>>> Can we just extend query-block? > > >>> > > >>> Primarily we want 'query-stop-reason' to tell us what caused the VM > > >>> CPUs to stop. If that reason was 'ioerror', then 'query-block' could > > >>> be used to find out which particular block device(s) caused the IO > > >>> error to occurr& get the "reason" that was in the BLOCK_IO_ERROR > > >>> event. > > >> > > >> My concern is that we're over abstracting here. We're not going to add > > >> additional stop reasons in the future. > > >> > > >> Maybe just add an 'io-error': True to query-state. > > > > > > Sure, adding a new field to query-state response would work as well. And > > > it > > > seems like a good idea to me since one already needs to call query-status > > > to > > > check if CPUs are stopped or not so it makes sense to incorporate the > > > additional information there as well. And if you want to be safe for the > > > future, the new field doesn't have to be boolean 'io-error' but it can be > > > the > > > string 'reason' which Luiz suggested above. > > > > > > String enumerations are a Bad Thing. It's impossible to figure out what > > strings are valid and it lacks type safety. > > > > Adding more booleans provides better type safety, and when we move to > > QAPI with a queryable schema, provides a way to figure out exactly what > > combinations are supported by QEMU. > > To summarize: > > 1. Add a 'io-error' field to query-status (which is only present if > field 'running' is false)
This isn't really enough. There are many reasons why a VM may have transitioned to the paused state, of which IO Error is merely one. The query-status needs to be able to report what the reason for the transitioning to the paused state is. > 2. Extend query-block to contain error information associated with the > device. This is interesting, because this information will be available > even if the error didn't cause the VM to stop Apps will only look for this data, if the query-status stop reason indicates that it was stopped due to an I/O error. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|