[libvirt] [PATCH] Detailed XML errors using the _private field

[Daniel P. Berrange]

On Wed, Jul 30, 2008 at 05:23:50PM +0100, Richard W.M. Jones wrote:
> Here is a patch which uses the _private field to store the
> virConnectPtr.

This looks nicer than the pthread approach. I'd still like to see
it report the first error, instead of last though.

> Index: src/domain_conf.c
> ===================================================================
> RCS file: /data/cvs/libvirt/src/domain_conf.c,v
> retrieving revision 1.10
> diff -u -r1.10 domain_conf.c
> --- src/domain_conf.c	28 Jul 2008 14:06:54 -0000	1.10
> +++ src/domain_conf.c	30 Jul 2008 16:21:54 -0000
> @@ -1838,32 +1838,57 @@
>      return NULL;
>  }
>  
> +/* Called from SAX on parsing errors in the XML. */
> +static void
> +catchXMLError (void *ctx, const char *msg ATTRIBUTE_UNUSED, ...)
> +{
> +    xmlParserCtxtPtr ctxt = (xmlParserCtxtPtr) ctx;
> +
> +    if (ctxt &&
> +        ctxt->lastError.level == XML_ERR_FATAL &&
> +        ctxt->lastError.message != NULL) {

eg to add in

            if (ctx->lastError.code == VIR_ERR_NONE) 

> +        virDomainReportError (ctxt->_private, VIR_ERR_XML_DETAIL,
> +                              ctxt->lastError.message, ctxt->lastError.line);
> +    }
> +}
>  
>  virDomainDefPtr virDomainDefParseString(virConnectPtr conn,
>                                          virCapsPtr caps,
>                                          const char *xmlStr)
>  {
> -    xmlDocPtr xml;
> +    xmlParserCtxtPtr pctxt;
> +    xmlDocPtr xml = NULL;
>      xmlNodePtr root;
>      virDomainDefPtr def = NULL;
>  
> -    if (!(xml = xmlReadDoc(BAD_CAST xmlStr, "domain.xml", NULL,
> -                           XML_PARSE_NOENT | XML_PARSE_NONET |
> -                           XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
> -        virDomainReportError(conn, VIR_ERR_XML_ERROR, NULL);
> -        return NULL;
> +    /* Set up a parser context so we can catch the details of XML errors. */
> +    pctxt = xmlNewParserCtxt ();
> +    if (!pctxt || !pctxt->sax)
> +        goto cleanup;
> +    pctxt->sax->error = catchXMLError;
> +    pctxt->_private = conn;
> +

And have

        virResetError(&conn->err);

> +    xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr, "domain.xml", NULL,
> +                          XML_PARSE_NOENT | XML_PARSE_NONET |
> +                          XML_PARSE_NOWARNING);
> +    if (!xml) {
> +        /* virDomainReportError has already been called by the
> +         * callback function (hopefully).
> +         */

Should set a generic error if no specific one is set..

           if (conn->err.code == VIR_ERR_NONE)
              virDomainReportError(conn, VIR_ERR_XML_ERR, _("failed to parse xml doc")


> +        goto cleanup;
>      }


[snip]

And same for other 3 parsers.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|