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

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> Am 02.06.2011 20:09, schrieb 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 }
>
> 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.

[...]