[libvirt] [PATCH 3/3] maint: simplify lots of libxml2 clients
Daniel Veillard
veillard at redhat.com
Fri Aug 19 08:49:55 UTC 2011
On Thu, Aug 18, 2011 at 03:40:49PM -0600, Eric Blake wrote:
> Repetitive patterns should be factored. The sign of a good
> factorization is a change that kills 5x more lines than it adds :)
>
> * src/conf/domain_conf.c (virDomainDeviceDefParse)
> (virDomainSnapshotDefParseString): Use new convenience macros.
> * src/conf/storage_conf.c (virStoragePoolDefParseSourceString):
> Likewise.
> * src/cpu/cpu.c (cpuCompareXML, cpuBaselineXML): Likewise.
> * src/esx/esx_vi.c (esxVI_Context_Execute): Likewise.
> * src/qemu/qemu_migration.c (qemuMigrationCookieXMLParseStr):
> Likewise.
> * src/security/virt-aa-helper.c (caps_mockup): Likewise.
> * src/test/test_driver.c (testOpenFromFile): Likewise.
> * tests/cputest.c (cpuTestLoadXML, cpuTestLoadMultiXML):
> Likewise.
> * tools/virsh.c (cmdFreecell, makeCloneXML, cmdVNCDisplay)
> (cmdTTYConsole, cmdDetachInterface, cmdDetachDisk)
> (cmdSnapshotCreate, cmdSnapshotCreateAs, cmdSnapshotCurrent)
> (cmdSnapshotList, cmdSnapshotParent): Likewise.
> ---
> src/conf/domain_conf.c | 20 +--------
> src/conf/storage_conf.c | 8 +---
> src/cpu/cpu.c | 18 +-------
> src/esx/esx_vi.c | 13 +-----
> src/qemu/qemu_migration.c | 9 +----
> src/security/virt-aa-helper.c | 12 +-----
> src/test/test_driver.c | 13 +-----
> tests/cputest.c | 9 +---
> tools/virsh.c | 84 ++++++----------------------------------
> 9 files changed, 30 insertions(+), 156 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index ce1f3c5..0a2c9eb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -5385,17 +5385,10 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps,
> xmlXPathContextPtr ctxt = NULL;
> virDomainDeviceDefPtr dev = NULL;
>
> - if (!(xml = virXMLParseString(xmlStr, "device.xml"))) {
> + if (!(xml = virXMLParseStringCtxt(xmlStr, "device.xml", &ctxt))) {
> goto error;
> }
> - node = xmlDocGetRootElement(xml);
> -
> - ctxt = xmlXPathNewContext(xml);
> - if (ctxt == NULL) {
> - virReportOOMError();
> - goto error;
> - }
> - ctxt->node = xmlDocGetRootElement(xml);
> + node = ctxt->node;
>
> if (VIR_ALLOC(dev) < 0) {
> virReportOOMError();
> @@ -10968,23 +10961,16 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr,
> char *creation = NULL, *state = NULL;
> struct timeval tv;
>
> - xml = virXMLParse(NULL, xmlStr, "domainsnapshot.xml");
> + xml = virXMLParseCtxt(NULL, xmlStr, "domainsnapshot.xml", &ctxt);
> if (!xml) {
> return NULL;
> }
>
> - ctxt = xmlXPathNewContext(xml);
> - if (ctxt == NULL) {
> - virReportOOMError();
> - goto cleanup;
> - }
> -
> if (VIR_ALLOC(def) < 0) {
> virReportOOMError();
> goto cleanup;
> }
>
> - ctxt->node = xmlDocGetRootElement(xml);
> if (!xmlStrEqual(ctxt->node->name, BAD_CAST "domainsnapshot")) {
> virDomainReportError(VIR_ERR_XML_ERROR, "%s", _("domainsnapshot"));
> goto cleanup;
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 995f9a6..8d14e87 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -505,13 +505,7 @@ virStoragePoolDefParseSourceString(const char *srcSpec,
> xmlXPathContextPtr xpath_ctxt = NULL;
> virStoragePoolSourcePtr def = NULL, ret = NULL;
>
> - if (!(doc = virXMLParseString(srcSpec, "srcSpec.xml"))) {
> - goto cleanup;
> - }
> -
> - xpath_ctxt = xmlXPathNewContext(doc);
> - if (xpath_ctxt == NULL) {
> - virReportOOMError();
> + if (!(doc = virXMLParseStringCtxt(srcSpec, "srcSpec.xml", &xpath_ctxt))) {
> goto cleanup;
> }
>
> diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c
> index 16e0709..b47f078 100644
> --- a/src/cpu/cpu.c
> +++ b/src/cpu/cpu.c
> @@ -1,7 +1,7 @@
> /*
> * cpu.c: internal functions for CPU manipulation
> *
> - * Copyright (C) 2009--2010 Red Hat, Inc.
> + * Copyright (C) 2009-2011 Red Hat, Inc.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -76,16 +76,9 @@ cpuCompareXML(virCPUDefPtr host,
>
> VIR_DEBUG("host=%p, xml=%s", host, NULLSTR(xml));
>
> - if (!(doc = virXMLParseString(xml, "cpu.xml")))
> + if (!(doc = virXMLParseStringCtxt(xml, "cpu.xml", &ctxt)))
> goto cleanup;
>
> - if ((ctxt = xmlXPathNewContext(doc)) == NULL) {
> - virReportOOMError();
> - goto cleanup;
> - }
> -
> - ctxt->node = xmlDocGetRootElement(doc);
> -
> cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO);
> if (cpu == NULL)
> goto cleanup;
> @@ -311,14 +304,9 @@ cpuBaselineXML(const char **xmlCPUs,
> goto no_memory;
>
> for (i = 0; i < ncpus; i++) {
> - if (!(doc = virXMLParseString(xmlCPUs[i], "cpu.xml")))
> + if (!(doc = virXMLParseStringCtxt(xmlCPUs[i], "cpu.xml", &ctxt)))
> goto error;
>
> - if ((ctxt = xmlXPathNewContext(doc)) == NULL)
> - goto no_memory;
> -
> - ctxt->node = xmlDocGetRootElement(doc);
> -
> cpus[i] = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_HOST);
> if (cpus[i] == NULL)
> goto error;
> diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
> index c82094b..5c8d79e 100644
> --- a/src/esx/esx_vi.c
> +++ b/src/esx/esx_vi.c
> @@ -910,21 +910,14 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName,
> (*response)->content = virBufferContentAndReset(&buffer);
>
> if ((*response)->responseCode == 500 || (*response)->responseCode == 200) {
> - (*response)->document = virXMLParseString((*response)->content,
> - "esx.xml");
> + (*response)->document = virXMLParseStringCtxt((*response)->content,
> + "esx.xml",
> + &xpathContext);
>
> if ((*response)->document == NULL) {
> goto cleanup;
> }
>
> - xpathContext = xmlXPathNewContext((*response)->document);
> -
> - if (xpathContext == NULL) {
> - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Could not create XPath context"));
> - goto cleanup;
> - }
> -
> xmlXPathRegisterNs(xpathContext, BAD_CAST "soapenv",
> BAD_CAST "http://schemas.xmlsoap.org/soap/envelope/");
> xmlXPathRegisterNs(xpathContext, BAD_CAST "vim", BAD_CAST "urn:vim25");
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 5510493..a84faf6 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -602,16 +602,9 @@ qemuMigrationCookieXMLParseStr(qemuMigrationCookiePtr mig,
>
> VIR_DEBUG("xml=%s", NULLSTR(xml));
>
> - if (!(doc = virXMLParseString(xml, "qemumigration.xml")))
> + if (!(doc = virXMLParseStringCtxt(xml, "qemumigration.xml", &ctxt)))
> goto cleanup;
>
> - if ((ctxt = xmlXPathNewContext(doc)) == NULL) {
> - virReportOOMError();
> - goto cleanup;
> - }
> -
> - ctxt->node = xmlDocGetRootElement(doc);
> -
> ret = qemuMigrationCookieXMLParse(mig, ctxt, flags);
>
> cleanup:
> diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
> index 1e2feae..bb577d3 100644
> --- a/src/security/virt-aa-helper.c
> +++ b/src/security/virt-aa-helper.c
> @@ -640,24 +640,16 @@ caps_mockup(vahControl * ctl, const char *xmlStr)
> int rc = -1;
> xmlDocPtr xml = NULL;
> xmlXPathContextPtr ctxt = NULL;
> - xmlNodePtr root;
>
> - if (!(xml = virXMLParseString(xmlStr, "domain.xml"))) {
> + if (!(xml = virXMLParseStringCtxt(xmlStr, "domain.xml", &ctxt))) {
> goto cleanup;
> }
>
> - root = xmlDocGetRootElement(xml);
> - if (!xmlStrEqual(root->name, BAD_CAST "domain")) {
> + if (!xmlStrEqual(ctxt->node->name, BAD_CAST "domain")) {
> vah_error(NULL, 0, _("incorrect root element"));
> goto cleanup;
> }
>
> - if ((ctxt = xmlXPathNewContext(xml)) == NULL) {
> - vah_error(ctl, 0, _("could not allocate memory"));
> - goto cleanup;
> - }
> - ctxt->node = root;
> -
> /* Quick sanity check for some required elements */
> if (verify_xpath_context(ctxt) != 0)
> goto cleanup;
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index fb14b10..544fd7e 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -742,7 +742,6 @@ static int testOpenFromFile(virConnectPtr conn,
> long l;
> char *str;
> xmlDocPtr xml = NULL;
> - xmlNodePtr root = NULL;
> xmlNodePtr *domains = NULL, *networks = NULL, *ifaces = NULL,
> *pools = NULL, *devs = NULL;
> xmlXPathContextPtr ctxt = NULL;
> @@ -771,24 +770,16 @@ static int testOpenFromFile(virConnectPtr conn,
> if (!(privconn->caps = testBuildCapabilities(conn)))
> goto error;
>
> - if (!(xml = virXMLParseFile(file))) {
> + if (!(xml = virXMLParseFileCtxt(file, &ctxt))) {
> goto error;
> }
>
> - root = xmlDocGetRootElement(xml);
> - if ((root == NULL) || (!xmlStrEqual(root->name, BAD_CAST "node"))) {
> + if (!xmlStrEqual(ctxt->node->name, BAD_CAST "node")) {
> testError(VIR_ERR_XML_ERROR, "%s",
> _("Root element is not 'node'"));
> goto error;
> }
>
> - ctxt = xmlXPathNewContext(xml);
> - if (ctxt == NULL) {
> - testError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("creating xpath context"));
> - goto error;
> - }
> -
> privconn->nextDomID = 1;
> privconn->numCells = 0;
> if ((privconn->path = strdup(file)) == NULL) {
> diff --git a/tests/cputest.c b/tests/cputest.c
> index 4dcf1aa..b5272c1 100644
> --- a/tests/cputest.c
> +++ b/tests/cputest.c
> @@ -96,11 +96,9 @@ cpuTestLoadXML(const char *arch, const char *name)
> if (virAsprintf(&xml, "%s/cputestdata/%s-%s.xml", abs_srcdir, arch, name) < 0)
> goto cleanup;
>
> - if (!(doc = virXMLParseFile(xml)) ||
> - !(ctxt = xmlXPathNewContext(doc)))
> + if (!(doc = virXMLParseFileCtxt(xml, &ctxt)))
> goto cleanup;
>
> - ctxt->node = xmlDocGetRootElement(doc);
> cpu = virCPUDefParseXML(ctxt->node, ctxt, VIR_CPU_TYPE_AUTO);
>
> cleanup:
> @@ -127,12 +125,9 @@ cpuTestLoadMultiXML(const char *arch,
> if (virAsprintf(&xml, "%s/cputestdata/%s-%s.xml", abs_srcdir, arch, name) < 0)
> goto cleanup;
>
> - if (!(doc = virXMLParseFile(xml)) ||
> - !(ctxt = xmlXPathNewContext(doc)))
> + if (!(doc = virXMLParseFileCtxt(xml, &ctxt)))
> goto error;
>
> - ctxt->node = xmlDocGetRootElement(doc);
> -
> n = virXPathNodeSet("/cpuTest/cpu", ctxt, &nodes);
> if (n <= 0 || !(cpus = calloc(n, sizeof(virCPUDefPtr))))
> goto error;
> diff --git a/tools/virsh.c b/tools/virsh.c
> index c43de4c..1c67150 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -67,6 +67,8 @@ static char *progname;
> #define VSH_PROMPT_RW "virsh # "
> #define VSH_PROMPT_RO "virsh > "
>
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +
> #define GETTIMEOFDAY(T) gettimeofday(T, NULL)
> #define DIFF_MSEC(T, U) \
> ((((int) ((T)->tv_sec - (U)->tv_sec)) * 1000000.0 + \
> @@ -2901,16 +2903,11 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
> goto cleanup;
> }
>
> - xml = xmlReadDoc((const xmlChar *) cap_xml, "node.xml", NULL,
> - XML_PARSE_NOENT | XML_PARSE_NONET |
> - XML_PARSE_NOWARNING);
> -
> + xml = virXMLParseStringCtxt(cap_xml, "node.xml", &ctxt);
> if (!xml) {
> vshError(ctl, "%s", _("unable to get node capabilities"));
> goto cleanup;
> }
> -
> - ctxt = xmlXPathNewContext(xml);
> nodes_cnt = virXPathNodeSet("/capabilities/host/topology/cells/cell",
> ctxt, &nodes);
>
> @@ -8404,13 +8401,9 @@ makeCloneXML(const char *origxml, const char *newname) {
> xmlChar *newxml = NULL;
> int size;
>
> - doc = xmlReadDoc((const xmlChar *) origxml, "domain.xml", NULL,
> - XML_PARSE_NOENT | XML_PARSE_NONET | XML_PARSE_NOWARNING);
> + doc = virXMLParseStringCtxt(origxml, "domain.xml", &ctxt);
> if (!doc)
> goto cleanup;
> - ctxt = xmlXPathNewContext(doc);
> - if (!ctxt)
> - goto cleanup;
>
> obj = xmlXPathEval(BAD_CAST "/volume/name", ctxt);
> if ((obj == NULL) || (obj->nodesetval == NULL) ||
> @@ -10199,15 +10192,10 @@ cmdVNCDisplay(vshControl *ctl, const vshCmd *cmd)
> if (!doc)
> goto cleanup;
>
> - xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL,
> - XML_PARSE_NOENT | XML_PARSE_NONET |
> - XML_PARSE_NOWARNING);
> + xml = virXMLParseStringCtxt(doc, "domain.xml", &ctxt);
> VIR_FREE(doc);
> if (!xml)
> goto cleanup;
> - ctxt = xmlXPathNewContext(xml);
> - if (!ctxt)
> - goto cleanup;
>
> obj = xmlXPathEval(BAD_CAST "string(/domain/devices/graphics[@type='vnc']/@port)", ctxt);
> if ((obj == NULL) || (obj->type != XPATH_STRING) ||
> @@ -10272,15 +10260,10 @@ cmdTTYConsole(vshControl *ctl, const vshCmd *cmd)
> if (!doc)
> goto cleanup;
>
> - xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL,
> - XML_PARSE_NOENT | XML_PARSE_NONET |
> - XML_PARSE_NOWARNING);
> + xml = virXMLParseStringCtxt(doc, "domain.xml", &ctxt);
> VIR_FREE(doc);
> if (!xml)
> goto cleanup;
> - ctxt = xmlXPathNewContext(xml);
> - if (!ctxt)
> - goto cleanup;
>
> obj = xmlXPathEval(BAD_CAST "string(/domain/devices/console/@tty)", ctxt);
> if ((obj == NULL) || (obj->type != XPATH_STRING) ||
> @@ -10664,19 +10647,12 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
> if (!doc)
> goto cleanup;
>
> - xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL,
> - XML_PARSE_NOENT | XML_PARSE_NONET |
> - XML_PARSE_NOWARNING);
> + xml = virXMLParseStringCtxt(doc, "domain.xml", &ctxt);
> VIR_FREE(doc);
> if (!xml) {
> vshError(ctl, "%s", _("Failed to get interface information"));
> goto cleanup;
> }
> - ctxt = xmlXPathNewContext(xml);
> - if (!ctxt) {
> - vshError(ctl, "%s", _("Failed to get interface information"));
> - goto cleanup;
> - }
>
> snprintf(buf, sizeof(buf), "/domain/devices/interface[@type='%s']", type);
> obj = xmlXPathEval(BAD_CAST buf, ctxt);
> @@ -11138,19 +11114,12 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
> if (!doc)
> goto cleanup;
>
> - xml = xmlReadDoc((const xmlChar *) doc, "domain.xml", NULL,
> - XML_PARSE_NOENT | XML_PARSE_NONET |
> - XML_PARSE_NOWARNING);
> + xml = virXMLParseStringCtxt(doc, "domain.xml", &ctxt);
> VIR_FREE(doc);
> if (!xml) {
> vshError(ctl, "%s", _("Failed to get disk information"));
> goto cleanup;
> }
> - ctxt = xmlXPathNewContext(xml);
> - if (!ctxt) {
> - vshError(ctl, "%s", _("Failed to get disk information"));
> - goto cleanup;
> - }
>
> obj = xmlXPathEval(BAD_CAST "/domain/devices/disk", ctxt);
> if ((obj == NULL) || (obj->type != XPATH_NODESET) ||
> @@ -11865,14 +11834,9 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd)
> if (!doc)
> goto cleanup;
>
> - xml = xmlReadDoc((const xmlChar *) doc, "domainsnapshot.xml", NULL,
> - XML_PARSE_NOENT | XML_PARSE_NONET |
> - XML_PARSE_NOWARNING);
> + xml = virXMLParseStringCtxt(doc, "domainsnapshot.xml", &ctxt);
> if (!xml)
> goto cleanup;
> - ctxt = xmlXPathNewContext(xml);
> - if (!ctxt)
> - goto cleanup;
>
> name = virXPathString("string(/domainsnapshot/name)", ctxt);
> if (!name) {
> @@ -11974,14 +11938,9 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd)
> if (!doc)
> goto cleanup;
>
> - xml = xmlReadDoc((const xmlChar *) doc, "domainsnapshot.xml", NULL,
> - XML_PARSE_NOENT | XML_PARSE_NONET |
> - XML_PARSE_NOWARNING);
> + xml = virXMLParseStringCtxt(doc, "domainsnapshot.xml", &ctxt);
> if (!xml)
> goto cleanup;
> - ctxt = xmlXPathNewContext(xml);
> - if (!ctxt)
> - goto cleanup;
>
> parsed_name = virXPathString("string(/domainsnapshot/name)", ctxt);
> if (!parsed_name) {
> @@ -12056,16 +12015,9 @@ cmdSnapshotCurrent(vshControl *ctl, const vshCmd *cmd)
> xmlDocPtr xmldoc = NULL;
> xmlXPathContextPtr ctxt = NULL;
>
> - xmldoc = xmlReadDoc((const xmlChar *) xml, "domainsnapshot.xml",
> - NULL, XML_PARSE_NOENT | XML_PARSE_NONET |
> - XML_PARSE_NOWARNING);
> + xmldoc = virXMLParseStringCtxt(xml, "domainsnapshot.xml", &ctxt);
> if (!xmldoc)
> goto cleanup;
> - ctxt = xmlXPathNewContext(xmldoc);
> - if (!ctxt) {
> - xmlFreeDoc(xmldoc);
> - goto cleanup;
> - }
>
> name = virXPathString("string(/domainsnapshot/name)", ctxt);
> xmlXPathFreeContext(ctxt);
> @@ -12165,14 +12117,9 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd)
> if (!doc)
> continue;
>
> - xml = xmlReadDoc((const xmlChar *) doc, "domainsnapshot.xml", NULL,
> - XML_PARSE_NOENT | XML_PARSE_NONET |
> - XML_PARSE_NOWARNING);
> + xml = virXMLParseStringCtxt(doc, "domainsnapshot.xml", &ctxt);
> if (!xml)
> continue;
> - ctxt = xmlXPathNewContext(xml);
> - if (!ctxt)
> - continue;
>
> state = virXPathString("string(/domainsnapshot/state)", ctxt);
> if (state == NULL)
> @@ -12312,14 +12259,9 @@ cmdSnapshotParent(vshControl *ctl, const vshCmd *cmd)
> if (!xml)
> goto cleanup;
>
> - xmldoc = xmlReadDoc((const xmlChar *) xml, "domainsnapshot.xml", NULL,
> - XML_PARSE_NOENT | XML_PARSE_NONET |
> - XML_PARSE_NOWARNING);
> + xmldoc = virXMLParseStringCtxt(xml, "domainsnapshot.xml", &ctxt);
> if (!xmldoc)
> goto cleanup;
> - ctxt = xmlXPathNewContext(xmldoc);
> - if (!ctxt)
> - goto cleanup;
>
> parent = virXPathString("string(/domainsnapshot/parent/name)", ctxt);
> if (!parent)
ACK, okay it does remove lines :-)
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel at veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
More information about the libvir-list
mailing list