[libvirt] [PATCH] Detailed XML errors when XML is not well-formed
Richard W.M. Jones
rjones at redhat.com
Wed Jul 30 14:49:17 UTC 2008
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.
(2) The XML parser routinely produces multiple error messages, and
virterror throws away all but the last one.
The errors do, however, get printed to stderr.
You can test this by using 'virsh define', 'virsh net-define', 'virsh
pool-define', etc. on not-well-formed XML files.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
-------------- next part --------------
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 14:38:51 -0000
@@ -1838,32 +1838,61 @@
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) {
+ virDomainReportError (NULL, 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;
+
+ /* It'd be nice to do this, but seems ->userData field is used
+ * by something else.
+ */
+ /*pctxt->userData = conn;*/
+
+ 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).
+ */
+ 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 +1900,43 @@
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;
+
+ /* It'd be nice to do this, but seems ->userData field is used
+ * by something else.
+ */
+ /*pctxt->userData = conn;*/
+
+ xml = xmlCtxtReadFile (pctxt, filename, NULL,
+ XML_PARSE_NOENT | XML_PARSE_NONET |
+ XML_PARSE_NOWARNING);
+ if (!xml) {
+ /* virDomainReportError has already been called by the
+ * callback function (hopefully).
+ */
+ 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;
}
Index: src/network_conf.c
===================================================================
RCS file: /data/cvs/libvirt/src/network_conf.c,v
retrieving revision 1.3
diff -u -r1.3 network_conf.c
--- src/network_conf.c 25 Jul 2008 14:27:25 -0000 1.3
+++ src/network_conf.c 30 Jul 2008 14:38:51 -0000
@@ -348,57 +348,103 @@
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) {
+ virNetworkReportError (NULL, VIR_ERR_XML_DETAIL,
+ ctxt->lastError.message, ctxt->lastError.line);
+ }
+}
+
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;
+
+ /* It'd be nice to do this, but seems ->userData field is used
+ * by something else.
+ */
+ /*pctxt->userData = conn;*/
+
+ xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr, "network.xml", NULL,
+ XML_PARSE_NOENT | XML_PARSE_NONET |
+ XML_PARSE_NOWARNING);
+ if (!xml) {
+ /* virNetworkReportError has already been called by the
+ * callback function (hopefully).
+ */
+ 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;
+
+ /* It'd be nice to do this, but seems ->userData field is used
+ * by something else.
+ */
+ /*pctxt->userData = conn;*/
+
+ xml = xmlCtxtReadFile (pctxt, filename, NULL,
+ XML_PARSE_NOENT | XML_PARSE_NONET |
+ XML_PARSE_NOWARNING);
+ if (!xml) {
+ /* virNetworkReportError has already been called by the
+ * callback function (hopefully).
+ */
+ 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;
}
Index: src/storage_conf.c
===================================================================
RCS file: /data/cvs/libvirt/src/storage_conf.c,v
retrieving revision 1.10
diff -u -r1.10 storage_conf.c
--- src/storage_conf.c 25 Jul 2008 14:27:25 -0000 1.10
+++ src/storage_conf.c 30 Jul 2008 14:38:52 -0000
@@ -361,22 +361,50 @@
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) {
+ virStorageReportError (NULL, VIR_ERR_XML_DETAIL,
+ ctxt->lastError.message, ctxt->lastError.line);
+ }
+}
+
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;
+
+ /* It'd be nice to do this, but seems ->userData field is used
+ * by something else.
+ */
+ /*pctxt->userData = conn;*/
+
+ xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr,
+ filename ? filename : "storage.xml",
+ NULL,
+ XML_PARSE_NOENT | XML_PARSE_NONET |
+ XML_PARSE_NOWARNING);
+ if (!xml) {
+ /* virStorageReportError has already been called by the
+ * callback function (hopefully).
+ */
goto cleanup;
}
@@ -396,12 +424,14 @@
ret = virStoragePoolDefParseDoc(conn, ctxt, node);
+ xmlFreeParserCtxt (pctxt);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
return ret;
cleanup:
+ xmlFreeParserCtxt (pctxt);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
return NULL;
@@ -725,15 +755,30 @@
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;
+
+ /* It'd be nice to do this, but seems ->userData field is used
+ * by something else.
+ */
+ /*pctxt->userData = conn;*/
+
+ xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr,
+ filename ? filename : "storage.xml", NULL,
+ XML_PARSE_NOENT | XML_PARSE_NONET |
+ XML_PARSE_NOWARNING);
+ if (!xml) {
+ /* virStorageReportError has already been called by the
+ * callback function (hopefully).
+ */
goto cleanup;
}
@@ -753,12 +798,14 @@
ret = virStorageVolDefParseDoc(conn, pool, ctxt, node);
+ xmlFreeParserCtxt (pctxt);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
return ret;
cleanup:
+ xmlFreeParserCtxt (pctxt);
xmlXPathFreeContext(ctxt);
xmlFreeDoc(xml);
return NULL;
More information about the libvir-list
mailing list