* RE: Error reporting capabilities for libxc
@ 2006-09-26 12:58 Ian Pratt
2006-09-26 13:20 ` Daniel P. Berrange
2006-10-23 18:00 ` Daniel P. Berrange
0 siblings, 2 replies; 25+ messages in thread
From: Ian Pratt @ 2006-09-26 12:58 UTC (permalink / raw)
To: Keir Fraser, Stephen C. Tweedie, Daniel P. Berrange; +Cc: xen-devel
> On 26/9/06 12:45, "Stephen C. Tweedie" <sct@redhat.com> wrote:
>
> >> 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.
> > ...
> >
> >> Any way, the upshot of all this work:
> >> # xm create error
> >> Using config file "error".
> >> Error: [2, 'Kernel ELF architecture 3 does not match Xen
> >> architecture 62']
> >
> > IMHO this is sorely needed. Any comments from XenSource people?
>
> I'd agree something like this is necessary in the 3.0.4
> timeframe. I'll let one of the guys more acquainted with xend
> comment in detail. There's also the question of how this will
> integrate with the proposed 'XenAPI'.
Yep, definitely needs to happen. However, I think we should at least
discuss alternative potentially less clunky implementation methods.
Perhaps we should use a thread local errno and error string variables?
Ian
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: Error reporting capabilities for libxc 2006-09-26 12:58 Error reporting capabilities for libxc Ian Pratt @ 2006-09-26 13:20 ` Daniel P. Berrange 2006-10-23 18:00 ` Daniel P. Berrange 1 sibling, 0 replies; 25+ messages in thread From: Daniel P. Berrange @ 2006-09-26 13:20 UTC (permalink / raw) To: Ian Pratt; +Cc: xen-devel On Tue, Sep 26, 2006 at 01:58:11PM +0100, Ian Pratt wrote: > > On 26/9/06 12:45, "Stephen C. Tweedie" <sct@redhat.com> wrote: > > > > >> 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. > > > ... > > > > > >> Any way, the upshot of all this work: > > >> # xm create error > > >> Using config file "error". > > >> Error: [2, 'Kernel ELF architecture 3 does not match Xen > > >> architecture 62'] > > > > > > IMHO this is sorely needed. Any comments from XenSource people? > > > > I'd agree something like this is necessary in the 3.0.4 > > timeframe. I'll let one of the guys more acquainted with xend > > comment in detail. There's also the question of how this will > > integrate with the proposed 'XenAPI'. > > Yep, definitely needs to happen. However, I think we should at least > discuss alternative potentially less clunky implementation methods. > Perhaps we should use a thread local errno and error string variables? Yes using thread locals instead of static vars is a good idea & should be easy to implement behind the scenes. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Error reporting capabilities for libxc 2006-09-26 12:58 Error reporting capabilities for libxc Ian Pratt 2006-09-26 13:20 ` Daniel P. Berrange @ 2006-10-23 18:00 ` Daniel P. Berrange 2006-10-23 19:17 ` Anthony Liguori ` (2 more replies) 1 sibling, 3 replies; 25+ messages in thread From: Daniel P. Berrange @ 2006-10-23 18:00 UTC (permalink / raw) To: Ian Pratt; +Cc: xen-devel [-- Attachment #1: Type: text/plain, Size: 2039 bytes --] On Tue, Sep 26, 2006 at 01:58:11PM +0100, Ian Pratt wrote: > > On 26/9/06 12:45, "Stephen C. Tweedie" <sct@redhat.com> wrote: > > > > >> 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. > > > ... > > > > > >> Any way, the upshot of all this work: > > >> # xm create error > > >> Using config file "error". > > >> Error: [2, 'Kernel ELF architecture 3 does not match Xen > > >> architecture 62'] > > > > > > IMHO this is sorely needed. Any comments from XenSource people? > > > > I'd agree something like this is necessary in the 3.0.4 > > timeframe. I'll let one of the guys more acquainted with xend > > comment in detail. There's also the question of how this will > > integrate with the proposed 'XenAPI'. > > Yep, definitely needs to happen. However, I think we should at least > discuss alternative potentially less clunky implementation methods. > Perhaps we should use a thread local errno and error string variables? Attached is an update to the original patch which annotates the static variables with __thread so that they are setup per-thread. It also adds more informative error reporting for bad kernels. The only issue is that __thread is a GCC specific extension, so I'm not sure this will work for the Solaris folks ? http://gcc.gnu.org/onlinedocs/gcc/Thread_002dLocal.html The other option is to use the POSIX APIs pthread_getspecific & pthread_setspecific. It'll make the code alot more cumbersome, but would certainly be portable. I'm certainly willing to do this though if need be... Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| [-- Attachment #2: xen-libxc-errors9.patch --] [-- Type: text/plain, Size: 25331 bytes --] diff -ruN xen-3.0.3_0-src.orig/tools/libxc/xc_hvm_build.c xen-3.0.3_0-src.new/tools/libxc/xc_hvm_build.c --- xen-3.0.3_0-src.orig/tools/libxc/xc_hvm_build.c 2006-10-15 08:22:03.000000000 -0400 +++ xen-3.0.3_0-src.new/tools/libxc/xc_hvm_build.c 2006-10-23 11:43:17.000000000 -0400 @@ -460,7 +460,6 @@ ctxt, domctl.u.getdomaininfo.shared_info_frame, vcpus, pae, acpi, apic, store_evtchn, store_mfn) < 0) { - ERROR("Error constructing guest OS"); goto error_out; } @@ -529,26 +528,30 @@ if ( !IS_ELF(*ehdr) ) { - ERROR("Kernel image does not have an ELF header."); + xc_set_error(XC_INVALID_KERNEL, + "Kernel image does not have an ELF header."); return -EINVAL; } if ( (ehdr->e_phoff + (ehdr->e_phnum * ehdr->e_phentsize)) > elfsize ) { - ERROR("ELF program headers extend beyond end of image."); + xc_set_error(XC_INVALID_KERNEL, + "ELF program headers extend beyond end of image."); return -EINVAL; } if ( (ehdr->e_shoff + (ehdr->e_shnum * ehdr->e_shentsize)) > elfsize ) { - ERROR("ELF section headers extend beyond end of image."); + xc_set_error(XC_INVALID_KERNEL, + "ELF section headers extend beyond end of image."); return -EINVAL; } /* Find the section-header strings table. */ if ( ehdr->e_shstrndx == SHN_UNDEF ) { - ERROR("ELF image has no section-header strings table (shstrtab)."); + xc_set_error(XC_INVALID_KERNEL, + "ELF image has no section-header strings table (shstrtab)."); return -EINVAL; } shdr = (Elf32_Shdr *)(elfbase + ehdr->e_shoff + @@ -570,7 +573,8 @@ (ehdr->e_entry < kernstart) || (ehdr->e_entry > kernend) ) { - ERROR("Malformed ELF image."); + xc_set_error(XC_INVALID_KERNEL, + "Malformed ELF image."); return -EINVAL; } diff -ruN xen-3.0.3_0-src.orig/tools/libxc/xc_linux_build.c xen-3.0.3_0-src.new/tools/libxc/xc_linux_build.c --- xen-3.0.3_0-src.orig/tools/libxc/xc_linux_build.c 2006-10-15 08:22:03.000000000 -0400 +++ xen-3.0.3_0-src.new/tools/libxc/xc_linux_build.c 2006-10-23 13:30:13.000000000 -0400 @@ -121,7 +121,7 @@ if ( probe_elf(image, image_size, load_funcs) && probe_bin(image, image_size, load_funcs) ) { - ERROR( "Unrecognized image format" ); + xc_set_error(XC_INVALID_KERNEL, "Not a valid ELF or raw kernel image"); return -EINVAL; } @@ -611,17 +611,20 @@ xen_capabilities_info_t xen_caps = ""; if (xc_version(xc_handle, XENVER_capabilities, &xen_caps) != 0) { - ERROR("Cannot determine host capabilities."); + xc_set_error(XC_INVALID_KERNEL, + "Cannot determine host capabilities."); return 0; } if (strstr(xen_caps, "xen-3.0-x86_32p")) { if (dsi->pae_kernel == PAEKERN_no) { - ERROR("Non PAE-kernel on PAE host."); + xc_set_error(XC_INVALID_KERNEL, + "Non PAE-kernel on PAE host."); return 0; } } else if (dsi->pae_kernel != PAEKERN_no) { - ERROR("PAE-kernel on non-PAE host."); + xc_set_error(XC_INVALID_KERNEL, + "PAE-kernel on non-PAE host."); return 0; } @@ -1167,7 +1170,6 @@ console_evtchn, console_mfn, features_bitmap) < 0 ) { - ERROR("Error constructing guest OS"); goto error_out; } diff -ruN xen-3.0.3_0-src.orig/tools/libxc/xc_load_elf.c xen-3.0.3_0-src.new/tools/libxc/xc_load_elf.c --- xen-3.0.3_0-src.orig/tools/libxc/xc_load_elf.c 2006-10-15 08:22:03.000000000 -0400 +++ xen-3.0.3_0-src.new/tools/libxc/xc_load_elf.c 2006-10-23 13:36:06.000000000 -0400 @@ -29,20 +29,46 @@ */ #if defined(__ia64__) #define ELFCLASS ELFCLASS64 +#define ELFCLASS_DESC "64-bit" + #define ELFDATA ELFDATA2LSB +#define ELFDATA_DESC "Little-Endian" + #define ELFMACHINE EM_IA_64 +#define ELFMACHINE_DESC "ia64" + + #elif defined(__i386__) #define ELFCLASS ELFCLASS32 +#define ELFCLASS_DESC "32-bit" + #define ELFDATA ELFDATA2LSB +#define ELFDATA_DESC "Little-Endian" + #define ELFMACHINE EM_386 +#define ELFMACHINE_DESC "i386" + + #elif defined(__x86_64__) #define ELFCLASS ELFCLASS64 +#define ELFCLASS_DESC "64-bit" + #define ELFDATA ELFDATA2LSB +#define ELFDATA_DESC "Little-Endian" + #define ELFMACHINE EM_X86_64 +#define ELFMACHINE_DESC "x86_64" + + #elif defined(__powerpc__) #define ELFCLASS ELFCLASS64 +#define ELFCLASS_DESC "64-bit" + #define ELFDATA ELFDATA2MSB +#define ELFDATA_DESC "Big-Endian" + #define ELFMACHINE EM_PPC64 +#define ELFMACHINE_DESC "ppc64" #endif int probe_elf(const char *image, @@ -231,7 +257,8 @@ *defined = 1; return *(uint64_t*)ELFNOTE_DESC(note); default: - ERROR("elfnotes: unknown data size %#x for numeric type note %#x\n", + xc_set_error(XC_INVALID_KERNEL, + "elfnotes: unknown data size %#x for numeric type note %#x\n", note->descsz, type); return 0; } @@ -250,35 +277,59 @@ if ( !IS_ELF(*ehdr) ) { - ERROR("Kernel image does not have an ELF header."); + xc_set_error(XC_INVALID_KERNEL, + "Kernel image does not have an ELF header."); return -EINVAL; } - if ( (ehdr->e_ident[EI_CLASS] != ELFCLASS) || - (ehdr->e_machine != ELFMACHINE) || - (ehdr->e_ident[EI_DATA] != ELFDATA) || - (ehdr->e_type != ET_EXEC) ) + if (ehdr->e_machine != ELFMACHINE) + { + xc_set_error(XC_INVALID_KERNEL, + "Kernel ELF architecture '%d' does not match Xen architecture '%d' (%s)", + ehdr->e_machine, ELFMACHINE, ELFMACHINE_DESC); + return -EINVAL; + } + if (ehdr->e_ident[EI_CLASS] != ELFCLASS) + { + xc_set_error(XC_INVALID_KERNEL, + "Kernel ELF wordsize '%d' does not match Xen wordsize '%d' (%s)", + ehdr->e_ident[EI_CLASS], ELFCLASS, ELFCLASS_DESC); + return -EINVAL; + } + if (ehdr->e_ident[EI_DATA] != ELFDATA) + { + xc_set_error(XC_INVALID_KERNEL, + "Kernel ELF endianness '%d' does not match Xen endianness '%d' (%s)", + ehdr->e_ident[EI_DATA], ELFDATA, ELFDATA_DESC); + return -EINVAL; + } + if (ehdr->e_type != ET_EXEC) { - ERROR("Kernel not a Xen-compatible Elf image."); + xc_set_error(XC_INVALID_KERNEL, + "Kernel ELF type '%d' does not match Xen type '%d'", + ehdr->e_type, ET_EXEC); return -EINVAL; } if ( (ehdr->e_phoff + (ehdr->e_phnum*ehdr->e_phentsize)) > image_len ) { - ERROR("ELF program headers extend beyond end of image."); + xc_set_error(XC_INVALID_KERNEL, + "ELF program headers extend beyond end of image."); return -EINVAL; } if ( (ehdr->e_shoff + (ehdr->e_shnum*ehdr->e_shentsize)) > image_len ) { - ERROR("ELF section headers extend beyond end of image."); + xc_set_error(XC_INVALID_KERNEL, + "ELF section headers extend beyond end of image."); return -EINVAL; } /* Find the section-header strings table. */ if ( ehdr->e_shstrndx == SHN_UNDEF ) { - ERROR("ELF image has no section-header strings table (shstrtab)."); + xc_set_error(XC_INVALID_KERNEL, + "ELF image has no section-header strings table (shstrtab)."); return -EINVAL; } shdr = (Elf_Shdr *)(image + ehdr->e_shoff + @@ -325,22 +376,25 @@ if ( ( loader == NULL || strncmp(loader, "generic", 7) ) && ( guest_os == NULL || strncmp(guest_os, "linux", 5) ) ) { - ERROR("Will only load images built for the generic loader " - "or Linux images"); + xc_set_error(XC_INVALID_KERNEL, + "Will only load images built for the generic loader " + "or Linux images"); return -EINVAL; } if ( xen_version == NULL || strncmp(xen_version, "xen-3.0", 7) ) { - ERROR("Will only load images built for Xen v3.0"); + xc_set_error(XC_INVALID_KERNEL, + "Will only load images built for Xen v3.0"); return -EINVAL; } } else { #if defined(__x86_64__) || defined(__i386__) - ERROR("Not a Xen-ELF image: " - "No ELF notes or '__xen_guest' section found."); + xc_set_error(XC_INVALID_KERNEL, + "Not a Xen-ELF image: " + "No ELF notes or '__xen_guest' section found."); return -EINVAL; #endif } @@ -396,8 +450,9 @@ if ( elf_pa_off_defined && !virt_base_defined ) { - ERROR("Neither ELF_PADDR_OFFSET nor VIRT_BASE found in ELF " - " notes or __xen_guest section."); + xc_set_error(XC_INVALID_KERNEL, + "Neither ELF_PADDR_OFFSET nor VIRT_BASE found in ELF " + " notes or __xen_guest section."); return -EINVAL; } @@ -409,7 +464,8 @@ vaddr = phdr->p_paddr - dsi->elf_paddr_offset + dsi->v_start; if ( (vaddr + phdr->p_memsz) < vaddr ) { - ERROR("ELF program header %d is too large.", h); + xc_set_error(XC_INVALID_KERNEL, + "ELF program header %d is too large.", h); return -EINVAL; } @@ -431,7 +487,8 @@ (dsi->v_kernentry > kernend) || (dsi->v_start > kernstart) ) { - ERROR("ELF start or entries are out of bounds."); + xc_set_error(XC_INVALID_KERNEL, + "ELF start or entries are out of bounds."); return -EINVAL; } diff -ruN xen-3.0.3_0-src.orig/tools/libxc/xc_private.c xen-3.0.3_0-src.new/tools/libxc/xc_private.c --- xen-3.0.3_0-src.orig/tools/libxc/xc_private.c 2006-10-15 08:22:03.000000000 -0400 +++ xen-3.0.3_0-src.new/tools/libxc/xc_private.c 2006-10-23 13:47:54.000000000 -0400 @@ -6,6 +6,85 @@ #include <inttypes.h> #include "xc_private.h" +#include <stdarg.h> + +static __thread int last_error_code = XC_ERROR_NONE; +static __thread char *last_error_message = NULL; +#if DEBUG +static xc_error_handler error_handler = xc_default_error_handler; +#else +static xc_error_handler error_handler = NULL; +#endif + +void xc_default_error_handler(int code, const char *msg) { + const char *desc = xc_error_code_to_desc(code); + fprintf(stderr, "ERROR %s: %s\n", desc, msg); +} + +int xc_get_last_error_code(void) { + return last_error_code; +} + +const char *xc_get_last_error_message(void) { + return last_error_message; +} + +void xc_clear_last_error(void) { + last_error_code = XC_ERROR_NONE; + if (last_error_message) { + free(last_error_message); + last_error_message = NULL; + } +} + +const char *xc_error_code_to_desc(int code) { + /* Sync to members of xc_error_code enumeration in xenctrl.h */ + switch (code) { + case XC_ERROR_NONE: return "No error details"; + case XC_INTERNAL_ERROR: return "Internal error"; + case XC_INVALID_KERNEL: return "Invalid kernel"; + default: + return "Unknown error code"; + } +} + +xc_error_handler xc_set_error_handler(xc_error_handler handler) { + xc_error_handler old = error_handler; + error_handler = handler; + return old; +} + + +static void _xc_set_error(int code, const char *msg) { + xc_clear_last_error(); + last_error_code = code; + /* NB, we delibrately ignore the OOM condition when + strdup()'ing here - there's not really anything + we can - particularly if the error we're reporting + was an OOM in the first place. The caller can always + find the latter via errno */ + last_error_message = strdup(msg); +} + +#define XC_MAX_ERROR_MSG 4096 +void xc_set_error(int code, const char *fmt, ...) { + int saved_errno = errno; + char msg[XC_MAX_ERROR_MSG]; + va_list args; + + va_start(args, fmt); + vsnprintf(msg, XC_MAX_ERROR_MSG-1, fmt, args); + msg[XC_MAX_ERROR_MSG-1] = '\0'; + va_end(args); + + _xc_set_error(code, msg); + + errno = saved_errno; + + if (error_handler) + error_handler(code, msg); +} + /* NB: arr must be mlock'ed */ int xc_get_pfn_type_batch(int xc_handle, diff -ruN xen-3.0.3_0-src.orig/tools/libxc/xc_private.h xen-3.0.3_0-src.new/tools/libxc/xc_private.h --- xen-3.0.3_0-src.orig/tools/libxc/xc_private.h 2006-10-15 08:22:03.000000000 -0400 +++ xen-3.0.3_0-src.new/tools/libxc/xc_private.h 2006-10-23 11:43:17.000000000 -0400 @@ -56,25 +56,13 @@ #define PPRINTF(_f, _a...) #endif -#define ERR(_f, _a...) do { \ - DPRINTF(_f ": %d\n" , ## _a, errno); \ - fflush(stderr); } \ -while (0) - -#define ERROR(_m, _a...) \ -do { \ - int __saved_errno = errno; \ - DPRINTF("ERROR: " _m "\n" , ## _a ); \ - errno = __saved_errno; \ -} while (0) - -#define PERROR(_m, _a...) \ -do { \ - int __saved_errno = errno; \ - DPRINTF("ERROR: " _m " (%d = %s)\n" , ## _a , \ - __saved_errno, strerror(__saved_errno)); \ - errno = __saved_errno; \ -} while (0) +void xc_set_error(int code, const char *fmt, ...); + +#define ERR(_f, _a...) ERROR(_f, ## _a) + +#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)) static inline void safe_munlock(const void *addr, size_t len) { diff -ruN xen-3.0.3_0-src.orig/tools/libxc/xenctrl.h xen-3.0.3_0-src.new/tools/libxc/xenctrl.h --- xen-3.0.3_0-src.orig/tools/libxc/xenctrl.h 2006-10-15 08:22:03.000000000 -0400 +++ xen-3.0.3_0-src.new/tools/libxc/xenctrl.h 2006-10-23 11:46:31.000000000 -0400 @@ -656,4 +656,43 @@ */ int xc_evtchn_unmask(int xce_handle, evtchn_port_t port); + +typedef enum { + XC_ERROR_NONE = 0, + XC_INTERNAL_ERROR = 1, + XC_INVALID_KERNEL = 2, +} xc_error_code; + +/* + * Return the code of the last error to occurr + */ +int xc_get_last_error_code(void); + +/* + * Return the message associated with the last error to occur + */ +const char *xc_get_last_error_message(void); + +/* + * Clear the last error + */ +void xc_clear_last_error(void); + +typedef void (*xc_error_handler)(int code, const char *msg); + +/* + * The default error handler which prints to stderr + */ +void xc_default_error_handler(int code, const char *msg); + +/* + * Convert an error code into a text description + */ +const char *xc_error_code_to_desc(int code); + +/* + * Registers a callback to handle errors + */ +xc_error_handler xc_set_error_handler(xc_error_handler handler); + #endif diff -ruN xen-3.0.3_0-src.orig/tools/python/xen/lowlevel/xc/xc.c xen-3.0.3_0-src.new/tools/python/xen/lowlevel/xc/xc.c --- xen-3.0.3_0-src.orig/tools/python/xen/lowlevel/xc/xc.c 2006-10-15 08:22:03.000000000 -0400 +++ xen-3.0.3_0-src.new/tools/python/xen/lowlevel/xc/xc.c 2006-10-23 11:50:00.000000000 -0400 @@ -37,6 +37,24 @@ static PyObject *dom_op(XcObject *self, PyObject *args, int (*fn)(int, uint32_t)); +static PyObject *pyxc_error_to_exception(void) +{ + PyObject *err; + int code = xc_get_last_error_code(); + const char *desc = xc_error_code_to_desc(code); + const char *msg = xc_get_last_error_message(); + + if (msg) + err = Py_BuildValue("[iss]", code, desc, msg); + else + err = Py_BuildValue("[is]", code, desc); + + xc_clear_last_error(); + + PyErr_SetObject(xc_error, err); + + return NULL; +} static PyObject *pyxc_domain_dumpcore(XcObject *self, PyObject *args) { @@ -50,7 +68,7 @@ return NULL; if (xc_domain_dumpcore(self->xc_handle, dom, corefile) != 0) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -95,12 +113,11 @@ } if ( (ret = xc_domain_create(self->xc_handle, ssidref, handle, &dom)) < 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); return PyInt_FromLong(dom); out_exception: - errno = EINVAL; PyErr_SetFromErrno(xc_error); return NULL; } @@ -113,7 +130,7 @@ return NULL; if (xc_domain_max_vcpus(self->xc_handle, dom, max) != 0) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -158,7 +175,7 @@ } if ( xc_vcpu_setaffinity(self->xc_handle, dom, vcpu, cpumap) != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -178,7 +195,7 @@ return NULL; if ( xc_domain_setcpuweight(self->xc_handle, dom, cpuweight) != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -209,13 +226,12 @@ } if (xc_domain_sethandle(self->xc_handle, dom, handle) < 0) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; out_exception: - errno = EINVAL; PyErr_SetFromErrno(xc_error); return NULL; } @@ -245,7 +261,7 @@ if (nr_doms < 0) { free(info); - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); } list = PyList_New(nr_doms); @@ -299,10 +315,10 @@ rc = xc_vcpu_getinfo(self->xc_handle, dom, vcpu, &info); if ( rc < 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); rc = xc_vcpu_getaffinity(self->xc_handle, dom, vcpu, &cpumap); if ( rc < 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); info_dict = Py_BuildValue("{s:i,s:i,s:i,s:L,s:i}", "online", info.online, @@ -352,9 +368,7 @@ ramdisk, cmdline, features, flags, store_evtchn, &store_mfn, console_evtchn, &console_mfn) != 0 ) { - if (!errno) - errno = EINVAL; - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); } return Py_BuildValue("{s:i,s:i}", "store_mfn", store_mfn, @@ -385,7 +399,7 @@ if ( xc_hvm_build(self->xc_handle, dom, memsize, image, vcpus, pae, acpi, apic, store_evtchn, &store_mfn) != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); return Py_BuildValue("{s:i}", "store_mfn", store_mfn); } @@ -404,7 +418,7 @@ return NULL; if ( (port = xc_evtchn_alloc_unbound(self->xc_handle, dom, remote_dom)) < 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); return PyInt_FromLong(port); } @@ -425,7 +439,7 @@ ret = xc_physdev_pci_access_modify( self->xc_handle, dom, bus, dev, func, enable); if ( ret != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -447,7 +461,7 @@ ret = xc_readconsolering(self->xc_handle, &str, &count, clear); if ( ret < 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); return PyString_FromStringAndSize(str, count); } @@ -477,7 +491,7 @@ int i; if ( xc_physinfo(self->xc_handle, &info) != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); *q=0; for(i=0;i<sizeof(info.hw_cap)/4;i++) @@ -515,25 +529,25 @@ xen_version = xc_version(self->xc_handle, XENVER_version, NULL); if ( xc_version(self->xc_handle, XENVER_extraversion, &xen_extra) != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); if ( xc_version(self->xc_handle, XENVER_compile_info, &xen_cc) != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); if ( xc_version(self->xc_handle, XENVER_changeset, &xen_chgset) != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); if ( xc_version(self->xc_handle, XENVER_capabilities, &xen_caps) != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); if ( xc_version(self->xc_handle, XENVER_platform_parameters, &p_parms) != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); sprintf(str, "virt_start=0x%lx", p_parms.virt_start); xen_pagesize = xc_version(self->xc_handle, XENVER_pagesize, NULL); if (xen_pagesize < 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); return Py_BuildValue("{s:i,s:i,s:s,s:s,s:i,s:s,s:s,s:s,s:s,s:s,s:s}", "xen_major", xen_version >> 16, @@ -566,7 +580,7 @@ return NULL; if ( xc_sedf_domain_set(self->xc_handle, domid, period, slice, latency, extratime,weight) != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -583,7 +597,7 @@ if (xc_sedf_domain_get(self->xc_handle, domid, &period, &slice,&latency,&extratime,&weight)) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); return Py_BuildValue("{s:i,s:L,s:L,s:L,s:i,s:i}", "domain", domid, @@ -611,7 +625,7 @@ if ( xc_shadow_control(xc->xc_handle, dom, op, NULL, 0, NULL, 0, NULL) < 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -641,7 +655,7 @@ op = XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION; } if ( xc_shadow_control(xc->xc_handle, dom, op, NULL, 0, &mb, 0, NULL) < 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); mbarg = mb; return Py_BuildValue("i", mbarg); @@ -668,7 +682,7 @@ sdom.cap = cap; if ( xc_sched_credit_domain_set(self->xc_handle, domid, &sdom) != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -683,7 +697,7 @@ return NULL; if ( xc_sched_credit_domain_get(self->xc_handle, domid, &sdom) != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); return Py_BuildValue("{s:H,s:H}", "weight", sdom.weight, @@ -699,7 +713,7 @@ return NULL; if (xc_domain_setmaxmem(self->xc_handle, dom, maxmem_kb) != 0) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -726,7 +740,7 @@ if ( xc_domain_memory_increase_reservation(self->xc_handle, dom, nr_extents, extent_order, address_bits, NULL) ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -748,7 +762,7 @@ ret = xc_domain_ioport_permission( self->xc_handle, dom, first_port, nr_ports, allow_access); if ( ret != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -771,7 +785,7 @@ ret = xc_domain_irq_permission( xc->xc_handle, dom, pirq, allow_access); if ( ret != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -794,7 +808,7 @@ ret = xc_domain_iomem_permission( xc->xc_handle, dom, first_pfn, nr_pfns, allow_access); if ( ret != 0 ) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -834,7 +848,7 @@ return NULL; if (fn(self->xc_handle, dom) != 0) - return PyErr_SetFromErrno(xc_error); + return pyxc_error_to_exception(); Py_INCREF(zero); return zero; @@ -1157,7 +1171,7 @@ PyXc_init(XcObject *self, PyObject *args, PyObject *kwds) { if ((self->xc_handle = xc_interface_open()) == -1) { - PyErr_SetFromErrno(xc_error); + pyxc_error_to_exception(); return -1; } [-- Attachment #3: Type: text/plain, Size: 138 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Error reporting capabilities for libxc 2006-10-23 18:00 ` Daniel P. Berrange @ 2006-10-23 19:17 ` Anthony Liguori 2006-10-23 19:27 ` Daniel P. Berrange 2006-10-23 20:40 ` John Levon 2006-10-23 21:04 ` John Levon 2 siblings, 1 reply; 25+ messages in thread From: Anthony Liguori @ 2006-10-23 19:17 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: xen-devel Daniel P. Berrange wrote: > On Tue, Sep 26, 2006 at 01:58:11PM +0100, Ian Pratt wrote: >>> On 26/9/06 12:45, "Stephen C. Tweedie" <sct@redhat.com> wrote: >>> >>>>> 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. >>>> ... >>>> >>>>> Any way, the upshot of all this work: >>>>> # xm create error >>>>> Using config file "error". >>>>> Error: [2, 'Kernel ELF architecture 3 does not match Xen >>>>> architecture 62'] >>>> IMHO this is sorely needed. Any comments from XenSource people? >>> I'd agree something like this is necessary in the 3.0.4 >>> timeframe. I'll let one of the guys more acquainted with xend >>> comment in detail. There's also the question of how this will >>> integrate with the proposed 'XenAPI'. >> Yep, definitely needs to happen. However, I think we should at least >> discuss alternative potentially less clunky implementation methods. >> Perhaps we should use a thread local errno and error string variables? > > Attached is an update to the original patch which annotates the static > variables with __thread so that they are setup per-thread. It also adds > more informative error reporting for bad kernels. What versions of GCC have you tested this with? I've found in the past that __thread can have problems when using shared libraries (the exact instance was a Python binding). Regards, Anthony Liguori > The only issue is that __thread is a GCC specific extension, so I'm not > sure this will work for the Solaris folks ? > > http://gcc.gnu.org/onlinedocs/gcc/Thread_002dLocal.html > > The other option is to use the POSIX APIs pthread_getspecific & > pthread_setspecific. It'll make the code alot more cumbersome, but > would certainly be portable. I'm certainly willing to do this though > if need be... > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > > Regards, > Dan. > > > ------------------------------------------------------------------------ > > diff -ruN xen-3.0.3_0-src.orig/tools/libxc/xc_hvm_build.c xen-3.0.3_0-src.new/tools/libxc/xc_hvm_build.c > --- xen-3.0.3_0-src.orig/tools/libxc/xc_hvm_build.c 2006-10-15 08:22:03.000000000 -0400 > +++ xen-3.0.3_0-src.new/tools/libxc/xc_hvm_build.c 2006-10-23 11:43:17.000000000 -0400 > @@ -460,7 +460,6 @@ > ctxt, domctl.u.getdomaininfo.shared_info_frame, > vcpus, pae, acpi, apic, store_evtchn, store_mfn) < 0) > { > - ERROR("Error constructing guest OS"); > goto error_out; > } > > @@ -529,26 +528,30 @@ > > if ( !IS_ELF(*ehdr) ) > { > - ERROR("Kernel image does not have an ELF header."); > + xc_set_error(XC_INVALID_KERNEL, > + "Kernel image does not have an ELF header."); > return -EINVAL; > } > > if ( (ehdr->e_phoff + (ehdr->e_phnum * ehdr->e_phentsize)) > elfsize ) > { > - ERROR("ELF program headers extend beyond end of image."); > + xc_set_error(XC_INVALID_KERNEL, > + "ELF program headers extend beyond end of image."); > return -EINVAL; > } > > if ( (ehdr->e_shoff + (ehdr->e_shnum * ehdr->e_shentsize)) > elfsize ) > { > - ERROR("ELF section headers extend beyond end of image."); > + xc_set_error(XC_INVALID_KERNEL, > + "ELF section headers extend beyond end of image."); > return -EINVAL; > } > > /* Find the section-header strings table. */ > if ( ehdr->e_shstrndx == SHN_UNDEF ) > { > - ERROR("ELF image has no section-header strings table (shstrtab)."); > + xc_set_error(XC_INVALID_KERNEL, > + "ELF image has no section-header strings table (shstrtab)."); > return -EINVAL; > } > shdr = (Elf32_Shdr *)(elfbase + ehdr->e_shoff + > @@ -570,7 +573,8 @@ > (ehdr->e_entry < kernstart) || > (ehdr->e_entry > kernend) ) > { > - ERROR("Malformed ELF image."); > + xc_set_error(XC_INVALID_KERNEL, > + "Malformed ELF image."); > return -EINVAL; > } > > diff -ruN xen-3.0.3_0-src.orig/tools/libxc/xc_linux_build.c xen-3.0.3_0-src.new/tools/libxc/xc_linux_build.c > --- xen-3.0.3_0-src.orig/tools/libxc/xc_linux_build.c 2006-10-15 08:22:03.000000000 -0400 > +++ xen-3.0.3_0-src.new/tools/libxc/xc_linux_build.c 2006-10-23 13:30:13.000000000 -0400 > @@ -121,7 +121,7 @@ > if ( probe_elf(image, image_size, load_funcs) && > probe_bin(image, image_size, load_funcs) ) > { > - ERROR( "Unrecognized image format" ); > + xc_set_error(XC_INVALID_KERNEL, "Not a valid ELF or raw kernel image"); > return -EINVAL; > } > > @@ -611,17 +611,20 @@ > xen_capabilities_info_t xen_caps = ""; > > if (xc_version(xc_handle, XENVER_capabilities, &xen_caps) != 0) { > - ERROR("Cannot determine host capabilities."); > + xc_set_error(XC_INVALID_KERNEL, > + "Cannot determine host capabilities."); > return 0; > } > > if (strstr(xen_caps, "xen-3.0-x86_32p")) { > if (dsi->pae_kernel == PAEKERN_no) { > - ERROR("Non PAE-kernel on PAE host."); > + xc_set_error(XC_INVALID_KERNEL, > + "Non PAE-kernel on PAE host."); > return 0; > } > } else if (dsi->pae_kernel != PAEKERN_no) { > - ERROR("PAE-kernel on non-PAE host."); > + xc_set_error(XC_INVALID_KERNEL, > + "PAE-kernel on non-PAE host."); > return 0; > } > > @@ -1167,7 +1170,6 @@ > console_evtchn, console_mfn, > features_bitmap) < 0 ) > { > - ERROR("Error constructing guest OS"); > goto error_out; > } > > diff -ruN xen-3.0.3_0-src.orig/tools/libxc/xc_load_elf.c xen-3.0.3_0-src.new/tools/libxc/xc_load_elf.c > --- xen-3.0.3_0-src.orig/tools/libxc/xc_load_elf.c 2006-10-15 08:22:03.000000000 -0400 > +++ xen-3.0.3_0-src.new/tools/libxc/xc_load_elf.c 2006-10-23 13:36:06.000000000 -0400 > @@ -29,20 +29,46 @@ > */ > #if defined(__ia64__) > #define ELFCLASS ELFCLASS64 > +#define ELFCLASS_DESC "64-bit" > + > #define ELFDATA ELFDATA2LSB > +#define ELFDATA_DESC "Little-Endian" > + > #define ELFMACHINE EM_IA_64 > +#define ELFMACHINE_DESC "ia64" > + > + > #elif defined(__i386__) > #define ELFCLASS ELFCLASS32 > +#define ELFCLASS_DESC "32-bit" > + > #define ELFDATA ELFDATA2LSB > +#define ELFDATA_DESC "Little-Endian" > + > #define ELFMACHINE EM_386 > +#define ELFMACHINE_DESC "i386" > + > + > #elif defined(__x86_64__) > #define ELFCLASS ELFCLASS64 > +#define ELFCLASS_DESC "64-bit" > + > #define ELFDATA ELFDATA2LSB > +#define ELFDATA_DESC "Little-Endian" > + > #define ELFMACHINE EM_X86_64 > +#define ELFMACHINE_DESC "x86_64" > + > + > #elif defined(__powerpc__) > #define ELFCLASS ELFCLASS64 > +#define ELFCLASS_DESC "64-bit" > + > #define ELFDATA ELFDATA2MSB > +#define ELFDATA_DESC "Big-Endian" > + > #define ELFMACHINE EM_PPC64 > +#define ELFMACHINE_DESC "ppc64" > #endif > > int probe_elf(const char *image, > @@ -231,7 +257,8 @@ > *defined = 1; > return *(uint64_t*)ELFNOTE_DESC(note); > default: > - ERROR("elfnotes: unknown data size %#x for numeric type note %#x\n", > + xc_set_error(XC_INVALID_KERNEL, > + "elfnotes: unknown data size %#x for numeric type note %#x\n", > note->descsz, type); > return 0; > } > @@ -250,35 +277,59 @@ > > if ( !IS_ELF(*ehdr) ) > { > - ERROR("Kernel image does not have an ELF header."); > + xc_set_error(XC_INVALID_KERNEL, > + "Kernel image does not have an ELF header."); > return -EINVAL; > } > > - if ( (ehdr->e_ident[EI_CLASS] != ELFCLASS) || > - (ehdr->e_machine != ELFMACHINE) || > - (ehdr->e_ident[EI_DATA] != ELFDATA) || > - (ehdr->e_type != ET_EXEC) ) > + if (ehdr->e_machine != ELFMACHINE) > + { > + xc_set_error(XC_INVALID_KERNEL, > + "Kernel ELF architecture '%d' does not match Xen architecture '%d' (%s)", > + ehdr->e_machine, ELFMACHINE, ELFMACHINE_DESC); > + return -EINVAL; > + } > + if (ehdr->e_ident[EI_CLASS] != ELFCLASS) > + { > + xc_set_error(XC_INVALID_KERNEL, > + "Kernel ELF wordsize '%d' does not match Xen wordsize '%d' (%s)", > + ehdr->e_ident[EI_CLASS], ELFCLASS, ELFCLASS_DESC); > + return -EINVAL; > + } > + if (ehdr->e_ident[EI_DATA] != ELFDATA) > + { > + xc_set_error(XC_INVALID_KERNEL, > + "Kernel ELF endianness '%d' does not match Xen endianness '%d' (%s)", > + ehdr->e_ident[EI_DATA], ELFDATA, ELFDATA_DESC); > + return -EINVAL; > + } > + if (ehdr->e_type != ET_EXEC) > { > - ERROR("Kernel not a Xen-compatible Elf image."); > + xc_set_error(XC_INVALID_KERNEL, > + "Kernel ELF type '%d' does not match Xen type '%d'", > + ehdr->e_type, ET_EXEC); > return -EINVAL; > } > > if ( (ehdr->e_phoff + (ehdr->e_phnum*ehdr->e_phentsize)) > image_len ) > { > - ERROR("ELF program headers extend beyond end of image."); > + xc_set_error(XC_INVALID_KERNEL, > + "ELF program headers extend beyond end of image."); > return -EINVAL; > } > > if ( (ehdr->e_shoff + (ehdr->e_shnum*ehdr->e_shentsize)) > image_len ) > { > - ERROR("ELF section headers extend beyond end of image."); > + xc_set_error(XC_INVALID_KERNEL, > + "ELF section headers extend beyond end of image."); > return -EINVAL; > } > > /* Find the section-header strings table. */ > if ( ehdr->e_shstrndx == SHN_UNDEF ) > { > - ERROR("ELF image has no section-header strings table (shstrtab)."); > + xc_set_error(XC_INVALID_KERNEL, > + "ELF image has no section-header strings table (shstrtab)."); > return -EINVAL; > } > shdr = (Elf_Shdr *)(image + ehdr->e_shoff + > @@ -325,22 +376,25 @@ > if ( ( loader == NULL || strncmp(loader, "generic", 7) ) && > ( guest_os == NULL || strncmp(guest_os, "linux", 5) ) ) > { > - ERROR("Will only load images built for the generic loader " > - "or Linux images"); > + xc_set_error(XC_INVALID_KERNEL, > + "Will only load images built for the generic loader " > + "or Linux images"); > return -EINVAL; > } > > if ( xen_version == NULL || strncmp(xen_version, "xen-3.0", 7) ) > { > - ERROR("Will only load images built for Xen v3.0"); > + xc_set_error(XC_INVALID_KERNEL, > + "Will only load images built for Xen v3.0"); > return -EINVAL; > } > } > else > { > #if defined(__x86_64__) || defined(__i386__) > - ERROR("Not a Xen-ELF image: " > - "No ELF notes or '__xen_guest' section found."); > + xc_set_error(XC_INVALID_KERNEL, > + "Not a Xen-ELF image: " > + "No ELF notes or '__xen_guest' section found."); > return -EINVAL; > #endif > } > @@ -396,8 +450,9 @@ > > if ( elf_pa_off_defined && !virt_base_defined ) > { > - ERROR("Neither ELF_PADDR_OFFSET nor VIRT_BASE found in ELF " > - " notes or __xen_guest section."); > + xc_set_error(XC_INVALID_KERNEL, > + "Neither ELF_PADDR_OFFSET nor VIRT_BASE found in ELF " > + " notes or __xen_guest section."); > return -EINVAL; > } > > @@ -409,7 +464,8 @@ > vaddr = phdr->p_paddr - dsi->elf_paddr_offset + dsi->v_start; > if ( (vaddr + phdr->p_memsz) < vaddr ) > { > - ERROR("ELF program header %d is too large.", h); > + xc_set_error(XC_INVALID_KERNEL, > + "ELF program header %d is too large.", h); > return -EINVAL; > } > > @@ -431,7 +487,8 @@ > (dsi->v_kernentry > kernend) || > (dsi->v_start > kernstart) ) > { > - ERROR("ELF start or entries are out of bounds."); > + xc_set_error(XC_INVALID_KERNEL, > + "ELF start or entries are out of bounds."); > return -EINVAL; > } > > diff -ruN xen-3.0.3_0-src.orig/tools/libxc/xc_private.c xen-3.0.3_0-src.new/tools/libxc/xc_private.c > --- xen-3.0.3_0-src.orig/tools/libxc/xc_private.c 2006-10-15 08:22:03.000000000 -0400 > +++ xen-3.0.3_0-src.new/tools/libxc/xc_private.c 2006-10-23 13:47:54.000000000 -0400 > @@ -6,6 +6,85 @@ > > #include <inttypes.h> > #include "xc_private.h" > +#include <stdarg.h> > + > +static __thread int last_error_code = XC_ERROR_NONE; > +static __thread char *last_error_message = NULL; > +#if DEBUG > +static xc_error_handler error_handler = xc_default_error_handler; > +#else > +static xc_error_handler error_handler = NULL; > +#endif > + > +void xc_default_error_handler(int code, const char *msg) { > + const char *desc = xc_error_code_to_desc(code); > + fprintf(stderr, "ERROR %s: %s\n", desc, msg); > +} > + > +int xc_get_last_error_code(void) { > + return last_error_code; > +} > + > +const char *xc_get_last_error_message(void) { > + return last_error_message; > +} > + > +void xc_clear_last_error(void) { > + last_error_code = XC_ERROR_NONE; > + if (last_error_message) { > + free(last_error_message); > + last_error_message = NULL; > + } > +} > + > +const char *xc_error_code_to_desc(int code) { > + /* Sync to members of xc_error_code enumeration in xenctrl.h */ > + switch (code) { > + case XC_ERROR_NONE: return "No error details"; > + case XC_INTERNAL_ERROR: return "Internal error"; > + case XC_INVALID_KERNEL: return "Invalid kernel"; > + default: > + return "Unknown error code"; > + } > +} > + > +xc_error_handler xc_set_error_handler(xc_error_handler handler) { > + xc_error_handler old = error_handler; > + error_handler = handler; > + return old; > +} > + > + > +static void _xc_set_error(int code, const char *msg) { > + xc_clear_last_error(); > + last_error_code = code; > + /* NB, we delibrately ignore the OOM condition when > + strdup()'ing here - there's not really anything > + we can - particularly if the error we're reporting > + was an OOM in the first place. The caller can always > + find the latter via errno */ > + last_error_message = strdup(msg); > +} > + > +#define XC_MAX_ERROR_MSG 4096 > +void xc_set_error(int code, const char *fmt, ...) { > + int saved_errno = errno; > + char msg[XC_MAX_ERROR_MSG]; > + va_list args; > + > + va_start(args, fmt); > + vsnprintf(msg, XC_MAX_ERROR_MSG-1, fmt, args); > + msg[XC_MAX_ERROR_MSG-1] = '\0'; > + va_end(args); > + > + _xc_set_error(code, msg); > + > + errno = saved_errno; > + > + if (error_handler) > + error_handler(code, msg); > +} > + > > /* NB: arr must be mlock'ed */ > int xc_get_pfn_type_batch(int xc_handle, > diff -ruN xen-3.0.3_0-src.orig/tools/libxc/xc_private.h xen-3.0.3_0-src.new/tools/libxc/xc_private.h > --- xen-3.0.3_0-src.orig/tools/libxc/xc_private.h 2006-10-15 08:22:03.000000000 -0400 > +++ xen-3.0.3_0-src.new/tools/libxc/xc_private.h 2006-10-23 11:43:17.000000000 -0400 > @@ -56,25 +56,13 @@ > #define PPRINTF(_f, _a...) > #endif > > -#define ERR(_f, _a...) do { \ > - DPRINTF(_f ": %d\n" , ## _a, errno); \ > - fflush(stderr); } \ > -while (0) > - > -#define ERROR(_m, _a...) \ > -do { \ > - int __saved_errno = errno; \ > - DPRINTF("ERROR: " _m "\n" , ## _a ); \ > - errno = __saved_errno; \ > -} while (0) > - > -#define PERROR(_m, _a...) \ > -do { \ > - int __saved_errno = errno; \ > - DPRINTF("ERROR: " _m " (%d = %s)\n" , ## _a , \ > - __saved_errno, strerror(__saved_errno)); \ > - errno = __saved_errno; \ > -} while (0) > +void xc_set_error(int code, const char *fmt, ...); > + > +#define ERR(_f, _a...) ERROR(_f, ## _a) > + > +#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)) > > static inline void safe_munlock(const void *addr, size_t len) > { > diff -ruN xen-3.0.3_0-src.orig/tools/libxc/xenctrl.h xen-3.0.3_0-src.new/tools/libxc/xenctrl.h > --- xen-3.0.3_0-src.orig/tools/libxc/xenctrl.h 2006-10-15 08:22:03.000000000 -0400 > +++ xen-3.0.3_0-src.new/tools/libxc/xenctrl.h 2006-10-23 11:46:31.000000000 -0400 > @@ -656,4 +656,43 @@ > */ > int xc_evtchn_unmask(int xce_handle, evtchn_port_t port); > > + > +typedef enum { > + XC_ERROR_NONE = 0, > + XC_INTERNAL_ERROR = 1, > + XC_INVALID_KERNEL = 2, > +} xc_error_code; > + > +/* > + * Return the code of the last error to occurr > + */ > +int xc_get_last_error_code(void); > + > +/* > + * Return the message associated with the last error to occur > + */ > +const char *xc_get_last_error_message(void); > + > +/* > + * Clear the last error > + */ > +void xc_clear_last_error(void); > + > +typedef void (*xc_error_handler)(int code, const char *msg); > + > +/* > + * The default error handler which prints to stderr > + */ > +void xc_default_error_handler(int code, const char *msg); > + > +/* > + * Convert an error code into a text description > + */ > +const char *xc_error_code_to_desc(int code); > + > +/* > + * Registers a callback to handle errors > + */ > +xc_error_handler xc_set_error_handler(xc_error_handler handler); > + > #endif > diff -ruN xen-3.0.3_0-src.orig/tools/python/xen/lowlevel/xc/xc.c xen-3.0.3_0-src.new/tools/python/xen/lowlevel/xc/xc.c > --- xen-3.0.3_0-src.orig/tools/python/xen/lowlevel/xc/xc.c 2006-10-15 08:22:03.000000000 -0400 > +++ xen-3.0.3_0-src.new/tools/python/xen/lowlevel/xc/xc.c 2006-10-23 11:50:00.000000000 -0400 > @@ -37,6 +37,24 @@ > static PyObject *dom_op(XcObject *self, PyObject *args, > int (*fn)(int, uint32_t)); > > +static PyObject *pyxc_error_to_exception(void) > +{ > + PyObject *err; > + int code = xc_get_last_error_code(); > + const char *desc = xc_error_code_to_desc(code); > + const char *msg = xc_get_last_error_message(); > + > + if (msg) > + err = Py_BuildValue("[iss]", code, desc, msg); > + else > + err = Py_BuildValue("[is]", code, desc); > + > + xc_clear_last_error(); > + > + PyErr_SetObject(xc_error, err); > + > + return NULL; > +} > > static PyObject *pyxc_domain_dumpcore(XcObject *self, PyObject *args) > { > @@ -50,7 +68,7 @@ > return NULL; > > if (xc_domain_dumpcore(self->xc_handle, dom, corefile) != 0) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -95,12 +113,11 @@ > } > > if ( (ret = xc_domain_create(self->xc_handle, ssidref, handle, &dom)) < 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > return PyInt_FromLong(dom); > > out_exception: > - errno = EINVAL; > PyErr_SetFromErrno(xc_error); > return NULL; > } > @@ -113,7 +130,7 @@ > return NULL; > > if (xc_domain_max_vcpus(self->xc_handle, dom, max) != 0) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -158,7 +175,7 @@ > } > > if ( xc_vcpu_setaffinity(self->xc_handle, dom, vcpu, cpumap) != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -178,7 +195,7 @@ > return NULL; > > if ( xc_domain_setcpuweight(self->xc_handle, dom, cpuweight) != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -209,13 +226,12 @@ > } > > if (xc_domain_sethandle(self->xc_handle, dom, handle) < 0) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > > out_exception: > - errno = EINVAL; > PyErr_SetFromErrno(xc_error); > return NULL; > } > @@ -245,7 +261,7 @@ > if (nr_doms < 0) > { > free(info); > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > } > > list = PyList_New(nr_doms); > @@ -299,10 +315,10 @@ > > rc = xc_vcpu_getinfo(self->xc_handle, dom, vcpu, &info); > if ( rc < 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > rc = xc_vcpu_getaffinity(self->xc_handle, dom, vcpu, &cpumap); > if ( rc < 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > info_dict = Py_BuildValue("{s:i,s:i,s:i,s:L,s:i}", > "online", info.online, > @@ -352,9 +368,7 @@ > ramdisk, cmdline, features, flags, > store_evtchn, &store_mfn, > console_evtchn, &console_mfn) != 0 ) { > - if (!errno) > - errno = EINVAL; > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > } > return Py_BuildValue("{s:i,s:i}", > "store_mfn", store_mfn, > @@ -385,7 +399,7 @@ > > if ( xc_hvm_build(self->xc_handle, dom, memsize, image, > vcpus, pae, acpi, apic, store_evtchn, &store_mfn) != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > return Py_BuildValue("{s:i}", "store_mfn", store_mfn); > } > @@ -404,7 +418,7 @@ > return NULL; > > if ( (port = xc_evtchn_alloc_unbound(self->xc_handle, dom, remote_dom)) < 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > return PyInt_FromLong(port); > } > @@ -425,7 +439,7 @@ > ret = xc_physdev_pci_access_modify( > self->xc_handle, dom, bus, dev, func, enable); > if ( ret != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -447,7 +461,7 @@ > > ret = xc_readconsolering(self->xc_handle, &str, &count, clear); > if ( ret < 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > return PyString_FromStringAndSize(str, count); > } > @@ -477,7 +491,7 @@ > int i; > > if ( xc_physinfo(self->xc_handle, &info) != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > *q=0; > for(i=0;i<sizeof(info.hw_cap)/4;i++) > @@ -515,25 +529,25 @@ > xen_version = xc_version(self->xc_handle, XENVER_version, NULL); > > if ( xc_version(self->xc_handle, XENVER_extraversion, &xen_extra) != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > if ( xc_version(self->xc_handle, XENVER_compile_info, &xen_cc) != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > if ( xc_version(self->xc_handle, XENVER_changeset, &xen_chgset) != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > if ( xc_version(self->xc_handle, XENVER_capabilities, &xen_caps) != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > if ( xc_version(self->xc_handle, XENVER_platform_parameters, &p_parms) != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > sprintf(str, "virt_start=0x%lx", p_parms.virt_start); > > xen_pagesize = xc_version(self->xc_handle, XENVER_pagesize, NULL); > if (xen_pagesize < 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > return Py_BuildValue("{s:i,s:i,s:s,s:s,s:i,s:s,s:s,s:s,s:s,s:s,s:s}", > "xen_major", xen_version >> 16, > @@ -566,7 +580,7 @@ > return NULL; > if ( xc_sedf_domain_set(self->xc_handle, domid, period, > slice, latency, extratime,weight) != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -583,7 +597,7 @@ > > if (xc_sedf_domain_get(self->xc_handle, domid, &period, > &slice,&latency,&extratime,&weight)) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > return Py_BuildValue("{s:i,s:L,s:L,s:L,s:i,s:i}", > "domain", domid, > @@ -611,7 +625,7 @@ > > if ( xc_shadow_control(xc->xc_handle, dom, op, NULL, 0, NULL, 0, NULL) > < 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -641,7 +655,7 @@ > op = XEN_DOMCTL_SHADOW_OP_SET_ALLOCATION; > } > if ( xc_shadow_control(xc->xc_handle, dom, op, NULL, 0, &mb, 0, NULL) < 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > mbarg = mb; > return Py_BuildValue("i", mbarg); > @@ -668,7 +682,7 @@ > sdom.cap = cap; > > if ( xc_sched_credit_domain_set(self->xc_handle, domid, &sdom) != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -683,7 +697,7 @@ > return NULL; > > if ( xc_sched_credit_domain_get(self->xc_handle, domid, &sdom) != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > return Py_BuildValue("{s:H,s:H}", > "weight", sdom.weight, > @@ -699,7 +713,7 @@ > return NULL; > > if (xc_domain_setmaxmem(self->xc_handle, dom, maxmem_kb) != 0) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -726,7 +740,7 @@ > if ( xc_domain_memory_increase_reservation(self->xc_handle, dom, > nr_extents, extent_order, > address_bits, NULL) ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -748,7 +762,7 @@ > ret = xc_domain_ioport_permission( > self->xc_handle, dom, first_port, nr_ports, allow_access); > if ( ret != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -771,7 +785,7 @@ > ret = xc_domain_irq_permission( > xc->xc_handle, dom, pirq, allow_access); > if ( ret != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -794,7 +808,7 @@ > ret = xc_domain_iomem_permission( > xc->xc_handle, dom, first_pfn, nr_pfns, allow_access); > if ( ret != 0 ) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -834,7 +848,7 @@ > return NULL; > > if (fn(self->xc_handle, dom) != 0) > - return PyErr_SetFromErrno(xc_error); > + return pyxc_error_to_exception(); > > Py_INCREF(zero); > return zero; > @@ -1157,7 +1171,7 @@ > PyXc_init(XcObject *self, PyObject *args, PyObject *kwds) > { > if ((self->xc_handle = xc_interface_open()) == -1) { > - PyErr_SetFromErrno(xc_error); > + pyxc_error_to_exception(); > return -1; > } > > > > ------------------------------------------------------------------------ > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Error reporting capabilities for libxc 2006-10-23 19:17 ` Anthony Liguori @ 2006-10-23 19:27 ` Daniel P. Berrange 2006-10-23 20:04 ` Anthony Liguori 2006-10-23 20:54 ` Ian Pratt 0 siblings, 2 replies; 25+ messages in thread From: Daniel P. Berrange @ 2006-10-23 19:27 UTC (permalink / raw) To: Anthony Liguori; +Cc: xen-devel On Mon, Oct 23, 2006 at 02:17:40PM -0500, Anthony Liguori wrote: > Daniel P. Berrange wrote: > >On Tue, Sep 26, 2006 at 01:58:11PM +0100, Ian Pratt wrote: > >>>On 26/9/06 12:45, "Stephen C. Tweedie" <sct@redhat.com> wrote: > >>> > >>>>>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. > >>>>... > >>>> > >>>>>Any way, the upshot of all this work: > >>>>> # xm create error > >>>>> Using config file "error". > >>>>> Error: [2, 'Kernel ELF architecture 3 does not match Xen > >>>>>architecture 62'] > >>>>IMHO this is sorely needed. Any comments from XenSource people? > >>>I'd agree something like this is necessary in the 3.0.4 > >>>timeframe. I'll let one of the guys more acquainted with xend > >>>comment in detail. There's also the question of how this will > >>>integrate with the proposed 'XenAPI'. > >>Yep, definitely needs to happen. However, I think we should at least > >>discuss alternative potentially less clunky implementation methods. > >>Perhaps we should use a thread local errno and error string variables? > > > >Attached is an update to the original patch which annotates the static > >variables with __thread so that they are setup per-thread. It also adds > >more informative error reporting for bad kernels. > > What versions of GCC have you tested this with? I'm doing this on FC6 - the __thread annotation is the same approach used in libc for the per-thread errno variable, hence why I chose it try it initially. The version are: glibc-2.5-3 gcc-4.1.1-30 > I've found in the past that __thread can have problems when using shared > libraries (the exact instance was a Python binding). I've not explicitly tested this integrating with the python bindings and ensuring the correct per-thread operation. > >The only issue is that __thread is a GCC specific extension, so I'm not > >sure this will work for the Solaris folks ? > > > > http://gcc.gnu.org/onlinedocs/gcc/Thread_002dLocal.html > > > >The other option is to use the POSIX APIs pthread_getspecific & > >pthread_setspecific. It'll make the code alot more cumbersome, but > >would certainly be portable. I'm certainly willing to do this though > >if need be... One further issue I've thought of is that the __thread annotation does not provide any way to do cleanup when a thread exists. Fine if the per-thread variable is a simple scalar, but not so good if its a char * since I think it'll leak memory. So I think I may have no choice by to re-write with pthread_getspecific(), since that allows registration of a cleanup function to free memory. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Error reporting capabilities for libxc 2006-10-23 19:27 ` Daniel P. Berrange @ 2006-10-23 20:04 ` Anthony Liguori 2006-10-23 20:08 ` Daniel P. Berrange 2006-10-23 20:54 ` Ian Pratt 1 sibling, 1 reply; 25+ messages in thread From: Anthony Liguori @ 2006-10-23 20:04 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: xen-devel Daniel P. Berrange wrote: > I'm doing this on FC6 - the __thread annotation is the same approach > used in libc for the per-thread errno variable, hence why I chose it > try it initially. The version are: > > glibc-2.5-3 > gcc-4.1.1-30 > > >> I've found in the past that __thread can have problems when using shared >> libraries (the exact instance was a Python binding). >> > > I've not explicitly tested this integrating with the python bindings > and ensuring the correct per-thread operation. > Will the python bindings actually load? In the past, I've not been able to import a module that contains __thread variables. Regards, Anthony Liguori > One further issue I've thought of is that the __thread annotation does > not provide any way to do cleanup when a thread exists. Fine if the > per-thread variable is a simple scalar, but not so good if its a char * > since I think it'll leak memory. So I think I may have no choice by to > re-write with pthread_getspecific(), since that allows registration > of a cleanup function to free memory. > > Regards, > Dan. > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Error reporting capabilities for libxc 2006-10-23 20:04 ` Anthony Liguori @ 2006-10-23 20:08 ` Daniel P. Berrange 0 siblings, 0 replies; 25+ messages in thread From: Daniel P. Berrange @ 2006-10-23 20:08 UTC (permalink / raw) To: Anthony Liguori; +Cc: xen-devel On Mon, Oct 23, 2006 at 03:04:41PM -0500, Anthony Liguori wrote: > Daniel P. Berrange wrote: > >I'm doing this on FC6 - the __thread annotation is the same approach > >used in libc for the per-thread errno variable, hence why I chose it > >try it initially. The version are: > > > > glibc-2.5-3 > > gcc-4.1.1-30 > > > > > >>I've found in the past that __thread can have problems when using shared > >>libraries (the exact instance was a Python binding). > >> > > > >I've not explicitly tested this integrating with the python bindings > >and ensuring the correct per-thread operation. > > > > Will the python bindings actually load? In the past, I've not been able > to import a module that contains __thread variables. Yes, that worked just fine. I tested using xend/xm to validate the saner error handling behaviour for invalid kernels, eg # xm create crash Using config file "crash". Error: [2, 'Invalid kernel', "Kernel ELF type '3' does not match Xen type '2'"] # xm create crash Using config file "crash". Error: [2, 'Invalid kernel', "Kernel ELF architecture '3' does not match Xen architecture '62' (x86_64)"] # xm create crash Using config file "crash". Error: [2, 'Invalid kernel', 'Not a valid ELF or raw kernel image'] Given the notes in the GCC docs about needing support from libdl.so, libc.so & libthread.so I suspect one of the libraries in your stack was not fully compliant with the __thread specs. > >One further issue I've thought of is that the __thread annotation does > >not provide any way to do cleanup when a thread exists. Fine if the > >per-thread variable is a simple scalar, but not so good if its a char * > >since I think it'll leak memory. So I think I may have no choice by to > >re-write with pthread_getspecific(), since that allows registration > >of a cleanup function to free memory. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| ^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: Re: Error reporting capabilities for libxc 2006-10-23 19:27 ` Daniel P. Berrange 2006-10-23 20:04 ` Anthony Liguori @ 2006-10-23 20:54 ` Ian Pratt 2006-10-23 20:57 ` Daniel P. Berrange 1 sibling, 1 reply; 25+ messages in thread From: Ian Pratt @ 2006-10-23 20:54 UTC (permalink / raw) To: Daniel P. Berrange, Anthony Liguori; +Cc: xen-devel > One further issue I've thought of is that the __thread annotation does > not provide any way to do cleanup when a thread exists. Fine if the > per-thread variable is a simple scalar, but not so good if its a char * > since I think it'll leak memory. So I think I may have no choice by to > re-write with pthread_getspecific(), since that allows registration > of a cleanup function to free memory. Do we actually need pointers? I'd much rather go with __thread. Ian ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Re: Error reporting capabilities for libxc 2006-10-23 20:54 ` Ian Pratt @ 2006-10-23 20:57 ` Daniel P. Berrange 2006-10-23 21:10 ` Ian Pratt 0 siblings, 1 reply; 25+ messages in thread From: Daniel P. Berrange @ 2006-10-23 20:57 UTC (permalink / raw) To: Ian Pratt; +Cc: Anthony Liguori, xen-devel On Mon, Oct 23, 2006 at 09:54:12PM +0100, Ian Pratt wrote: > > One further issue I've thought of is that the __thread annotation does > > not provide any way to do cleanup when a thread exists. Fine if the > > per-thread variable is a simple scalar, but not so good if its a char > * > > since I think it'll leak memory. So I think I may have no choice by to > > re-write with pthread_getspecific(), since that allows registration > > of a cleanup function to free memory. > > Do we actually need pointers? I'd much rather go with __thread. Well, we need a char buffer to store the error message, since that has much more useful info than the error code alone. I could always just do a fixed 200 byte buffer, and truncate anything longer than this which would also actually remove the annoying OOM problem in copying the error message. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| ^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: Re: Error reporting capabilities for libxc 2006-10-23 20:57 ` Daniel P. Berrange @ 2006-10-23 21:10 ` Ian Pratt 2006-10-23 21:14 ` Anthony Liguori 2006-10-23 21:21 ` Daniel P. Berrange 0 siblings, 2 replies; 25+ messages in thread From: Ian Pratt @ 2006-10-23 21:10 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: Anthony Liguori, xen-devel > > Do we actually need pointers? I'd much rather go with __thread. > > Well, we need a char buffer to store the error message, since that has much > more useful info than the error code alone. I could always just do a fixed > 200 byte buffer, and truncate anything longer than this which would also > actually remove the annoying OOM problem in copying the error message. Would we be better off returning an error code and a set of parameters, requiring a call-back into the library to get the string? It's worth thinking about future language localisation here too. Ian ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Re: Error reporting capabilities for libxc 2006-10-23 21:10 ` Ian Pratt @ 2006-10-23 21:14 ` Anthony Liguori 2006-10-23 21:28 ` Daniel P. Berrange 2006-10-24 7:47 ` Gerd Hoffmann 2006-10-23 21:21 ` Daniel P. Berrange 1 sibling, 2 replies; 25+ messages in thread From: Anthony Liguori @ 2006-10-23 21:14 UTC (permalink / raw) To: Ian Pratt; +Cc: xen-devel, Daniel P. Berrange Ian Pratt wrote: > Would we be better off returning an error code and a set of parameters, > requiring a call-back into the library to get the string? > > It's worth thinking about future language localisation here too. > I know it's a bigger patch, but the Right Thing to do here is to just propagate an error code through the libxc functions. The whole xc_{get,set}_error() is a cludge. Threading wouldn't be a problem if we returned proper error codes. Looking at your patch, it doesn't seem like it would really be that hard... Regards, Anthony Liguori > Ian > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Re: Error reporting capabilities for libxc 2006-10-23 21:14 ` Anthony Liguori @ 2006-10-23 21:28 ` Daniel P. Berrange 2006-10-23 21:38 ` Anthony Liguori 2006-10-24 7:47 ` Gerd Hoffmann 1 sibling, 1 reply; 25+ messages in thread From: Daniel P. Berrange @ 2006-10-23 21:28 UTC (permalink / raw) To: Anthony Liguori; +Cc: Ian Pratt, xen-devel On Mon, Oct 23, 2006 at 04:14:59PM -0500, Anthony Liguori wrote: > Ian Pratt wrote: > >Would we be better off returning an error code and a set of parameters, > >requiring a call-back into the library to get the string? > > > >It's worth thinking about future language localisation here too. > > > > I know it's a bigger patch, but the Right Thing to do here is to just > propagate an error code through the libxc functions. > > The whole xc_{get,set}_error() is a cludge. Threading wouldn't be a > problem if we returned proper error codes. This would be an insufficient level of detail compared to my patch. An error code alone can tell you there was an invalid kernel, or even particular tests which failed. It can *not* tell you that when the architecture mis-matched, the expected arch was 'i386' and the actual arch was 'x86_64'. Hence why I provided both an error code & a detailed message. Notice in the following there are two strings - 'Invalid kernel' is the string associated with the error code 'XC_INVALID_KERNEL'. This is the generic static mapping. The second string though is dynamically generated according to the specific metadata which was incorrect - this is the invaluable user facing information which cuts down on debugging pain. [root@dhcp-4-245 ~]# xm create crash Using config file "/etc/xen/crash". Error: [2, 'Invalid kernel', "Kernel ELF architecture '3' does not match Xen architecture '62' (x86_64)"] [root@dhcp-4-245 ~]# xm create crash Using config file "/etc/xen/crash". Error: [2, 'Invalid kernel', "Kernel ELF type '3' does not match Xen type '2'"] [root@dhcp-4-245 ~]# xm create crash Using config file "/etc/xen/crash". Error: [2, 'Invalid kernel', 'Not a valid ELF or raw kernel image'] Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Re: Error reporting capabilities for libxc 2006-10-23 21:28 ` Daniel P. Berrange @ 2006-10-23 21:38 ` Anthony Liguori 2006-10-23 21:47 ` John Levon 2006-10-23 21:48 ` Daniel P. Berrange 0 siblings, 2 replies; 25+ messages in thread From: Anthony Liguori @ 2006-10-23 21:38 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: Ian Pratt, xen-devel Daniel P. Berrange wrote: > On Mon, Oct 23, 2006 at 04:14:59PM -0500, Anthony Liguori wrote: > >> Ian Pratt wrote: >> >>> Would we be better off returning an error code and a set of parameters, >>> requiring a call-back into the library to get the string? >>> >>> It's worth thinking about future language localisation here too. >>> >>> >> I know it's a bigger patch, but the Right Thing to do here is to just >> propagate an error code through the libxc functions. >> >> The whole xc_{get,set}_error() is a cludge. Threading wouldn't be a >> problem if we returned proper error codes. >> > > This would be an insufficient level of detail compared to my patch. An error > code alone can tell you there was an invalid kernel, or even particular tests > which failed. It can *not* tell you that when the architecture mis-matched, > the expected arch was 'i386' and the actual arch was 'x86_64'. Hence why > I provided both an error code & a detailed message. > > Notice in the following there are two strings - 'Invalid kernel' is the > string associated with the error code 'XC_INVALID_KERNEL'. This is the > generic static mapping. The second string though is dynamically generated > according to the specific metadata which was incorrect - this is the > invaluable user facing information which cuts down on debugging pain. > stdout from libxenctrl gets redirected to the Xend logs. You could write this sort of info to the logs. I don't know of many users that will be able to make sense out of the following lines. How many users know what an "ELF architecture" is that wouldn't be capable of looking in log file? If a user passes an invalid kernel line in the config, I think an appropriate error would be "Kernel <filename> is not a valid Xen kernel." That's pretty clear and understandable. This message is totally reproducible in xm with just an XC_INVALID_KERNEL error code. Regards, Anthony Liguori > [root@dhcp-4-245 ~]# xm create crash > Using config file "/etc/xen/crash". > Error: [2, 'Invalid kernel', "Kernel ELF architecture '3' does not match Xen architecture '62' (x86_64)"] > [root@dhcp-4-245 ~]# xm create crash > Using config file "/etc/xen/crash". > Error: [2, 'Invalid kernel', "Kernel ELF type '3' does not match Xen type '2'"] > [root@dhcp-4-245 ~]# xm create crash > Using config file "/etc/xen/crash". > Error: [2, 'Invalid kernel', 'Not a valid ELF or raw kernel image'] > > Dan. > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Re: Error reporting capabilities for libxc 2006-10-23 21:38 ` Anthony Liguori @ 2006-10-23 21:47 ` John Levon 2006-10-24 9:15 ` Daniel Veillard 2006-10-23 21:48 ` Daniel P. Berrange 1 sibling, 1 reply; 25+ messages in thread From: John Levon @ 2006-10-23 21:47 UTC (permalink / raw) To: Anthony Liguori; +Cc: Ian Pratt, xen-devel, Daniel P. Berrange On Mon, Oct 23, 2006 at 04:38:54PM -0500, Anthony Liguori wrote: > stdout from libxenctrl gets redirected to the Xend logs. You could > write this sort of info to the logs. We already have enough problems with stuff disappearing into the logs and having to look through it carefully to find the actual problem :( > I don't know of many users that will be able to make sense out of the > following lines. How many users know what an "ELF architecture" is that > wouldn't be capable of looking in log file? The messages could use some loving certainly. > If a user passes an invalid kernel line in the config, I think an > appropriate error would be "Kernel <filename> is not a valid Xen kernel." Throwing away information on the error never seems like a good idea, especially when you consider, say, the Xen API. Logs are particular awkward when you're remotely managing a machine via a VM console. Even finding the right host machine could be frustrating enough. regards john ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Re: Error reporting capabilities for libxc 2006-10-23 21:47 ` John Levon @ 2006-10-24 9:15 ` Daniel Veillard 0 siblings, 0 replies; 25+ messages in thread From: Daniel Veillard @ 2006-10-24 9:15 UTC (permalink / raw) To: John Levon; +Cc: Ian Pratt, Anthony Liguori, xen-devel, Daniel P. Berrange On Mon, Oct 23, 2006 at 10:47:40PM +0100, John Levon wrote: > On Mon, Oct 23, 2006 at 04:38:54PM -0500, Anthony Liguori wrote: > > If a user passes an invalid kernel line in the config, I think an > > appropriate error would be "Kernel <filename> is not a valid Xen kernel." > > Throwing away information on the error never seems like a good idea, > especially when you consider, say, the Xen API. Logs are particular > awkward when you're remotely managing a machine via a VM console. Even > finding the right host machine could be frustrating enough. I agree totally. You can have structured information where you need it if reported at the API level. Saving in unstructured logs on the managed machine is in general not very helpful. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Re: Error reporting capabilities for libxc 2006-10-23 21:38 ` Anthony Liguori 2006-10-23 21:47 ` John Levon @ 2006-10-23 21:48 ` Daniel P. Berrange 1 sibling, 0 replies; 25+ messages in thread From: Daniel P. Berrange @ 2006-10-23 21:48 UTC (permalink / raw) To: Anthony Liguori; +Cc: Ian Pratt, xen-devel On Mon, Oct 23, 2006 at 04:38:54PM -0500, Anthony Liguori wrote: > Daniel P. Berrange wrote: > >On Mon, Oct 23, 2006 at 04:14:59PM -0500, Anthony Liguori wrote: > > > >>Ian Pratt wrote: > >> > >>>Would we be better off returning an error code and a set of parameters, > >>>requiring a call-back into the library to get the string? > >>> > >>>It's worth thinking about future language localisation here too. > >>> > >>> > >>I know it's a bigger patch, but the Right Thing to do here is to just > >>propagate an error code through the libxc functions. > >> > >>The whole xc_{get,set}_error() is a cludge. Threading wouldn't be a > >>problem if we returned proper error codes. > >> > > > >This would be an insufficient level of detail compared to my patch. An > >error > >code alone can tell you there was an invalid kernel, or even particular > >tests > >which failed. It can *not* tell you that when the architecture mis-matched, > >the expected arch was 'i386' and the actual arch was 'x86_64'. Hence why > >I provided both an error code & a detailed message. > > > >Notice in the following there are two strings - 'Invalid kernel' is the > >string associated with the error code 'XC_INVALID_KERNEL'. This is the > >generic static mapping. The second string though is dynamically generated > >according to the specific metadata which was incorrect - this is the > >invaluable user facing information which cuts down on debugging pain. > > > > stdout from libxenctrl gets redirected to the Xend logs. You could > write this sort of info to the logs. Where is it utterly inaccessible for any applications talking to XenD. > I don't know of many users that will be able to make sense out of the > following lines. How many users know what an "ELF architecture" is that > wouldn't be capable of looking in log file? You don't neccessarily show that level of detail to the user, but it is important to propagate it all the way back to the tools talking to libxc or Xend. xm, or libvirt, or virt-manager, or any other tool talking to XenD should have access to this kind of debugging info. If I'm doing off-box management of hosts & a domain create operation fails, I don't want to have to login to the box in question to get this information out of the xend logs, when xend could quite easily have passed it all back over the wire in the first place. > If a user passes an invalid kernel line in the config, I think an > appropriate error would be "Kernel <filename> is not a valid Xen kernel." > > That's pretty clear and understandable. This message is totally > reproducible in xm with just an XC_INVALID_KERNEL error code. It really depends on the end user in question. If the information is available, an API should pass it back to the applicationxs to decide what level of detail is appropriate to present. We shouldn't encode the error reporting policy in the API itself, because it has no idea of the needs of the caller. BTW, when I say 'user' i mean caller of the (Xend) APIs (ie applications), rather than human operators. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Re: Error reporting capabilities for libxc 2006-10-23 21:14 ` Anthony Liguori 2006-10-23 21:28 ` Daniel P. Berrange @ 2006-10-24 7:47 ` Gerd Hoffmann 2006-10-24 9:07 ` Keir Fraser 1 sibling, 1 reply; 25+ messages in thread From: Gerd Hoffmann @ 2006-10-24 7:47 UTC (permalink / raw) To: Anthony Liguori; +Cc: Ian Pratt, xen-devel, Daniel P. Berrange Anthony Liguori wrote: > I know it's a bigger patch, but the Right Thing to do here is to just > propagate an error code through the libxc functions. That doesn't allow a descriptive text message, which is often quite useful to figure what went wrong in detail. error codes such as errno == EINVAL are not very helpful for trouble shooting ... What number space we are talking about btw? errno? something else? cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> http://www.suse.de/~kraxel/julika-dora.jpeg ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Re: Error reporting capabilities for libxc 2006-10-24 7:47 ` Gerd Hoffmann @ 2006-10-24 9:07 ` Keir Fraser 2006-10-24 10:26 ` Gerd Hoffmann 0 siblings, 1 reply; 25+ messages in thread From: Keir Fraser @ 2006-10-24 9:07 UTC (permalink / raw) To: Gerd Hoffmann, Anthony Liguori; +Cc: Ian Pratt, xen-devel, Daniel P. Berrange On 24/10/06 08:47, "Gerd Hoffmann" <kraxel@suse.de> wrote: >> I know it's a bigger patch, but the Right Thing to do here is to just >> propagate an error code through the libxc functions. > > That doesn't allow a descriptive text message, which is often quite > useful to figure what went wrong in detail. error codes such as errno > == EINVAL are not very helpful for trouble shooting ... > > What number space we are talking about btw? errno? something else? I think a separate numbering space, where each err number would have some number of auxiliary arguments is being suggested, probably with an xc_{get,set}_errnum() interface (since the 'errnum' would not be a simple scalar so would be a bit of a pain to pass around). xc_{get,set}_errtext() (or whatever it is called) could co-exist with this, so I am personally in favour of the patch, at least as a starting point for further work. It directly tackles our biggest current err-handling problem, which is stupidly throwing away useful diagnostic info (or writing it to log files where it gets hidden in with piles of other crap). I guess using __thread is the best of a bad set of choices, by the way. It'll limit us to gcc-3.3.x and later, which is fine. And we'll want to build 32-bit with -mno-tls-direct-seg-refs. We can add that to CFLAGS for all tools. I was wondering about explicitly stating that libxc is no thread-safe for any given 'xc handle', but actually that is just storing up bigger trouble. -- Keir ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Re: Error reporting capabilities for libxc 2006-10-24 9:07 ` Keir Fraser @ 2006-10-24 10:26 ` Gerd Hoffmann 0 siblings, 0 replies; 25+ messages in thread From: Gerd Hoffmann @ 2006-10-24 10:26 UTC (permalink / raw) To: Keir Fraser; +Cc: Ian Pratt, Anthony Liguori, xen-devel, Daniel P. Berrange Keir Fraser wrote: > On 24/10/06 08:47, "Gerd Hoffmann" <kraxel@suse.de> wrote: > >> What number space we are talking about btw? errno? something else? > > I think a separate numbering space, where each err number would have some > number of auxiliary arguments is being suggested, probably with an > xc_{get,set}_errnum() interface (since the 'errnum' would not be a simple > scalar so would be a bit of a pain to pass around). I somehow feel this is a bit overdesigned. I've just walked through my domain builder rewrite, trying to assign useful error codes. After all there are not that many different cases: (1) internal errors (all sorts of sanity checks which normally never ever fail except when coding new bugs ;) ). (2) out of memory. (3) reading some file failed (kernel / initrd / hvmloader / whatelse). (4) invalid kernel image (broken elf headers, truncated file, ...). (5) incompatible kernel image (pae vs. nonpae, ppc kernel on x86, ...). (6) some invalid parameter (such as asking for a feature not supported by hypervisor or guest kernel, ...) And the is only one where a fixed set of parameters specifying in detail what went wrong somehow makes sense: for #3 this would be the filename and maybe the reason (EPERM, ENOENT, ...). For the other ones IMHO only free text makes sense as detailed description ... cheers, Gerd -- Gerd Hoffmann <kraxel@suse.de> http://www.suse.de/~kraxel/julika-dora.jpeg ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Re: Error reporting capabilities for libxc 2006-10-23 21:10 ` Ian Pratt 2006-10-23 21:14 ` Anthony Liguori @ 2006-10-23 21:21 ` Daniel P. Berrange 2006-10-23 22:15 ` Ian Pratt 1 sibling, 1 reply; 25+ messages in thread From: Daniel P. Berrange @ 2006-10-23 21:21 UTC (permalink / raw) To: Ian Pratt; +Cc: Anthony Liguori, xen-devel On Mon, Oct 23, 2006 at 10:10:31PM +0100, Ian Pratt wrote: > > > Do we actually need pointers? I'd much rather go with __thread. > > > > Well, we need a char buffer to store the error message, since that has > much > > more useful info than the error code alone. I could always just do a > fixed > > 200 byte buffer, and truncate anything longer than this which would > also > > actually remove the annoying OOM problem in copying the error message. > > Would we be better off returning an error code and a set of parameters, > requiring a call-back into the library to get the string? That assumes that there is a static mapping between an error code and the error description, which there isn't. The error description can contain info about actual bits of metadata which were incorrect. For example when reporting an invalid ELF architecture, it can tell you exactly what ELF arch was found & what was expected. > It's worth thinking about future language localisation here too. Yep, I don't see any trouble adding localization to this API. We'd just feed the error messages through gettext before passing them into the xc_set_error method. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| ^ permalink raw reply [flat|nested] 25+ messages in thread
* RE: Re: Error reporting capabilities for libxc 2006-10-23 21:21 ` Daniel P. Berrange @ 2006-10-23 22:15 ` Ian Pratt 2006-10-23 23:05 ` Daniel P. Berrange 0 siblings, 1 reply; 25+ messages in thread From: Ian Pratt @ 2006-10-23 22:15 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: Anthony Liguori, xen-devel > > Would we be better off returning an error code and a set of parameters, > > requiring a call-back into the library to get the string? > > That assumes that there is a static mapping between an error code > and the error description, which there isn't. The error description > can contain info about actual bits of metadata which were incorrect. > For example when reporting an invalid ELF architecture, it can tell > you exactly what ELF arch was found & what was expected. That's precisely my point. The various bits of metadata can be parameters passed back along with the error code. It's possible that some callers will be able to do things with the metadata that are more useful than just printing the string. I guess the error parameters could be returned as a va_list. Many callers would just call back into the library to get the appropriate error string. Ian ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Re: Error reporting capabilities for libxc 2006-10-23 22:15 ` Ian Pratt @ 2006-10-23 23:05 ` Daniel P. Berrange 0 siblings, 0 replies; 25+ messages in thread From: Daniel P. Berrange @ 2006-10-23 23:05 UTC (permalink / raw) To: Ian Pratt; +Cc: Anthony Liguori, xen-devel On Mon, Oct 23, 2006 at 11:15:21PM +0100, Ian Pratt wrote: > > > Would we be better off returning an error code and a set of > parameters, > > > requiring a call-back into the library to get the string? > > > > That assumes that there is a static mapping between an error code > > and the error description, which there isn't. The error description > > can contain info about actual bits of metadata which were incorrect. > > For example when reporting an invalid ELF architecture, it can tell > > you exactly what ELF arch was found & what was expected. > > That's precisely my point. The various bits of metadata can be > parameters passed back along with the error code. It's possible that > some callers will be able to do things with the metadata that are more > useful than just printing the string. I guess the error parameters could > be returned as a va_list. Many callers would just call back into the > library to get the appropriate error string. Ahh, I mis-read your comment a little. That is certainly one possibility although I'm not entirely a fan of va_list as an API - perhaps either a struct with a handful of generic data fields for returning metadata, or a union switched on the error code would work - it'd be nice to be able to copy all the bits around without needing memory allocation in any of the error handling paths. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Error reporting capabilities for libxc 2006-10-23 18:00 ` Daniel P. Berrange 2006-10-23 19:17 ` Anthony Liguori @ 2006-10-23 20:40 ` John Levon 2006-10-23 21:04 ` John Levon 2 siblings, 0 replies; 25+ messages in thread From: John Levon @ 2006-10-23 20:40 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: Ian Pratt, xen-devel On Mon, Oct 23, 2006 at 07:00:17PM +0100, Daniel P. Berrange wrote: > The only issue is that __thread is a GCC specific extension, so I'm not > sure this will work for the Solaris folks ? Yes, it will. We use GCC for all the Xen code itself, and __thread seems to work fine. I'll give your patch a sanity check tomorrow... regards john ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Error reporting capabilities for libxc 2006-10-23 18:00 ` Daniel P. Berrange 2006-10-23 19:17 ` Anthony Liguori 2006-10-23 20:40 ` John Levon @ 2006-10-23 21:04 ` John Levon 2006-10-23 21:09 ` Daniel P. Berrange 2 siblings, 1 reply; 25+ messages in thread From: John Levon @ 2006-10-23 21:04 UTC (permalink / raw) To: Daniel P. Berrange; +Cc: Ian Pratt, xen-devel On Mon, Oct 23, 2006 at 07:00:17PM +0100, Daniel P. Berrange wrote: > Attached is an update to the original patch which annotates the static > variables with __thread so that they are setup per-thread. It also adds > more informative error reporting for bad kernels. It needs merging with latest tip I think. xc_private.h is missing a prototype for xc_set_error(), so it doesn't compile. Also it re-introduces ERR(), which causes problems for us, and can just be removed. I fixed these two problems and did some simple tests on Solaris; all seems OK: bash-3.00# xm create -c johnlev-64.py Using config file "johnlev-64.py". Error: [2, 'Invalid kernel', "Kernel ELF architecture '62' does not match Xen architecture '3' (i386)"] (It's so wonderful to get a meaningful error message at last!!) regards john ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Error reporting capabilities for libxc 2006-10-23 21:04 ` John Levon @ 2006-10-23 21:09 ` Daniel P. Berrange 0 siblings, 0 replies; 25+ messages in thread From: Daniel P. Berrange @ 2006-10-23 21:09 UTC (permalink / raw) To: John Levon; +Cc: Ian Pratt, xen-devel On Mon, Oct 23, 2006 at 10:04:59PM +0100, John Levon wrote: > On Mon, Oct 23, 2006 at 07:00:17PM +0100, Daniel P. Berrange wrote: > > > Attached is an update to the original patch which annotates the static > > variables with __thread so that they are setup per-thread. It also adds > > more informative error reporting for bad kernels. > > It needs merging with latest tip I think. xc_private.h is missing a > prototype for xc_set_error(), so it doesn't compile. Also it > re-introduces ERR(), which causes problems for us, and can just be > removed. Ah yes, I hadn't noticed there were xen-unstable changes in this area. I did the patch against 3.0.3 final release. I'll rebase to xen-unstable before posting again. > I fixed these two problems and did some simple tests on Solaris; all > seems OK: > > bash-3.00# xm create -c johnlev-64.py > Using config file "johnlev-64.py". > Error: [2, 'Invalid kernel', "Kernel ELF architecture '62' does not match Xen architecture '3' (i386)"] > > (It's so wonderful to get a meaningful error message at last!!) Thats good news, thanks for testing it. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=| ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2006-10-24 10:26 UTC | newest] Thread overview: 25+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-09-26 12:58 Error reporting capabilities for libxc Ian Pratt 2006-09-26 13:20 ` Daniel P. Berrange 2006-10-23 18:00 ` Daniel P. Berrange 2006-10-23 19:17 ` Anthony Liguori 2006-10-23 19:27 ` Daniel P. Berrange 2006-10-23 20:04 ` Anthony Liguori 2006-10-23 20:08 ` Daniel P. Berrange 2006-10-23 20:54 ` Ian Pratt 2006-10-23 20:57 ` Daniel P. Berrange 2006-10-23 21:10 ` Ian Pratt 2006-10-23 21:14 ` Anthony Liguori 2006-10-23 21:28 ` Daniel P. Berrange 2006-10-23 21:38 ` Anthony Liguori 2006-10-23 21:47 ` John Levon 2006-10-24 9:15 ` Daniel Veillard 2006-10-23 21:48 ` Daniel P. Berrange 2006-10-24 7:47 ` Gerd Hoffmann 2006-10-24 9:07 ` Keir Fraser 2006-10-24 10:26 ` Gerd Hoffmann 2006-10-23 21:21 ` Daniel P. Berrange 2006-10-23 22:15 ` Ian Pratt 2006-10-23 23:05 ` Daniel P. Berrange 2006-10-23 20:40 ` John Levon 2006-10-23 21:04 ` John Levon 2006-10-23 21:09 ` Daniel P. Berrange
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.
