Re: [Qemu-devel] Ignoring errno makes QMP errors suck

[Kevin Wolf <kwolf@redhat.com>]

Am 26.03.2012 16:54, schrieb Luiz Capitulino:
> On Mon, 26 Mar 2012 16:47:51 +0200
> Kevin Wolf <kwolf@redhat.com> wrote:
> 
>> Am 26.03.2012 16:33, schrieb Luiz Capitulino:
>>> On Mon, 26 Mar 2012 16:04:26 +0200
>>> Kevin Wolf <kwolf@redhat.com> wrote:
>>>
>>>>>> Does the patch that you mentioned add a generic way for adding an
>>>>>> (converted) errno to QMP errors? Or does it split up existing errors
>>>>>> into more and finer grained errors?
>>>>>
>>>>> The latter. The QMP errors have to be added manually. But it's just a matter
>>>>> of time to get the most used errnos added.
>>>>
>>>> Your PermissionDenied example doesn't really do this. It is a generic
>>>> error message that may occur in multiple contexts, right? So in one
>>>> context you may need a file name as additional information, in another
>>>> context permission for something completely different may be missing
>>>> (especially if you include EPERM in the same error).
>>>
>>> Yes, but it's not a severe problem. Not yet. Because most of the time the
>>> error context can be inferred from the command's context. Say, you fail to
>>> create a file where only one file could be created.
>>
>> So how do you like the 'transaction' command? :-)
> 
> That's the only exception I think. If this is hurting you, then we have two
> options: think of a way to add optional parameters to errors right now or
> add new QERR_s macros that accept the filename argument (note that we wouldn't
> add a new error, just a macro that accepts a new argument).

It isn't hurting anyone yet, but if we don't design it in, I'm pretty
sure that it will hurt us badly at some point.

>> But even if you could infer the context, the possible error details may
>> still differ in the possible contexts. With -blockdev this will become
>> even more fun, because even for bdrv_open you may or may not have a
>> filename. Maybe you rather have a hostname, port and sheepdog volume ID.
>> Or directory name, FAT type and stuff.
> 
> Right, and the solution for this is really extending the error infra to
> accept optional parameters. Although I don't know how to support different
> error messages.

Does this really solve the problem? Wouldn't you end up with one Error
object that has optional parameters for the sheepdog volume name,
network backend, port number of the tcp chardev, etc.? Would this be any
better than just having a QDict with arbitrary content?

This is why I believe that having a specific error is much better, there
are just too many types of PermissionDenied or NotFound.

Or maybe we need inheritance. Wait, I didn't say that...

Kevin