On 06/20/2012 02:40 PM, Luiz Capitulino wrote:
On Wed, 20 Jun 2012 13:46:08 -0500
Anthony Liguori<anth...@codemonkey.ws>  wrote:

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.

Would 'domain' be one of the classes I suggested above?

No. We shouldn't attempt to design new error classes. Just let it evolve naturally.

I'm not sure this is better because it suggests that all classes we have today
are still valid.

Yes, they are still valid. We cannot and should not change any of the error behavior we have today.

My main goal is to simplify,

My main goal is to get rid of all the printf() droppings we have scattered through the code base. There is absolutely not benefit in touching the current users of Error. They work fine. We need to focus our attention on cleaning up the crap that we have left.

Also, maybe it's just how I'm interpreting it, but GenericError reminds of
UndefinedError in the sense that we'd prefer commands to use more specific
errors instead.

Using strings mean that errors are basically useless from a programmatic perspective. So yeah, it's just like using UndefinedError. Except we may have a few additional kinds of "UndefinedErrors" by having domains.

I'm fine with keeping the current code, but I think this proposal overly
simplifies something that we're already overdoing.

We have something that works--why should we change it in any way? There's no maintenance burden here.

We need to focus on the parts of the code that don't work--all of the random users of printf/error_report.

Regards,

Anthony Liguori

Reply via email to