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

[Luiz Capitulino]

On Thu, 02 Jun 2011 13:00:04 -0500
Anthony Liguori <address@hidden> wrote:

> On 06/02/2011 12:57 PM, Luiz Capitulino wrote:
> > On Wed, 01 Jun 2011 16:35:03 -0500
> > Anthony Liguori<address@hidden>  wrote:
> >
> >> On 06/01/2011 04:12 PM, Luiz Capitulino wrote:
> >>> Hi there,
> >>>
> >>> There are people who want to use QMP for thin provisioning. That's, the 
> >>> VM is
> >>> started with a small storage and when a no space error is triggered, more 
> >>> space
> >>> is allocated and the VM is put to run again.
> >>>
> >>> QMP has two limitations that prevent people from doing this today:
> >>>
> >>> 1. The BLOCK_IO_ERROR doesn't contain error information
> >>>
> >>> 2. Considering we solve item 1, we still have to provide a way for clients
> >>>      to query why a VM stopped. This is needed because clients may miss 
> >>> the
> >>>      BLOCK_IO_ERROR event or may connect to the VM while it's already 
> >>> stopped
> >>>
> >>> A proposal to solve both problems follow.
> >>>
> >>> A. BLOCK_IO_ERROR information
> >>> -----------------------------
> >>>
> >>> We already have discussed this a lot, but didn't reach a consensus. My 
> >>> solution
> >>> is quite simple: to add a stringfied errno name to the BLOCK_IO_ERROR 
> >>> event,
> >>> for example (see the "reason" key):
> >>>
> >>> { "event": "BLOCK_IO_ERROR",
> >>>      "data": { "device": "ide0-hd1",
> >>>                "operation": "write",
> >>>                "action": "stop",
> >>>                "reason": "enospc", }
> >>
> >> you can call the reason whatever you want, but don't call it stringfied
> >> errno name :-)
> >>
> >> In fact, just make reason "no space".
> >
> > You mean, we should do:
> >
> >    "reason": "no space"
> >
> > Or that we should make it a boolean, like:
> >
> >   "no space": true
> 
> 
> Do we need reason in BLOCK_IO_ERROR if query-block returns this information?

True, no.

> > I'm ok with either way. But in case you meant the second one, I guess
> > we should make "reason" a dictionary so that we can group related
> > information when we extend the field, for example:
> >
> >   "reason": { "no space": false, "no permission": true }
> 
> Why would we ever have "no permission"?

It's an I/O error. I have a report from a developer who was getting
the BLOCK_IO_ERROR event and had to debug qemu to know the error cause,
it turned out to be no permission.

> Part of my argument for not having reason is I don't think we actually 
> need to be this generic.  I think we're over abstracting.

I'm quite sure we'll want to add new errors reasons in the near future.