On Fri, 2006-09-22 at 21:31 +0100, Daniel P. Berrange wrote:
> Currently APIs in the libxc library have a very limited means for reporting
> errors. They basically return -1 / NULL on error and set errno. Unfortunately
> since most of the errors which can occur in libxc are not related to system
> calls failing, the information available via errno is essentially useless.
> Indeed libxc simply sets errno to -EINVAL in most cases.
>
> The irritating thing is that libxc *does* actually know exactly what went
> wrong in all these cases & if you compile with -DDEBUG will report this
> information on STDERR. This is not much use for applications using libxc
> since they can hardly trap & parse STDERR to find out what went wrong!
>
> As an illustration of why better error handling is needed, consider the
> frequently encountered problem of trying to create a non-PAE guest on a
> PAE hypervisor:
>
> # xm create demo
> Using config file "demo".
> Error: (22, 'Invalid argument')
>
> This makes it essentially impossible to debug any errors with domain
> creation, especially if -DDEBUG was not defined in your build of libxc
I agree; this is a serious problem that I suspect we've all run into at
least once.
> So, I've prototyped a simple error reporting mechanism for libxc. The idea
> is we first define an enum for the broad classes of errors which can occur.
> As proof of concept I've defined one generic error code, and a special one
> for invalid kernels
>
> typedef enum {
> XC_ERROR_NONE = 0,
> XC_INTERNAL_ERROR = 1,
> XC_INVALID_KERNEL = 2,
> } xc_error_code;
>
> Next, I created an internal function for setting the error code, and
> providing an associated descritive message
>
> void xc_set_error(int code, const char *fmt, ...);
>
> And simply re-defined the existing ERROR & PERROR macros to use this
> function passing the generic XC_INTERNAL_ERROR
>
> #define ERROR(_m, _a...) xc_set_error(XC_INTERNAL_ERROR, _m, ## _a)
>
> #define PERROR(_m, _a...) xc_set_error(XC_INTERNAL_ERROR, "%s (%d = %s)",
> _m, errno, strerror(errno))
>
> In the various places which deal with validating the guest kernel I
> changed calls to ERROR to call xc_set_error(XC_INVALID_KERNEL,...)
> instead. We can define further error codes over time to reduce use
> fo the genreic XC_INTERNAL_ERROR as desired.
>
> In the public API defined a way to either fetch the last error, or
> register a callback for receving errors:
>
> int xc_get_last_error_code(void);
> const char *xc_get_last_error_message(void);
> void xc_clear_last_error(void);
> const char *xc_error_code_to_desc(int code);
>
> typedef void (*xc_error_handler)(int code, const char *msg);
> void xc_default_error_handler(int code, const char *msg);
> xc_error_handler xc_set_error_handler(xc_error_handler handler);
Isn't the whole idea of "get_last_error" racy by design? I guess errno
has the same problem, but I'm not quite clear how it's solved there (a
magic array of errnos that is somehow sized to include all threads?)
> Any way there's some more cleanup to do & some error messages to improve,
> and switch more errors to use an explicit error code rather than the
> generic XC_INTERNAL_ERROR, but I thought I'd post the patch for review
> before going too much further.
Thanks for sending this out! It's really helpful to see the design
early.
--
Hollis Blanchard
IBM Linux Technology Center
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home