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
