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.

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

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,
+    /* Set up a parser context so we can catch the details of XML errors. */
-                           XML_PARSE_NOENT | XML_PARSE_NONET |
+    pctxt = xmlNewParserCtxt ();
-                           XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
+    if (!pctxt || !pctxt->sax)
-        virDomainReportError(conn, VIR_ERR_XML_ERROR, NULL);
+        goto cleanup;
-        return NULL;
+    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);
+        goto cleanup;
-        return NULL;
     }
 
     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,
+    /* Set up a parser context so we can catch the details of XML errors. */
-                            XML_PARSE_NOENT | XML_PARSE_NONET |
+    pctxt = xmlNewParserCtxt ();
-                            XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
+    if (!pctxt || !pctxt->sax)
-        virDomainReportError(conn, VIR_ERR_XML_ERROR, NULL);
+        goto cleanup;
-        return NULL;
+    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);
+        goto cleanup;
-        return NULL;
     }
 
     def = virDomainDefParseNode(conn, caps, xml, root);
 
-    xmlFreeDoc(xml);
+cleanup:
+    xmlFreeParserCtxt (pctxt);
+    xmlFreeDoc (xml);
     return def;
 }
 

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,
+    /* Set up a parser context so we can catch the details of XML errors. */
-                           XML_PARSE_NOENT | XML_PARSE_NONET |
+    pctxt = xmlNewParserCtxt ();
-                           XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
+    if (!pctxt || !pctxt->sax)
-        virNetworkReportError(conn, VIR_ERR_XML_ERROR, NULL);
+        goto cleanup;
-        return NULL;
+    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);
+        goto cleanup;
-        return NULL;
     }
 
     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,
+    /* Set up a parser context so we can catch the details of XML errors. */
-                            XML_PARSE_NOENT | XML_PARSE_NONET |
+    pctxt = xmlNewParserCtxt ();
-                            XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
+    if (!pctxt || !pctxt->sax)
-        virNetworkReportError(conn, VIR_ERR_XML_ERROR, NULL);
+        goto cleanup;
-        return NULL;
+    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);
+        goto cleanup;
-        return NULL;
     }
 
     def = virNetworkDefParseNode(conn, xml, root);
 
-    xmlFreeDoc(xml);
+cleanup:
+    xmlFreeParserCtxt (pctxt);
+    xmlFreeDoc (xml);
     return def;
 }
 

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,
+    /* Set up a parser context so we can catch the details of XML errors. */
-                           filename ? filename : "storage.xml",
+    pctxt = xmlNewParserCtxt ();
-                           NULL,
+    if (!pctxt || !pctxt->sax)
-                           XML_PARSE_NOENT | XML_PARSE_NONET |
+        goto cleanup;
-                           XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
+    pctxt->sax->error = catchXMLError;
-        virStorageReportError(conn, VIR_ERR_XML_ERROR,
+    pctxt->_private = conn;
-                              "%s", _("malformed xml document"));
+
+    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,
+    /* Set up a parser context so we can catch the details of XML errors. */
-                           XML_PARSE_NOENT | XML_PARSE_NONET |
+    pctxt = xmlNewParserCtxt ();
-                           XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
+    if (!pctxt || !pctxt->sax)
-        virStorageReportError(conn, VIR_ERR_XML_ERROR,
+        goto cleanup;
-                              "%s", _("malformed xml document"));
+    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;