From cf11bb2249cf996a195f9f311b5584edd4f38129 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Fri, 1 Aug 2008 09:39:44 +0000 Subject: [PATCH] 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 | 8 +++++ src/domain_conf.c | 82 +++++++++++++++++++++++++++++++++---------- src/network_conf.c | 87 +++++++++++++++++++++++++++++++++++----------- src/storage_conf.c | 71 ++++++++++++++++++++++++++++++------- 4 files changed, 198 insertions(+), 50 deletions(-) diff --git a/ChangeLog b/ChangeLog index b5a1100634..9c73ea799c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +Fri Aug 1 10:38:00 BST 2008 Richard W.M. Jones + + 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 * docs/formatdomain.html docs/formatdomain.html.in docs/libvirt-api.xml diff --git a/src/domain_conf.c b/src/domain_conf.c index 30a40eb819..0e99ccc5e1 100644 --- 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 39ec00aed3..10c9dca9e4 100644 --- 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 1880886688..05b68af15b 100644 --- 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;