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

[Paolo Bonzini]

Il 20/06/2012 19:48, Luiz Capitulino ha scritto:
> 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

PropertyValueNotFound, PropertyValueNotPowerOf2,
PropertyValueOutOfRange, AmbiguousPath, BadBusForDevice, BusNotFound,
CommandNotFound, DuplicateId

> 
>   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

I agree with Daniel that this is going a bit too far, in particular for
the last example.  Many of these are actually ParameterErrors, and we
can lump all of these in a small number of meaningful classes.  Keeping
in mind Markus's "check failure-check transient-report", my proposal is:

* ParameterError is "the" non-transient caused by user error or a bug in
management, hopefully the former: the ones you gave above, but also
PropertyValueNotFound, PropertyValueNotPowerOf2,
PropertyValueOutOfRange, AmbiguousPath, BadBusForDevice, BusNotFound,
CommandNotFound, DuplicateId.  Perhaps PropertyValueInUse too.

* FeatureNotSupportedError is "the" non-transient error caused by user
or admin configuration (FeatureDisabled, Unsupported, CommandDisabled,
MigrationNotSupported, NotSupported, VirtFSFeatureBlocksMigration).

* SystemError could be transient or not, but is kept separate because it
is pretty much the only case in which a rich error is useful (the
richness will for now be limited to errno, perhaps in the future a file
name or socket address)

* InvalidStateError is generally caused by the interaction with other
commands, could be fixed by sending some commands and retrying:
DeviceEncrypted, MigrationActive, MigrationExpected, NotSupported,
PropertyValueInUse, ResetRequired.

* JSONParseError should be a separate error, also because it may be very
difficult to recover.

There are some cases where I'm a bit doubtful about how to proceed, but
for these we can keep the current rich errors.  One example is
InvalidPassword.  In some cases, we could use events to remove the need
for rich errors.  For example, DeviceEncrypted could be changed to a
KEY_REQUESTED event that is sent early for all devices, rather than at
"cont" time.

Paolo