Re: [Qemu-devel] QMP: RFC: I/O error info & query-stop-reason

[Anthony Liguori]

On 06/02/2011 01:01 PM, Luiz Capitulino wrote:
> On Thu, 02 Jun 2011 09:02:30 -0500
> Anthony Liguori<address@hidden>  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)

It may or may not be present.  Lack of presence does not tell you anything.

It is only true when running is false AND the guest was stopped because 
of an io error.

>   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

Well we need at least some way to indicate that a block device is in a 
failed state.  For instance, if you have two block device, but you miss 
the IO_ERROR event, you need to figure out which of the two devices is 
giving errors.

But I was thinking of something that had the semantics of, last_iop_failed.

Regards,

Anthony Liguori

> Seems good enough to me, comments?