May 2011 - Devel - libvirt List Archives

[libvirt] [PATCH] xml: Use virXMLParse* helpers everywhere
by Cole Robinson 13 May '11

13 May '11

virt-aa-helper isn't even compile tested since I don't have the setup for it. Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/conf/domain_conf.c | 33 ++++-------------- src/conf/nwfilter_conf.c | 76 ++++------------------------------------- src/conf/storage_conf.c | 8 +---- src/esx/esx_vi.c | 14 +------ src/security/virt-aa-helper.c | 43 +---------------------- src/test/test_driver.c | 17 +-------- 6 files changed, 21 insertions(+), 170 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f7e4959..a0eb43e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4877,26 +4877,17 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, xmlXPathContextPtr ctxt = NULL; virDomainDeviceDefPtr dev = NULL; - if (!(xml = xmlReadDoc(BAD_CAST xmlStr, "device.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) { - virDomainReportError(VIR_ERR_XML_ERROR, NULL); + if (!(xml = virXMLParseString(xmlStr, "device.xml"))) { goto error; } - node = xmlDocGetRootElement(xml); - if (node == NULL) { - virDomainReportError(VIR_ERR_XML_ERROR, - "%s", _("missing root element")); - goto error; - } ctxt = xmlXPathNewContext(xml); if (ctxt == NULL) { virReportOOMError(); goto error; } - ctxt->node = node; + ctxt->node = xmlDocGetRootElement(xml); if (VIR_ALLOC(dev) < 0) { virReportOOMError(); @@ -9048,7 +9039,6 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, { xmlXPathContextPtr ctxt = NULL; xmlDocPtr xml = NULL; - xmlNodePtr root; virDomainSnapshotDefPtr def = NULL; virDomainSnapshotDefPtr ret = NULL; char *creation = NULL, *state = NULL; @@ -9056,22 +9046,9 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, xml = virXMLParse(NULL, xmlStr, "domainsnapshot.xml"); if (!xml) { - virDomainReportError(VIR_ERR_XML_ERROR, - "%s",_("failed to parse snapshot xml document")); return NULL; } - if ((root = xmlDocGetRootElement(xml)) == NULL) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("missing root element")); - goto cleanup; - } - - if (!xmlStrEqual(root->name, BAD_CAST "domainsnapshot")) { - virDomainReportError(VIR_ERR_XML_ERROR, "%s", _("domainsnapshot")); - goto cleanup; - } - ctxt = xmlXPathNewContext(xml); if (ctxt == NULL) { virReportOOMError(); @@ -9083,7 +9060,11 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, goto cleanup; } - ctxt->node = root; + ctxt->node = xmlDocGetRootElement(xml); + if (!xmlStrEqual(ctxt->node->name, BAD_CAST "domainsnapshot")) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", _("domainsnapshot")); + goto cleanup; + } gettimeofday(&tv, NULL); diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index eb75bad..a32bdb3 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2066,28 +2066,6 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) { } -/* 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) { - virNWFilterReportError(VIR_ERR_XML_DETAIL, - _("at line %d: %s"), - ctxt->lastError.line, - ctxt->lastError.message); - } - } -} - - virNWFilterDefPtr virNWFilterDefParseNode(xmlDocPtr xml, xmlNodePtr root) { @@ -2117,58 +2095,18 @@ cleanup: static virNWFilterDefPtr -virNWFilterDefParse(virConnectPtr conn, +virNWFilterDefParse(virConnectPtr conn ATTRIBUTE_UNUSED, const char *xmlStr, const char *filename) { - virNWFilterDefPtr ret = NULL; - xmlParserCtxtPtr pctxt; - xmlDocPtr xml = NULL; - xmlNodePtr node = 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); - if (filename) { - xml = xmlCtxtReadFile (pctxt, filename, NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - } else { - xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr, - "nwfilter.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - } - - if (!xml) { - if (conn && conn->err.code == VIR_ERR_NONE) - virNWFilterReportError(VIR_ERR_XML_ERROR, - "%s",_("failed to parse xml document")); - goto cleanup; - } + virNWFilterDefPtr def = NULL; + xmlDocPtr xml; - node = xmlDocGetRootElement(xml); - if (node == NULL) { - virNWFilterReportError(VIR_ERR_XML_ERROR, - "%s", _("missing root element")); - goto cleanup; + if ((xml = virXMLParse(filename, xmlStr, "nwfilter.xml"))) { + def = virNWFilterDefParseNode(xml, xmlDocGetRootElement(xml)); + xmlFreeDoc(xml); } - ret = virNWFilterDefParseNode(xml, node); - - xmlFreeParserCtxt (pctxt); - xmlFreeDoc(xml); - - return ret; - - cleanup: - xmlFreeParserCtxt (pctxt); - xmlFreeDoc(xml); - return NULL; + return def; } diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index ed7d300..ca86f19 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; - doc = xmlReadDoc((const xmlChar *)srcSpec, "srcSpec.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOERROR | XML_PARSE_NOWARNING); - - if (doc == NULL) { - virStorageReportError(VIR_ERR_XML_ERROR, - "%s", _("bad <source> spec")); + if (!(doc = virXMLParseString(srcSpec, "srcSpec.xml"))) { goto cleanup; } diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c index ca5376e..381b547 100644 --- a/src/esx/esx_vi.c +++ b/src/esx/esx_vi.c @@ -894,20 +894,10 @@ esxVI_Context_Execute(esxVI_Context *ctx, const char *methodName, (*response)->content = virBufferContentAndReset(&buffer); if ((*response)->responseCode == 500 || (*response)->responseCode == 200) { - (*response)->document = xmlReadDoc(BAD_CAST (*response)->content, "", - NULL, XML_PARSE_NONET); + (*response)->document = virXMLParseString((*response)->content, + "esx.xml"); if ((*response)->document == NULL) { - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Response for call to '%s' could not be parsed"), - methodName); - goto cleanup; - } - - if (xmlDocGetRootElement((*response)->document) == NULL) { - ESX_VI_ERROR(VIR_ERR_INTERNAL_ERROR, - _("Response for call to '%s' is an empty XML document"), - methodName); goto cleanup; } diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e481095..a14fb77 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -592,29 +592,6 @@ valid_path(const char *path, const bool readonly) return 0; } -/* 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) { - if (virGetLastError() == NULL && - ctxt->lastError.level == XML_ERR_FATAL && - ctxt->lastError.message != NULL) { - char *err_str = NULL; - if (virAsprintf(&err_str, "XML error at line %d: %s", - ctxt->lastError.line, - ctxt->lastError.message) == -1) - vah_error(NULL, 0, _("could not get XML error")); - else { - vah_error(NULL, 0, err_str); - VIR_FREE(err_str); - } - } - } -} - static int verify_xpath_context(xmlXPathContextPtr ctxt) { @@ -658,31 +635,15 @@ static int caps_mockup(vahControl * ctl, const char *xmlStr) { int rc = -1; - xmlParserCtxtPtr pctxt = NULL; xmlDocPtr xml = NULL; xmlXPathContextPtr ctxt = NULL; xmlNodePtr root; - /* 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; - - xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr, "domain.xml", NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOWARNING); - if (!xml) { - if (virGetLastError() == NULL) - vah_error(NULL, 0, _("failed to parse xml document")); - goto cleanup; - } - - if ((root = xmlDocGetRootElement(xml)) == NULL) { - vah_error(NULL, 0, _("missing root element")); + if (!(xml = virXMLParseString(xmlStr, "domain.xml"))) { goto cleanup; } + root = xmlDocGetRootElement(xml); if (!xmlStrEqual(root->name, BAD_CAST "domain")) { vah_error(NULL, 0, _("incorrect root element")); goto cleanup; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 58ff250..119b027 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -746,7 +746,7 @@ error: static int testOpenFromFile(virConnectPtr conn, const char *file) { - int fd = -1, i, ret; + int i, ret; long l; char *str; xmlDocPtr xml = NULL; @@ -779,22 +779,10 @@ static int testOpenFromFile(virConnectPtr conn, if (!(privconn->caps = testBuildCapabilities(conn))) goto error; - if ((fd = open(file, O_RDONLY)) < 0) { - virReportSystemError(errno, - _("loading host definition file '%s'"), - file); + if (!(xml = virXMLParseFile(file))) { goto error; } - if (!(xml = xmlReadFd(fd, file, NULL, - XML_PARSE_NOENT | XML_PARSE_NONET | - XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) { - testError(VIR_ERR_INTERNAL_ERROR, - _("Invalid XML in file '%s'"), file); - goto error; - } - VIR_FORCE_CLOSE(fd); - root = xmlDocGetRootElement(xml); if ((root == NULL) || (!xmlStrEqual(root->name, BAD_CAST "node"))) { testError(VIR_ERR_XML_ERROR, "%s", @@ -1100,7 +1088,6 @@ static int testOpenFromFile(virConnectPtr conn, VIR_FREE(networks); VIR_FREE(ifaces); VIR_FREE(pools); - VIR_FORCE_CLOSE(fd); virDomainObjListDeinit(&privconn->domains); virNetworkObjListFree(&privconn->networks); virInterfaceObjListFree(&privconn->ifaces); -- 1.7.4.4

2 2

[libvirt] [PATCH] xml: Make sure virXpathNodeSet always sets an error
by Cole Robinson 13 May '11

13 May '11

And update callers to actually respect the error Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/conf/domain_conf.c | 38 ------------------------------------ src/conf/interface_conf.c | 15 ++++++++++--- src/conf/network_conf.c | 3 ++ src/conf/node_device_conf.c | 12 ++++------ src/conf/storage_conf.c | 3 ++ src/conf/storage_encryption_conf.c | 2 - src/qemu/qemu_domain.c | 2 - src/test/test_driver.c | 7 ------ src/util/xml.c | 7 +++++- 9 files changed, 28 insertions(+), 61 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d3efec6..9ab9a5a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5228,8 +5228,6 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, /* analysis of the boot devices */ if ((n = virXPathNodeSet("./os/boot", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract boot device")); goto cleanup; } @@ -5517,8 +5515,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, def->cputune.shares = 0; if ((n = virXPathNodeSet("./cputune/vcpupin", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract vcpupin nodes")); goto error; } @@ -5613,8 +5609,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } if ((n = virXPathNodeSet("./clock/timer", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to parse timers")); goto error; } if (n && VIR_ALLOC_N(def->clock.timers, n) < 0) @@ -5742,8 +5736,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the disk devices */ if ((n = virXPathNodeSet("./devices/disk", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract disk devices")); goto error; } if (n && VIR_ALLOC_N(def->disks, n) < 0) @@ -5762,8 +5754,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the controller devices */ if ((n = virXPathNodeSet("./devices/controller", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract controller devices")); goto error; } if (n && VIR_ALLOC_N(def->controllers, n) < 0) @@ -5780,8 +5770,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the filesystems */ if ((n = virXPathNodeSet("./devices/filesystem", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract filesystem devices")); goto error; } if (n && VIR_ALLOC_N(def->fss, n) < 0) @@ -5798,8 +5786,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the network devices */ if ((n = virXPathNodeSet("./devices/interface", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract network devices")); goto error; } if (n && VIR_ALLOC_N(def->nets, n) < 0) @@ -5820,8 +5806,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the smartcard devices */ if ((n = virXPathNodeSet("./devices/smartcard", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract smartcard devices")); goto error; } if (n && VIR_ALLOC_N(def->smartcards, n) < 0) @@ -5840,8 +5824,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the character devices */ if ((n = virXPathNodeSet("./devices/parallel", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract parallel devices")); goto error; } if (n && VIR_ALLOC_N(def->parallels, n) < 0) @@ -5868,8 +5850,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, VIR_FREE(nodes); if ((n = virXPathNodeSet("./devices/serial", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract serial devices")); goto error; } if (n && VIR_ALLOC_N(def->serials, n) < 0) @@ -5926,8 +5906,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, } if ((n = virXPathNodeSet("./devices/channel", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract channel devices")); goto error; } if (n && VIR_ALLOC_N(def->channels, n) < 0) @@ -5967,8 +5945,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the input devices */ if ((n = virXPathNodeSet("./devices/input", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract input devices")); goto error; } if (n && VIR_ALLOC_N(def->inputs, n) < 0) @@ -6001,8 +5977,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the graphics devices */ if ((n = virXPathNodeSet("./devices/graphics", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract graphics devices")); goto error; } if (n && VIR_ALLOC_N(def->graphics, n) < 0) @@ -6044,8 +6018,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the sound devices */ if ((n = virXPathNodeSet("./devices/sound", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract sound devices")); goto error; } if (n && VIR_ALLOC_N(def->sounds, n) < 0) @@ -6062,8 +6034,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the video devices */ if ((n = virXPathNodeSet("./devices/video", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract video devices")); goto error; } if (n && VIR_ALLOC_N(def->videos, n) < 0) @@ -6102,8 +6072,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the host devices */ if ((n = virXPathNodeSet("./devices/hostdev", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract host devices")); goto error; } if (n && VIR_ALLOC_N(def->hostdevs, n) < 0) @@ -6122,8 +6090,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the watchdog devices */ def->watchdog = NULL; if ((n = virXPathNodeSet("./devices/watchdog", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract watchdog devices")); goto error; } if (n > 1) { @@ -6144,8 +6110,6 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, /* analysis of the memballoon devices */ def->memballoon = NULL; if ((n = virXPathNodeSet("./devices/memballoon", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot extract memory balloon devices")); goto error; } if (n > 1) { @@ -6312,8 +6276,6 @@ static virDomainObjPtr virDomainObjParseXML(virCapsPtr caps, obj->pid = (pid_t)val; if ((n = virXPathNodeSet("./taint", ctxt, &nodes)) < 0) { - virDomainReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("failed to parse taint flags")); goto error; } for (i = 0 ; i < n ; i++) { diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 2c051ea..f3848bd 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -424,7 +424,10 @@ virInterfaceDefParseIfAdressing(virInterfaceDefPtr def, save = ctxt->node; nProtoNodes = virXPathNodeSet("./protocol", ctxt, &protoNodes); - if (nProtoNodes <= 0) { + if (nProtoNodes < 0) + goto error; + + if (nProtoNodes == 0) { /* no protocols is an acceptable outcome */ return 0; } @@ -495,8 +498,6 @@ virInterfaceDefParseBridge(virInterfaceDefPtr def, bridge = ctxt->node; nbItf = virXPathNodeSet("./interface", ctxt, &interfaces); if (nbItf < 0) { - virInterfaceReportError(VIR_ERR_XML_ERROR, - "%s", _("bridge interfaces")); ret = -1; goto error; } @@ -536,12 +537,18 @@ virInterfaceDefParseBondItfs(virInterfaceDefPtr def, int ret = 0; nbItf = virXPathNodeSet("./interface", ctxt, &interfaces); - if (nbItf <= 0) { + if (nbItf < 0) { + ret = -1; + goto error; + } + + if (nbItf == 0) { virInterfaceReportError(VIR_ERR_XML_ERROR, "%s", _("bond has no interfaces")); ret = -1; goto error; } + if (VIR_ALLOC_N(def->data.bond.itf, nbItf) < 0) { virReportOOMError(); ret = -1; diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 4eb46fa..e4765ea 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -642,6 +642,9 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt) } nIps = virXPathNodeSet("./ip", ctxt, &ipNodes); + if (nIps < 0) + goto error; + if (nIps > 0) { int ii; diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index e9b8978..dde2921 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -570,9 +570,6 @@ virNodeDevCapStorageParseXML(xmlXPathContextPtr ctxt, data->storage.serial = virXPathString("string(./serial[1])", ctxt); if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0) { - virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, - _("error parsing storage capabilities for '%s'"), - def->name); goto out; } @@ -735,9 +732,6 @@ virNodeDevCapScsiHostParseXML(xmlXPathContextPtr ctxt, } if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0) { - virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, - _("error parsing SCSI host capabilities for '%s'"), - def->name); goto out; } @@ -1170,7 +1164,11 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt, int create) /* Parse device capabilities */ nodes = NULL; - if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) <= 0) { + if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0) { + goto error; + } + + if (n == 0) { virNodeDeviceReportError(VIR_ERR_INTERNAL_ERROR, _("no device capabilities for '%s'"), def->name); diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 9be4cae..ed7d300 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -438,6 +438,9 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, source->initiator.iqn = virXPathString("string(./initiator/iqn/@name)", ctxt); nsource = virXPathNodeSet("./device", ctxt, &nodeset); + if (nsource < 0) + goto cleanup; + if (nsource > 0) { if (VIR_ALLOC_N(source->devices, nsource) < 0) { VIR_FREE(nodeset); diff --git a/src/conf/storage_encryption_conf.c b/src/conf/storage_encryption_conf.c index 9ec3dad..545efad 100644 --- a/src/conf/storage_encryption_conf.c +++ b/src/conf/storage_encryption_conf.c @@ -159,8 +159,6 @@ virStorageEncryptionParseXML(xmlXPathContextPtr ctxt) n = virXPathNodeSet("./secret", ctxt, &nodes); if (n < 0){ - virStorageReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot extract volume encryption secrets")); goto cleanup; } if (n != 0 && VIR_ALLOC_N(ret->secrets, n) < 0) { diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 19e1938..f579439 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -351,7 +351,6 @@ qemuDomainDefNamespaceParse(xmlDocPtr xml, /* first handle the extra command-line arguments */ n = virXPathNodeSet("./qemu:commandline/qemu:arg", ctxt, &nodes); if (n < 0) - /* virXPathNodeSet already set the error */ goto error; if (n && VIR_ALLOC_N(cmd->args, n) < 0) @@ -372,7 +371,6 @@ qemuDomainDefNamespaceParse(xmlDocPtr xml, /* now handle the extra environment variables */ n = virXPathNodeSet("./qemu:commandline/qemu:env", ctxt, &nodes); if (n < 0) - /* virXPathNodeSet already set the error */ goto error; if (n && VIR_ALLOC_N(cmd->env_name, n) < 0) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index bf33d13..58ff250 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -681,8 +681,6 @@ static int testOpenVolumesForPool(xmlDocPtr xml, ret = virXPathNodeSet(vol_xpath, ctxt, &vols); VIR_FREE(vol_xpath); if (ret < 0) { - testError(VIR_ERR_XML_ERROR, - _("node vol list for pool '%s'"), pool->def->name); goto error; } @@ -892,7 +890,6 @@ static int testOpenFromFile(virConnectPtr conn, ret = virXPathNodeSet("/node/domain", ctxt, &domains); if (ret < 0) { - testError(VIR_ERR_XML_ERROR, "%s", _("node domain list")); goto error; } @@ -936,7 +933,6 @@ static int testOpenFromFile(virConnectPtr conn, ret = virXPathNodeSet("/node/network", ctxt, &networks); if (ret < 0) { - testError(VIR_ERR_XML_ERROR, "%s", _("node network list")); goto error; } for (i = 0 ; i < ret ; i++) { @@ -972,7 +968,6 @@ static int testOpenFromFile(virConnectPtr conn, /* Parse interface definitions */ ret = virXPathNodeSet("/node/interface", ctxt, &ifaces); if (ret < 0) { - testError(VIR_ERR_XML_ERROR, "%s", _("node interface list")); goto error; } for (i = 0 ; i < ret ; i++) { @@ -1008,7 +1003,6 @@ static int testOpenFromFile(virConnectPtr conn, /* Parse Storage Pool list */ ret = virXPathNodeSet("/node/pool", ctxt, &pools); if (ret < 0) { - testError(VIR_ERR_XML_ERROR, "%s", _("node pool list")); goto error; } for (i = 0 ; i < ret ; i++) { @@ -1059,7 +1053,6 @@ static int testOpenFromFile(virConnectPtr conn, ret = virXPathNodeSet("/node/device", ctxt, &devs); if (ret < 0) { - testError(VIR_ERR_XML_ERROR, "%s", _("node device list")); goto error; } for (i = 0 ; i < ret ; i++) { diff --git a/src/util/xml.c b/src/util/xml.c index d2989e2..cdf8786 100644 --- a/src/util/xml.c +++ b/src/util/xml.c @@ -589,7 +589,7 @@ virXPathNodeSet(const char *xpath, if ((ctxt == NULL) || (xpath == NULL)) { virXMLError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Invalid parameter to virXPathNodeSet()")); + _("Invalid parameter to %s"), __FUNCTION__); return (-1); } @@ -601,10 +601,15 @@ virXPathNodeSet(const char *xpath, ctxt->node = relnode; if (obj == NULL) return(0); + if (obj->type != XPATH_NODESET) { + virXMLError(VIR_ERR_INTERNAL_ERROR, + _("Incorrect xpath '%s' passed to %s"), + xpath, __FUNCTION__); xmlXPathFreeObject(obj); return (-1); } + if ((obj->nodesetval == NULL) || (obj->nodesetval->nodeNr < 0)) { xmlXPathFreeObject(obj); return (0); -- 1.7.4.4

2 2

[libvirt] [PATCH] virsh: Correctly initialize libvirt
by Jiri Denemark 13 May '11

13 May '11

virsh didn't call virInitialize(), which (among other things) initializes virLastErr thread local variable. As a result of that, virsh could just segfault in virEventRegisterDefaultImpl() since that is the first call that touches (resets) virLastErr. I have no idea what lucky coincidence made this bug visible but I was able to reproduce it in 100% cases but only in one specific environment which included building in sandbox. --- tools/virsh.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 2b16714..08e6905 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -12819,14 +12819,20 @@ main(int argc, char **argv) char *defaultConn; bool ret = true; + memset(ctl, 0, sizeof(vshControl)); + ctl->imode = true; /* default is interactive mode */ + ctl->log_fd = -1; /* Initialize log file descriptor */ + if (!setlocale(LC_ALL, "")) { perror("setlocale"); /* failure to setup locale is not fatal */ } - if (!bindtextdomain(PACKAGE, LOCALEDIR)) { - perror("bindtextdomain"); + + if (virInitialize() < 0) { + vshError(ctl, "%s", _("Failed to initialize libvirt")); return EXIT_FAILURE; } + if (!textdomain(PACKAGE)) { perror("textdomain"); return EXIT_FAILURE; @@ -12837,10 +12843,6 @@ main(int argc, char **argv) else progname++; - memset(ctl, 0, sizeof(vshControl)); - ctl->imode = true; /* default is interactive mode */ - ctl->log_fd = -1; /* Initialize log file descriptor */ - if ((defaultConn = getenv("VIRSH_DEFAULT_CONNECT_URI"))) { ctl->name = vshStrdup(ctl, defaultConn); } -- 1.7.5.rc3

3 3

[libvirt] [PATCH] virsh: time_t is not a long on FreeBSD
by Matthias Bolte 13 May '11

13 May '11

localtime_r expects time_t. --- tools/virsh.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 3baa015..93a504e 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -10406,7 +10406,8 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) char *doc = NULL; virDomainSnapshotPtr snapshot = NULL; char *state = NULL; - long creation; + long creation_long; + time_t creation_time_t; char timestr[100]; struct tm time_info; @@ -10466,9 +10467,10 @@ cmdSnapshotList(vshControl *ctl, const vshCmd *cmd) if (state == NULL) continue; if (virXPathLong("string(/domainsnapshot/creationTime)", ctxt, - &creation) < 0) + &creation_long) < 0) continue; - localtime_r(&creation, &time_info); + creation_time_t = creation_long; + localtime_r(&creation_time_t, &time_info); strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S %z", &time_info); vshPrint(ctl, " %-20s %-25s %s\n", names[i], timestr, state); -- 1.7.0.4

2 1

[libvirt] [PATCH 0/8 v3] Integration of lock managers in QEMU
by Daniel P. Berrange 13 May '11

13 May '11

This is an update to http://www.redhat.com/archives/libvir-list/2011-January/msg00952.html Changes in this series - Lock manager plugin API is dramatically simpler - Lock manager only protects disk content, not disk metadata (eg file ownership/selinux label changes) - Migration state transfer integrated - Updated for latest sanlock API - Locks are released upon VM pause and reacquired upon resume - Updated documentation NB, as before, the lock manager plugin API is currently *internal* only, so out-of-tree 3rd party plugin impls will be not be supported. This restriction may be released in the future, once we have determined that the current plugin API is suitable for long term ABI guarentees. My intention is to merge this initial series with the nop and sanlock plugin impls, and then provide a fcntl based impl later.

2 11

[libvirt] documentation request
by Igor Serebryany 12 May '11

12 May '11

I'd like to make a request to the people on here to change the way documentation is done when libvirt is changed. I'd love to see two things in the documentation with every change: 1) Version tracking -- every feature that's added should always list which version it was added in 2) Examples -- especially for the changes to domain XML, every new attribute should be demonstrated in the example box for that section To pick a random example, there's the new cpushares and vcpupin attributes. Looking here: http://libvirt.org/formatdomain.html#elementsResources neither of them is listed in the example box. vcpupin has a pretty complicated syntax, and the only way to figure out what it is is to dig through list archives. Also, users might become really confused when these features fail to do anything on versions of libvirt < 0.9.0. .... Actually, digging further I noticed that Osier Yang actually did write an example section: http://www.redhat.com/archives/libvir-list/2011-March/msg01342.html but it fell away by the time his change was pushed (commit 6b3644). --Igor

3 2

[libvirt] [PATCH] build: require newer gettext
by Eric Blake 12 May '11

12 May '11

Now that RHEL 5.6 ships with gettext 0.17, we can get out of the stone age and use the newer gettext capabilities and improved macros for certain configure tests. * configure.ac (AM_GNU_GETTEXT_VERSION): Bump to 0.17. (MKINSTALLDIRS): Delete hack that is no longer needed. * bootstrap.conf (buildreq): Check for minimum gettext version. Based on a report by Wen Congyang. --- bootstrap.conf | 2 +- configure.ac | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index f3e204a..769e57c 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -164,7 +164,7 @@ buildreq="\ autoconf 2.59 automake 1.9.6 autopoint - -gettext - +gettext 0.17 git 1.5.5 gzip - libtool - diff --git a/configure.ac b/configure.ac index a2ce97e..86c9666 100644 --- a/configure.ac +++ b/configure.ac @@ -1980,13 +1980,9 @@ AM_CONDITIONAL([WITH_CIL],[test "$enable_locking" = "yes"]) dnl Enable building libvirtd? AM_CONDITIONAL([WITH_LIBVIRTD],[test "x$with_libvirtd" = "xyes"]) -dnl Check for gettext -AM_GNU_GETTEXT_VERSION([0.14.1]) +dnl Check for gettext - don't go any newer than what RHEL 5 supports +AM_GNU_GETTEXT_VERSION([0.17]) AM_GNU_GETTEXT([external]) -dnl Since we're using such an old version of gettext, we must also define -dnl this symbol, in order to make the autopoint-supplied Makefile.in.in -dnl work properly. -AC_SUBST([MKINSTALLDIRS], ["\$(top_builddir)/$ac_aux_dir/mkinstalldirs"]) ALL_LINGUAS=`cd "$srcdir/po" > /dev/null && ls *.po | sed 's+\.po$++'` -- 1.7.4.4

1 0

[libvirt] libvirt cannot be built with -Werror on RHEL6RC(x86_64)
by Wen Congyang 12 May '11

12 May '11

My steps to build libvirt: 1. # export CFLAGS=-Werror 2. # ./autogen.sh 3. # make dist && rpmbuild --nodeps --define "_sourcedir `pwd`" -ba libvirt.spec ... checking for msgmerge... /usr/bin/msgmerge checking whether NLS is requested... yes checking for GNU gettext in libc... no checking for iconv... yes checking for GNU gettext in libintl... no checking whether to use NLS... no checking for UDEV... yes checking for PCIACCESS... yes ... + rm -f /home/wency/rpmbuild/BUILDROOT/libvirt-0.9.1-1.el6.x86_64/etc/libvirt/qemu/networks/default.xml + rm -f /home/wency/rpmbuild/BUILDROOT/libvirt-0.9.1-1.el6.x86_64/etc/libvirt/qemu/networks/autostart/default.xml + sed -i -e '/<uuid>/d' /home/wency/rpmbuild/BUILDROOT/libvirt-0.9.1-1.el6.x86_64/usr/share/libvirt/networks/default.xml + /usr/lib/rpm/find-lang.sh /home/wency/rpmbuild/BUILDROOT/libvirt-0.9.1-1.el6.x86_64 libvirt No translations found for libvirt in /home/wency/rpmbuild/BUILDROOT/libvirt-0.9.1-1.el6.x86_64 error: Bad exit status from /var/tmp/rpm-tmp.nSgCNo (%install) RPM build errors: Bad exit status from /var/tmp/rpm-tmp.nSgCNo (%install) 4. # cat config.log ... configure:82567: checking for GNU gettext in libc configure:82596: gcc -std=gnu99 -o conftest -Werror conftest.c -ldl >&5 cc1: warnings being treated as errors conftest.c: In function 'main': conftest.c:468: error: cast from pointer to integer of different size configure:82603: $? = 1 configure: failed program was: ... | int | main () | { | bindtextdomain ("", ""); | return (int) gettext ("") + _nl_msg_cat_cntr + *_nl_domain_bindings | ; | return 0; | } configure:82623: result: no ... 5. # cat po/Makefile.in.in ... install: install-exec install-data install-exec: install-data: install-data-@USE_NLS@ if test "$(PACKAGE)" = "gettext-tools"; then \ $(mkinstalldirs) $(DESTDIR)$(gettextsrcdir); \ for file in $(DISTFILES.common) Makevars.template; do \ $(INSTALL_DATA) $(srcdir)/$$file \ $(DESTDIR)$(gettextsrcdir)/$$file; \ done; \ for file in Makevars; do \ rm -f $(DESTDIR)$(gettextsrcdir)/$$file; \ done; \ else \ : ; \ fi install-data-no: all install-data-yes: all $(mkinstalldirs) $(DESTDIR)$(datadir) ... gettext() returns char *, and we try to convert it to int. The size of pointer and int is different, and gcc will give a warning. I build it with -Werror, so the test program can not be built, and NLS is disabled. But We only install translations when NLS is enabled.

2 5

[libvirt] [PATCH] remote: remove special case for getting version
by Eric Blake 12 May '11

12 May '11

* src/remote/remote_protocol.x (remote_get_version_ret) (remote_get_lib_version_ret): Match public API. * daemon/remote_generator.pl: Drop special case. --- By the way, the previous patch and this one both assume that Matthias' generator whitelist patch is applied. daemon/remote_generator.pl | 23 +++++++++-------------- src/remote/remote_protocol.x | 4 ++-- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/daemon/remote_generator.pl b/daemon/remote_generator.pl index c53ebc8..6d91d15 100755 --- a/daemon/remote_generator.pl +++ b/daemon/remote_generator.pl @@ -504,13 +504,6 @@ elsif ($opt_b) { $type_name = $1 if ($1); $type_name .= "long"; - if ($type_name eq "long" and - $calls{$_}->{ProcName} =~ m/^Get(Lib)?Version$/) { - # SPECIAL: virConnectGet(Lib)?Version uses unsigned long - # in public API but hyper in XDR protocol - $type_name = "unsigned long"; - } - push(@vars_list, "$type_name $ret_name"); push(@ret_list, "ret->$ret_name = $ret_name;"); $single_ret_var = $ret_name; @@ -990,19 +983,21 @@ elsif ($opt_k) { $single_ret_type = "int"; } elsif ($ret_member =~ m/^unsigned hyper (\S+);/) { my $arg_name = $1; - push(@ret_list, "rv = ret.$arg_name;"); - $single_ret_var = "unsigned long rv = 0"; - $single_ret_type = "unsigned long"; - } elsif ($ret_member =~ m/^hyper (\S+);/) { - my $arg_name = $1; - if ($call->{ProcName} =~ m/Get(Lib)?Version/) { push(@args_list, "unsigned long *$arg_name"); push(@ret_list, "if ($arg_name) *$arg_name = ret.$arg_name;"); push(@ret_list, "rv = 0;"); $single_ret_var = "int rv = -1"; $single_ret_type = "int"; - } elsif ($call->{ProcName} eq "NodeGetFreeMemory") { + } else { + push(@ret_list, "rv = ret.$arg_name;"); + $single_ret_var = "unsigned long rv = 0"; + $single_ret_type = "unsigned long"; + } + } elsif ($ret_member =~ m/^hyper (\S+);/) { + my $arg_name = $1; + + if ($call->{ProcName} eq "NodeGetFreeMemory") { push(@ret_list, "rv = ret.$arg_name;"); $single_ret_var = "unsigned long long rv = 0"; $single_ret_type = "unsigned long long"; diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 0b88353..04937e2 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -391,11 +391,11 @@ struct remote_get_type_ret { }; struct remote_get_version_ret { - hyper hv_ver; + unsigned hyper hv_ver; }; struct remote_get_lib_version_ret { - hyper lib_ver; + unsigned hyper lib_ver; }; struct remote_get_hostname_ret { -- 1.7.4.4

2 2

[libvirt] [PATCH] fix documentation for cputune parameters
by Igor Serebryany 12 May '11

12 May '11

this re-adds the example section originally written by Osier Yang, and indicates the version in which the cputune parameters became available in libvirt. --- docs/formatdomain.html.in | 28 ++++++++++++++++++++++------ 1 files changed, 22 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index dcfcd94..f3bf8fd 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -288,6 +288,13 @@ <min_guarantee>65536</min_guarantee> </memtune> <vcpu cpuset="1-4,^3,6" current="1">2</vcpu> + <cputune> + <vcpupin vcpu="0" cpuset="1-4,^2"> + <vcpupin vcpu="1" cpuset="0,1"> + <vcpupin vcpu="2" cpuset="2,3"> + <vcpupin vcpu="3" cpuset="0,4"> + <cpushares>2048</cpushares> + </cputune> ...</pre> <dl> @@ -351,21 +358,30 @@ virtual CPUs should be enabled. </dd> <dt><code>cputune</code></dt> - <dd> The optional <code>cputune</code> element provides details - regarding the cpu tunable parameters for the domain.</dd> + <dd> + The optional <code>cputune</code> element provides details + regarding the cpu tunable parameters for the domain. + <span class="since">Since 0.9.0</span> + </dd> <dt><code>vcpupin</code></dt> - <dd> The optional <code>vcpupin</code> element specifies which of host + <dd> + The optional <code>vcpupin</code> element specifies which of host physical CPUS the domain VCPU will be pinned to. If this is ommited, each VCPU pinned to all the physical CPUS by default. It contains two required attributes, the attribute <code>vcpu</code> specifies vcpu id, and the attribute <code>cpuset</code> is same as attribute <code>cpuset</code> - of element <code>vcpu</code>. NB, Only qemu driver supports</dd> + of element <code>vcpu</code>. (NB: Only qemu driver support) + <span class="since">Since 0.9.0</span> + </dd> <dt><code>shares</code></dt> - <dd> The optional <code>shares</code> element specifies the proportional + <dd> + The optional <code>shares</code> element specifies the proportional weighted share for the domain. If this is ommited, it defaults to the OS provided defaults. NB, There is no unit for the value, it's a relative measure based on the setting of other VM, e.g. A VM configured with value - 2048 will get twice as much CPU time as a VM configured with value 1024.</dd> + 2048 will get twice as much CPU time as a VM configured with value 1024. + <span class="since">Since 0.9.0</span> + </dd> </dl> <h3><a name="elementsCPU">CPU model and topology</a></h3> -- 1.7.1

2 1