Re: [Qemu-devel] [RFC] Fixing the error failure

[Anthony Liguori <anthony@codemonkey.ws>]

On 06/20/2012 12:48 PM, Luiz Capitulino wrote:
> Yet another thread fork.
>
> After talking with Daniel and Markus about QMP errors (which is not just about
> QMP, as this affects QEMU as whole), I've put together the proposal below.
>
> I'll discuss three points. First, the error format and classes. Second, the
> internal API and third compatibility.
>
> Don't be afraid about the length of this email, only the first section is long
> but it mostly contains error classes listings.
>
> 1. Error format and classes
>
> We can keep the same error format we have today, which is:
>
>   { "error": { "class": json-string,
>                "data": json-object,
>                "desc": json-string }, "id": json-value }
>
>    where 'data', 'desc' and 'id' are optional fields.
>
> However, we'd change how we use 'desc' and our error classes. 'desc' would
> become a string which is filled by a printf-like function (see section 2) and
> we'd replace all error classes we have today by the following ones:
>
>    o ParameterError: any error which involves a bad parameter. Replaces
>      InvalidParameter, InvalidParameterCombination, InvalidParameterType,
>      InvalidParameterValue, MissingParameter
>
>    o SystemError: syscall or library errors. Replaces BufferOverrun,
>      IOError, OpenFileFailed, PermissionDenied, TooManyFiles,
>      SockConnectInprogress, SockConnectFailed, SockListenFailed,
>      SockBindFailed, SockCreateFailed.
>
>      This error can include an optional 'os-error' field in the 'data'
>      member (see section 2).
>
>    o QEMUError: errors that are internal to QEMU, like AmbiguousPath,
>      BadBusForDevice, BusNoHotplug, BusNotFound, CommandDisabled,
>      CommandNotFound, DuplicateId, FeatureDisabled, JSONParseError,
>      JSONParsing, KVMMissingCap, MigrationActive, MigrationNotSupported,
>      MigrationExpected, NoBusForDevice, NotSupported, PropertyValueBad,
>      PropertyValueInUse, PropertyValueNotFound, PropertyValueNotPowerOf2,
>      PropertyValueOutOfRange, ResetRequired, SetPasswdFailed, Unsupported,
>      VirtFSFeatureBlocksMigration, VNCServerFailed
>
>    o UndefinedError: the same it's today, undefined :)
>
> Now, there are two important points to be observed:
>
>   - We check for DeviceEncrypted in hmp.c and fetch its parameters. This
>     probably indicates that we might want to create specialized classes
>     when necessary
>
>   - I don't know where to put all the DeviceFoo errors, but they probably fit
>     in QEMUError or a new class like DeviceError
>
> 2. Internal API
>
> This is very straightforward:
>
>   o error_set_deprecated();
>
>     Current error_set(), case we keep it for compatibility (see section 3).
>
>   o error_set(ErrorClass err_class, const char *fmt, ...);
>
>     Main function, sets the error classes and allow for a free human-readable
>     error message.
>
>   o error_set_sys_fmt(int err_no, const char *fmt, ...);
>   o error_set_sys(int err_no);
>
>     Helpers for setting SystemError errors. error_set_sys() gets the 'desc'
>     string from strerror().

Um, why not just do:

#define GENERIC_ERROR "{'class': 'GenericError', 'data': { 'domain': %s, 'msg': %s}"

And then just use:

error_set(errp, GENERIC_ERROR, SOME_DOMAIN, "This operation failed!");

If you want to make a little wrapper that does:

static void error_set_generic(Error **errp, const char *domain, const char *msg, 
...);

That's fine too.

But let's not overdo this and let's absolutely not change existing code! 
There's simply no need to go through a convert-everything project here.

Regards,

Anthony Liguori