- Devel - libvirt List Archives

[libvirt] [PATCH] virsh: Fix uninitialized variable warning
by Matthias Bolte 14 May '11

14 May '11

Reported on FreeBSD only. --- tools/virsh.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index fbeb7c8..3baa015 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -11131,7 +11131,7 @@ vshCmddefHelp(vshControl *ctl, const char *cmdname) if (def->opts) { const vshCmdOptDef *opt; for (opt = def->opts; opt->name; opt++) { - const char *fmt; + const char *fmt = "%s"; switch (opt->type) { case VSH_OT_BOOL: fmt = "[--%s]"; -- 1.7.0.4

2 2

[libvirt] [PATCH] apparmor: Fix compilation by removing remains from virCommand conversion
by Matthias Bolte 14 May '11

14 May '11

Commit aaf20355b87d3bfda7579a7f6a4a978e848635c3 was incomplete here and missed to remove some parts. --- Pushing this one under the build breaker rule. src/security/security_apparmor.c | 5 +---- 1 files changed, 1 insertions(+), 4 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 221e331..f47ded7 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -163,7 +163,7 @@ load_profile(virSecurityManagerPtr mgr, const char *fn, bool append) { - int rc = -1, status, ret; + int rc = -1; bool create = true; char *xml = NULL; virCommandPtr cmd; @@ -194,9 +194,6 @@ load_profile(virSecurityManagerPtr mgr, clean: VIR_FREE(xml); - VIR_FORCE_CLOSE(pipefd[0]); - VIR_FORCE_CLOSE(pipefd[1]); - return rc; } -- 1.7.0.4

1 0

[libvirt] [PATCH] configure: Fix mpath check on non-Linux systems
by Matthias Bolte 14 May '11

14 May '11

--- configure.ac | 14 +++++++++----- 1 files changed, 9 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index a2ce97e..4f5c2d7 100644 --- a/configure.ac +++ b/configure.ac @@ -1667,11 +1667,15 @@ if test "$with_storage_scsi" = "check"; then fi AM_CONDITIONAL([WITH_STORAGE_SCSI], [test "$with_storage_scsi" = "yes"]) -if test "$with_storage_mpath" = "check" && test "$with_linux" = "yes"; then - with_storage_mpath=yes - - AC_DEFINE_UNQUOTED([WITH_STORAGE_MPATH], 1, - [whether mpath backend for storage driver is enabled]) +if test "$with_storage_mpath" = "check"; then + if test "$with_linux" = "yes"; then + with_storage_mpath=yes + + AC_DEFINE_UNQUOTED([WITH_STORAGE_MPATH], 1, + [whether mpath backend for storage driver is enabled]) + else + with_storage_mpath=no + fi fi AM_CONDITIONAL([WITH_STORAGE_MPATH], [test "$with_storage_mpath" = "yes"]) -- 1.7.0.4

2 2

[libvirt] [PATCH] screenshot: Expose the new API in virsh
by Michal Privoznik 13 May '11

13 May '11

* tools/virsh.c: Add screenshot command * tools/virsh.pod: Document new command * src/libvirt.c: Fix off-be-one error --- src/libvirt.c | 2 +- tools/virsh.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 9 +++++ 3 files changed, 107 insertions(+), 1 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 6325188..ac6eda6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2461,7 +2461,7 @@ error: * The screen ID is the sequential number of screen. In case of multiple * graphics cards, heads are enumerated before devices, e.g. having * two graphics cards, both with four heads, screen ID 5 addresses - * the first head on the second card. + * the second head on the second card. * * Returns a string representing the mime-type of the image format, or * NULL upon error. The caller must free() the returned value. diff --git a/tools/virsh.c b/tools/virsh.c index fbeb7c8..f000730 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1873,6 +1873,102 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) return ret; } +static const vshCmdInfo info_screenshot[] = { + {"help", N_("take a screenshot of a current domain console and store it " + "into a file")}, + {"desc", N_("screenshot of a current domain console")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_screenshot[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("where to store the screenshot")}, + {"screen", VSH_OT_INT, VSH_OFLAG_NONE, N_("ID of a screen to take screenshot of")}, + {NULL, 0, 0, NULL} +}; + +static int cmdScreenshotSink(virStreamPtr st ATTRIBUTE_UNUSED, + const char *bytes, size_t nbytes, void *opaque) +{ + int *fd = opaque; + + return safewrite(*fd, bytes, nbytes); +} + +static bool +cmdScreenshot(vshControl *ctl, const vshCmd *cmd) { + virDomainPtr dom; + const char *name = NULL; + const char *file = NULL; + int fd = -1; + virStreamPtr st = NULL; + unsigned int screen = 0; + unsigned int flags = 0; /* currently unused */ + int ret = false; + bool created = true; + char *mime = NULL; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (vshCommandOptString(cmd, "file", &file) < 0) { + vshError(ctl, "%s", _("file must not be empty")); + return false; + } + + if (vshCommandOptInt(cmd, "screen", (int*) &screen) < 0) { + vshError(ctl, "%s", _("invalid screen ID")); + return false; + } + + if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) + return false; + + if ((fd = open(file, O_WRONLY|O_CREAT|O_EXCL, 0666)) < 0) { + created = false; + if (errno != EEXIST || + (fd = open(file, O_WRONLY|O_TRUNC, 0666)) < 0) { + vshError(ctl, _("cannot create file %s"), file); + goto cleanup; + } + } + + st = virStreamNew(ctl->conn, 0); + + mime = virDomainScreenshot(dom, st, screen, flags); + if (mime == NULL) { + vshError(ctl, _("could not take a screenshot of %s"), name); + goto cleanup; + } + + if (virStreamRecvAll(st, cmdScreenshotSink, &fd) < 0) { + vshError(ctl, _("could not receive data from domain %s"), name); + goto cleanup; + } + + if (VIR_CLOSE(fd) < 0) { + vshError(ctl, _("cannot close file %s"), file); + goto cleanup; + } + + if (virStreamFinish(st) < 0) { + vshError(ctl, _("cannot close stream on domain %s"), name); + goto cleanup; + } + + vshPrint(ctl, _("Screenshot saved to %s it's type is %s"), file, mime); + ret = true; + +cleanup: + if (ret == false && created) + unlink(file); + virDomainFree(dom); + if (st) + virStreamFree(st); + VIR_FORCE_CLOSE(fd); + return ret; +} + /* * "resume" command */ @@ -10751,6 +10847,7 @@ static const vshCmdDef domManagementCmds[] = { {"resume", cmdResume, opts_resume, info_resume}, {"save", cmdSave, opts_save, info_save}, {"schedinfo", cmdSchedinfo, opts_schedinfo, info_schedinfo}, + {"screenshot", cmdScreenshot, opts_screenshot, info_screenshot}, {"setmaxmem", cmdSetmaxmem, opts_setmaxmem, info_setmaxmem}, {"setmem", cmdSetmem, opts_setmem, info_setmem}, {"setvcpus", cmdSetvcpus, opts_setvcpus, info_setvcpus}, diff --git a/tools/virsh.pod b/tools/virsh.pod index f317c57..2aa66b9 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -588,6 +588,15 @@ Therefore, -1 is a useful shorthand for 262144. B<Note>: The weight and cap parameters are defined only for the XEN_CREDIT scheduler and are now I<DEPRECATED>. +=item B<screenshot> I<domain-id> I<imagefilepath> optional I<--screen> B<screenID> + +Takes a screenshot of a current domain console and stores it into a file. +Optionally, if hypervisor supports more displays for a domain, I<screenID> +allows to specify from which should be the screenshot taken. It is the +sequential number of screen. In case of multiple graphics cards, heads +are enumerated before devices, e.g. having two graphics cards, both with +four heads, screen ID 5 addresses the second head on the second card. + =item B<setmem> I<domain-id> B<kilobytes> optional I<--config> I<--live> I<--current> -- 1.7.5.rc3

2 1

[libvirt] [PATCH 1/2] libxl: fix typos in previous patch
by Eric Blake 13 May '11

13 May '11

* src/libxl/libxl_driver.c (libxlDomainEventFlush, libxlShutdown) (libxlStartup): Fix typos. --- Pushing under the build-breaker rule. src/libxl/libxl_driver.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 3598dd6..60557fc 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -133,7 +133,7 @@ libxlDomainEventFlush(int timer ATTRIBUTE_UNUSED, void *opaque) libxlDriverLock(driver); virDomainEventStateFlush(driver->domainEventState, - libxmlDomainEventDispatchFunc, + libxlDomainEventDispatchFunc, driver); libxlDriverUnlock(driver); } @@ -686,7 +686,7 @@ libxlShutdown(void) VIR_FREE(libxl_driver->libDir); VIR_FREE(libxl_driver->saveDir); - virDomainEventStateFree(privconn->domainEventState); + virDomainEventStateFree(libxl_driver->domainEventState); libxlDriverUnlock(libxl_driver); virMutexDestroy(&libxl_driver->lock); @@ -799,10 +799,10 @@ libxlStartup(int privileged) { libxl_driver->domainEventState = virDomainEventStateNew( libxlDomainEventFlush, - libxml_driver, + libxl_driver, NULL, false); - if (!libxml_driver->domainEventState) + if (!libxl_driver->domainEventState) goto error; libxl_driver->logger = -- 1.7.4.4

1 1

[libvirt] [PATCH 0/7 v3] events: Add helpers for driver dispatching
by Cole Robinson 13 May '11

13 May '11

I noticed that there is quite a bit of code duplication among the drivers that support domain events. This patch series is an attempt to consolidate the shared logic. The new virDomainEventState structure isn't opaque to the clients, though it could be. It would just require adding wrappers for a few more event functions, and possibly finding a way to integrate this cleanup with the xen and vbox event impls, which use less infrastructure than the converted drivers. v2: 2 patches were applied Addressed Eric's comments: NONNULL tagging Use bool for isDispatching Move libvirt_private.syms earlier Add NULL check in StateFree v3: Rebased to latest Convert libxl driver Addressed danpb's comment: virDomainEventStateNew now takes a requireTimer parameter Cole Robinson (7): domain_event: Add virDomainEventState structure domain_event: Add common domain event queue/flush helpers qemu: Use virDomainEventState helpers lxc: Use virDomainEventState helpers test: Use virDomainEventState helpers libxl: Convert to virDomainEventState remote: Use virDomainEventState helpers .gnulib | 2 +- cfg.mk | 1 + src/conf/domain_event.c | 183 +++++++++++++++++++++++++++++++++++++++----- src/conf/domain_event.h | 70 +++++++++++++---- src/libvirt_private.syms | 6 ++ src/libxl/libxl_conf.h | 6 +- src/libxl/libxl_driver.c | 80 ++++++-------------- src/lxc/lxc_conf.h | 6 +- src/lxc/lxc_driver.c | 74 +++++------------- src/qemu/qemu_conf.h | 6 +- src/qemu/qemu_domain.c | 29 +------ src/qemu/qemu_driver.c | 46 +++++------- src/remote/remote_driver.c | 163 +++++++++++++++------------------------ src/test/test_driver.c | 105 ++++++++------------------ 14 files changed, 391 insertions(+), 386 deletions(-) -- 1.7.4.4

3 17

[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