[libvirt] [PATCH] Detailed XML errors when XML is not well-formed

[Daniel Veillard]

On Wed, Jul 30, 2008 at 03:57:57PM +0100, Daniel P. Berrange wrote:
> On Wed, Jul 30, 2008 at 03:49:17PM +0100, Richard W.M. Jones wrote:
> > This patch changes the implementations lots of functions which parse
> > XML documents, so that if the XML document is not well-formed then we
> > get detailed error messages.
> > 
> > The general form of the change is:
> > 
> >     static void
> >     catchXMLError (void *ctx, const char *msg ATTRIBUTE_UNUSED, ...)
> >     {
> >         // a callback which calls virDomainReportError
> >     }
> >     
> >     virDomainDefPtr virDomainDefParseString(virConnectPtr conn,
> >                                             virCapsPtr caps,
> >                                             const char *xmlStr)
> >     {
> >         xmlParserCtxtPtr pctxt;
> >     
> >         pctxt = xmlNewParserCtxt ();
> >         pctxt->sax->error = catchXMLError;
> >     
> >         xml = xmlCtxtReadDoc (pctxt, //...) etc.
> > 
> > There are some unavoidable shortcomings:
> > 
> > (1) There is no place to stash user pointers during the callback (the
> > suggestively named pctxt->userData field is already used for something
> > else), so we cannot pass the virConnectPtr to the error function.  As
> > a result, virterror will store the error in a global variable, and
> > callers will probably not be able to access it.

      void              *_private; /* For user data, libxml won't touch it */

just set _private in the parser context, you should get the parser context
back in the SAX handler, then you can get to your _private data. that
should work and avoid the mess of global variables and thread stuff...

> > (2) The XML parser routinely produces multiple error messages, and
> > virterror throws away all but the last one.
> 
> It'd probably be more use to only report the first one, since the latter
> problems may well be caused by earlier problems.

  Likely but not always correct, maybe the solution is to build a big 
string concatenating all the errors and passing it back

> You won't see those except with the 'test' or 'xen' drivers run 
> locally. Anything going  via the daemon will be output on the
> daemon's stderr (/dev/null) and not on the client.

  yeah, unfortunately we have to assume a model where each errors can't
be reported individually,

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/