- Devel - libvirt List Archives

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

14 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

[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