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

[Paolo Bonzini]

Il 26/03/2012 18:01, Anthony Liguori ha scritto:
> On 03/26/2012 10:59 AM, Kevin Wolf wrote:
>> Am 26.03.2012 17:38, schrieb Anthony Liguori:
>>> You can add parameters to Errors in a fully compatible fashion, so
>>> just add an
>>> filename parameter to PermissionDenied.  Problem solved.
>>
>> So your error types will end up accumulating optional parameters for all
>> contexts in which they can occur?
>>
>> In the long run, PermissionDenied would have an optional file name (for
>> raw), optional host name, port, sheepdog volume ID (for sheepdog),
>> optional source and destination block devices (for blkmirror), remote
>> host and port, local address and port (for UDP chardevs)...
> 
> I don't see the generalization.

If you use an extremely generic error such as PermissionDenied, you have
no idea whether it happened in the guts of the implementation (internal
error in QEMU or management), or it is due to a comparatively trivial
OS-level error.

If you use PermissionDenied for internal errors such as QOM property
accesses, and a more high-level error such as OpenFileFailed, you
clearly separate the two cases.  Having the same error for something
internal and something related to user configuration sucks.

>>
>> I could go on forever. Does this really make sense?
> 
> What's the alternative proposal?  If you just add errno to
> OPEN_FILE_FAILED, then you end up with errors for
> SHEEP_DOG_ATTACH_VOLUME_FAILED, NBD_CONNECT_FAILED, etc.
> 
> The proper way to design an error is to make it a verb, and have
> parameters that correspond to the direct object and/or indirect object. 
> The subject is implied by the command itself.
> 
> So in the case of block_snapshot_sync, the failure is:
> 
> The block_snapshot_sync command failed due to insufficient permission
> when creating foo.img.
> 
> So the error is "PERMISSION_DENIED", with the data "filename=foo.img"

No, the error is OPEN_FILE_FAILED, and the data is
"errno=QEMU_ERRNO_EACCES,<blah>" where <blah> is some kind of union that
would be the same type you pass to a hypothetical QMP command
blockdev_add.  Let's define our own enum so that errno values can be
mapped to a platform-independent value.

Paolo