Improve error messages when XML is not well-formed.

	* src/domain_conf.c, src/network_conf.c, src/storage_conf.c:
	  Improve error messages from commands such as 'virsh define'
	  when the XML is not well-formed by passing libxml2 errors
	  back out through virterror.

diff --git a/ChangeLog b/ChangeLog
index b5a110063454598213ae63af8d625aca980ea383..9c73ea799c5ddd0d00eb8e69b2a98d58dbd68939 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+Fri Aug  1 10:38:00 BST 2008 Richard W.M. Jones <rjones@redhat.com>
+
+       Improve error messages when XML is not well-formed.
+       * src/domain_conf.c, src/network_conf.c, src/storage_conf.c:
+         Improve error messages from commands such as 'virsh define'
+         when the XML is not well-formed by passing libxml2 errors
+         back out through virterror.
+
 Fri Aug  1 08:40:48 CEST 2008 Daniel Veillard <veillard@redhat.com>
 
        * docs/formatdomain.html docs/formatdomain.html.in docs/libvirt-api.xml

diff --git a/src/domain_conf.c b/src/domain_conf.c
index 30a40eb8190498c09d4b963ac10428dd86cfda0d..0e99ccc5e19fbcfad03213bee1e4527e2759937e 100644 (file)
--- a/src/domain_conf.c
+++ b/src/domain_conf.c
@@ -1838,32 +1838,65 @@ static virDomainDefPtr virDomainDefParseXML(virConnectPtr conn,
     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) {
+        virConnectPtr conn = ctxt->_private;
+
+        if (conn &&
+            conn->err.code == VIR_ERR_NONE &&
+            ctxt->lastError.level == XML_ERR_FATAL &&
+            ctxt->lastError.message != NULL) {
+            virDomainReportError (conn, VIR_ERR_XML_DETAIL,
+                                  _("at line %d: %s"),
+                                  ctxt->lastError.line,
+                                  ctxt->lastError.message);
+        }
+    }
+}
 
 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;
+
+    if (conn) virResetError (&conn->err);
+    xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr, "domain.xml", NULL,
+                          XML_PARSE_NOENT | XML_PARSE_NONET |
+                          XML_PARSE_NOWARNING);
+    if (!xml) {
+        if (conn && conn->err.code == VIR_ERR_NONE)
+              virDomainReportError(conn, VIR_ERR_XML_ERROR,
+                                   _("failed to parse xml document"));
+        goto cleanup;
     }
 
     if ((root = xmlDocGetRootElement(xml)) == NULL) {
         virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               "%s", _("missing root element"));
-        xmlFreeDoc(xml);
-        return NULL;
+        goto cleanup;
     }
 
     def = virDomainDefParseNode(conn, caps, xml, root);
 
-    xmlFreeDoc(xml);
+cleanup:
+    xmlFreeParserCtxt (pctxt);
+    xmlFreeDoc (xml);
     return def;
 }
 
@@ -1871,27 +1904,40 @@ virDomainDefPtr virDomainDefParseFile(virConnectPtr conn,
                                       virCapsPtr caps,
                                       const char *filename)
 {
-    xmlDocPtr xml;
+    xmlParserCtxtPtr pctxt;
+    xmlDocPtr xml = NULL;
     xmlNodePtr root;
     virDomainDefPtr def = NULL;
 
-    if (!(xml = xmlReadFile(filename, 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;
+
+    if (conn) virResetError (&conn->err);
+    xml = xmlCtxtReadFile (pctxt, filename, NULL,
+                           XML_PARSE_NOENT | XML_PARSE_NONET |
+                           XML_PARSE_NOWARNING);
+    if (!xml) {
+        if (conn && conn->err.code == VIR_ERR_NONE)
+              virDomainReportError(conn, VIR_ERR_XML_ERROR,
+                                   _("failed to parse xml document"));
+        goto cleanup;
     }
 
     if ((root = xmlDocGetRootElement(xml)) == NULL) {
         virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               "%s", _("missing root element"));
-        xmlFreeDoc(xml);
-        return NULL;
+        goto cleanup;
     }
 
     def = virDomainDefParseNode(conn, caps, xml, root);
 
-    xmlFreeDoc(xml);
+cleanup:
+    xmlFreeParserCtxt (pctxt);
+    xmlFreeDoc (xml);
     return def;
 }
 

diff --git a/src/network_conf.c b/src/network_conf.c
index 39ec00aed37521d4a3f7ea66440fc52c371382e6..10c9dca9e46f105e156a41cc79bf00e4e3593f32 100644 (file)
--- a/src/network_conf.c
+++ b/src/network_conf.c
@@ -348,57 +348,104 @@ virNetworkDefParseXML(virConnectPtr conn,
     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) {
+        virConnectPtr conn = ctxt->_private;
+
+        if (conn &&
+            conn->err.code == VIR_ERR_NONE &&
+            ctxt->lastError.level == XML_ERR_FATAL &&
+            ctxt->lastError.message != NULL) {
+            virNetworkReportError (conn, VIR_ERR_XML_DETAIL,
+                                   _("at line %d: %s"),
+                                   ctxt->lastError.line,
+                                   ctxt->lastError.message);
+        }
+    }
+}
+
 virNetworkDefPtr virNetworkDefParseString(virConnectPtr conn,
                                           const char *xmlStr)
 {
-    xmlDocPtr xml;
+    xmlParserCtxtPtr pctxt;
+    xmlDocPtr xml = NULL;
     xmlNodePtr root;
-    virNetworkDefPtr def;
+    virNetworkDefPtr def = NULL;
 
-    if (!(xml = xmlReadDoc(BAD_CAST xmlStr, "network.xml", NULL,
-                           XML_PARSE_NOENT | XML_PARSE_NONET |
-                           XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
-        virNetworkReportError(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;
+
+    if (conn) virResetError (&conn->err);
+    xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr, "network.xml", NULL,
+                          XML_PARSE_NOENT | XML_PARSE_NONET |
+                          XML_PARSE_NOWARNING);
+    if (!xml) {
+        if (conn && conn->err.code == VIR_ERR_NONE)
+              virNetworkReportError(conn, VIR_ERR_XML_ERROR,
+                                    _("failed to parse xml document"));
+        goto cleanup;
     }
 
     if ((root = xmlDocGetRootElement(xml)) == NULL) {
         virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               "%s", _("missing root element"));
-        xmlFreeDoc(xml);
-        return NULL;
+        goto cleanup;
     }
 
     def = virNetworkDefParseNode(conn, xml, root);
 
-    xmlFreeDoc(xml);
+cleanup:
+    xmlFreeParserCtxt (pctxt);
+    xmlFreeDoc (xml);
     return def;
 }
 
 virNetworkDefPtr virNetworkDefParseFile(virConnectPtr conn,
                                         const char *filename)
 {
-    xmlDocPtr xml;
+    xmlParserCtxtPtr pctxt;
+    xmlDocPtr xml = NULL;
     xmlNodePtr root;
-    virNetworkDefPtr def;
+    virNetworkDefPtr def = NULL;
 
-    if (!(xml = xmlReadFile(filename, NULL,
-                            XML_PARSE_NOENT | XML_PARSE_NONET |
-                            XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
-        virNetworkReportError(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;
+
+    if (conn) virResetError (&conn->err);
+    xml = xmlCtxtReadFile (pctxt, filename, NULL,
+                           XML_PARSE_NOENT | XML_PARSE_NONET |
+                           XML_PARSE_NOWARNING);
+    if (!xml) {
+        if (conn && conn->err.code == VIR_ERR_NONE)
+              virNetworkReportError(conn, VIR_ERR_XML_ERROR,
+                                    _("failed to parse xml document"));
+        goto cleanup;
     }
 
     if ((root = xmlDocGetRootElement(xml)) == NULL) {
         virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               "%s", _("missing root element"));
-        xmlFreeDoc(xml);
-        return NULL;
+        goto cleanup;
     }
 
     def = virNetworkDefParseNode(conn, xml, root);
 
-    xmlFreeDoc(xml);
+cleanup:
+    xmlFreeParserCtxt (pctxt);
+    xmlFreeDoc (xml);
     return def;
 }
 

diff --git a/src/storage_conf.c b/src/storage_conf.c
index 1880886688a766264022c4ae3a5e77e7114e0a46..05b68af15b99a74a379f8dd8f463bf31ad11bbf7 100644 (file)
--- a/src/storage_conf.c
+++ b/src/storage_conf.c
@@ -361,22 +361,53 @@ virStoragePoolDefParseDoc(virConnectPtr conn,
     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) {
+        virConnectPtr conn = ctxt->_private;
+
+        if (conn &&
+            conn->err.code == VIR_ERR_NONE &&
+            ctxt->lastError.level == XML_ERR_FATAL &&
+            ctxt->lastError.message != NULL) {
+            virStorageReportError (conn, VIR_ERR_XML_DETAIL,
+                                   _("at line %d: %s"),
+                                   ctxt->lastError.line,
+                                   ctxt->lastError.message);
+        }
+    }
+}
+
 virStoragePoolDefPtr
 virStoragePoolDefParse(virConnectPtr conn,
                        const char *xmlStr,
                        const char *filename) {
     virStoragePoolDefPtr ret = NULL;
+    xmlParserCtxtPtr pctxt;
     xmlDocPtr xml = NULL;
     xmlNodePtr node = NULL;
     xmlXPathContextPtr ctxt = NULL;
 
-    if (!(xml = xmlReadDoc(BAD_CAST xmlStr,
-                           filename ? filename : "storage.xml",
-                           NULL,
-                           XML_PARSE_NOENT | XML_PARSE_NONET |
-                           XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
-        virStorageReportError(conn, VIR_ERR_XML_ERROR,
-                              "%s", _("malformed xml document"));
+    /* 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;
+
+    if (conn) virResetError (&conn->err);
+    xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr,
+                          filename ? filename : "storage.xml", NULL,
+                          XML_PARSE_NOENT | XML_PARSE_NONET |
+                          XML_PARSE_NOWARNING);
+    if (!xml) {
+        if (conn && conn->err.code == VIR_ERR_NONE)
+              virStorageReportError(conn, VIR_ERR_XML_ERROR,
+                                    _("failed to parse xml document"));
         goto cleanup;
     }
 
@@ -396,12 +427,14 @@ virStoragePoolDefParse(virConnectPtr conn,
 
     ret = virStoragePoolDefParseDoc(conn, ctxt, node);
 
+    xmlFreeParserCtxt (pctxt);
     xmlXPathFreeContext(ctxt);
     xmlFreeDoc(xml);
 
     return ret;
 
  cleanup:
+    xmlFreeParserCtxt (pctxt);
     xmlXPathFreeContext(ctxt);
     xmlFreeDoc(xml);
     return NULL;
@@ -725,15 +758,27 @@ virStorageVolDefParse(virConnectPtr conn,
                       const char *xmlStr,
                       const char *filename) {
     virStorageVolDefPtr ret = NULL;
+    xmlParserCtxtPtr pctxt;
     xmlDocPtr xml = NULL;
     xmlNodePtr node = NULL;
     xmlXPathContextPtr ctxt = NULL;
 
-    if (!(xml = xmlReadDoc(BAD_CAST xmlStr, filename ? filename : "storage.xml", NULL,
-                           XML_PARSE_NOENT | XML_PARSE_NONET |
-                           XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
-        virStorageReportError(conn, VIR_ERR_XML_ERROR,
-                              "%s", _("malformed xml document"));
+    /* 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;
+
+    if (conn) virResetError (&conn->err);
+    xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr,
+                          filename ? filename : "storage.xml", NULL,
+                          XML_PARSE_NOENT | XML_PARSE_NONET |
+                          XML_PARSE_NOWARNING);
+    if (!xml) {
+        if (conn && conn->err.code == VIR_ERR_NONE)
+              virStorageReportError(conn, VIR_ERR_XML_ERROR,
+                                    _("failed to parse xml document"));
         goto cleanup;
     }
 
@@ -753,12 +798,14 @@ virStorageVolDefParse(virConnectPtr conn,
 
     ret = virStorageVolDefParseDoc(conn, pool, ctxt, node);
 
+    xmlFreeParserCtxt (pctxt);
     xmlXPathFreeContext(ctxt);
     xmlFreeDoc(xml);
 
     return ret;
 
  cleanup:
+    xmlFreeParserCtxt (pctxt);
     xmlXPathFreeContext(ctxt);
     xmlFreeDoc(xml);
     return NULL;