Kevin Wolf <kw...@redhat.com> writes: > Am 02.06.2011 20:09, schrieb Luiz Capitulino: >> On Thu, 02 Jun 2011 13:00:04 -0500 >> Anthony Liguori <anth...@codemonkey.ws> wrote: >> >>> On 06/02/2011 12:57 PM, Luiz Capitulino wrote: >>>> On Wed, 01 Jun 2011 16:35:03 -0500 >>>> Anthony Liguori<anth...@codemonkey.ws> 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 } > > Splitting up enums into a number of booleans looks like a bad idea to > me. It makes things more verbose than they should be, and even worse, it > implies that more than one field could be true. > > If this new schema thing doesn't support proper enums, that's something > that should be changed. > >>> >>> 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. > > And I want to add that it's a PITA to handle bug report when the only > message you get from qemu is "something went wrong". Sorry, that's not > useful at all. I want to see the real error reason (and at least for > debugging this means, I want to see the errno value/string).
And I want it straight, not wrapped in a pile of pseudo-abstractions that make me go to the source code to figure out how to unwrap them. [...]