May 2011 - Devel - libvirt List Archives

[libvirt] [PATCH] phyp: avoid a crash
by Eric Blake 14 May '11

14 May '11

This has been present since the introduction of phypAttachDevice in commit 444fd07a. * src/phyp/phyp_driver.c (phypAttachDevice): Don't dereference NULL. --- Found by clang, but the NULL dereference is very blatant. However, I'm worried that this patch, while solving the NULL deref, is dead wrong. In researching this patch, I also found a memory leak: phypDomainCreateAndStart mallocs a virDomainDefPtr and passes it phypBuildLpar, but phypBuildLpar neither stashes that memory into a hash table nor frees it. If it were to stash it into a table, then subsequent operations on the same domain should reuse that existing def, rather than malloc'ing one from scratch. Conversely, if the driver can reconstruct a def in entirety by reading lpar state, then def should be freed, and there should be a function to recreate a def at will. Mallocing a temporary def in functions like phypAttachDevice is probably the wrong thing to do, when really we want to get at the domain definition corresponding to the domain we are modifying. src/phyp/phyp_driver.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 71a3d29..3b4235c 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -1716,14 +1716,19 @@ phypAttachDevice(virDomainPtr domain, const char *xml) char *vios_name = NULL; char *profile = NULL; virDomainDeviceDefPtr dev = NULL; virDomainDefPtr def = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; char *domain_name = NULL; + if (VIR_ALLOC(def) < 0) { + virReportOOMError(); + goto cleanup; + } + domain_name = escape_specialcharacters(domain->name); if (domain_name == NULL) { goto cleanup; } def->os.type = strdup("aix"); -- 1.7.4.4

2 2

[libvirt] Potential problems in virFDStreamClose
by Matthias Bolte 14 May '11

14 May '11

virFDStreamClose calls virFDStreamFree that frees fdst, but virFDStreamClose access fdst->lock after that. virFDStreamClose doesn't destroy fdst->lock. static int virFDStreamClose(virStreamPtr st) { struct virFDStreamData *fdst = st->privateData; int ret; VIR_DEBUG("st=%p", st); if (!fdst) return 0; virMutexLock(&fdst->lock); ret = virFDStreamFree(fdst); st->privateData = NULL; virMutexUnlock(&fdst->lock); return ret; } Matthias

2 3

[libvirt] [PATCH] time_t is not a long on FreeBSD, need to add casts
by Matthias Bolte 14 May '11

14 May '11

--- src/conf/domain_conf.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d3efec6..875f90e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9115,7 +9115,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, def->name = virXPathString("string(./name)", ctxt); if (def->name == NULL) - ignore_value(virAsprintf(&def->name, "%ld", tv.tv_sec)); + ignore_value(virAsprintf(&def->name, "%ld", (long)tv.tv_sec)); if (def->name == NULL) { virReportOOMError(); @@ -9126,7 +9126,7 @@ virDomainSnapshotDefPtr virDomainSnapshotDefParseString(const char *xmlStr, if (!newSnapshot) { if (virXPathLong("string(./creationTime)", ctxt, - &def->creationTime) < 0) { + (long *)&def->creationTime) < 0) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing creationTime from existing snapshot")); goto cleanup; @@ -9192,7 +9192,7 @@ char *virDomainSnapshotDefFormat(char *domain_uuid, virBufferAddLit(&buf, " </parent>\n"); } virBufferAsprintf(&buf, " <creationTime>%ld</creationTime>\n", - def->creationTime); + (long)def->creationTime); virBufferAddLit(&buf, " <domain>\n"); virBufferAsprintf(&buf, " <uuid>%s</uuid>\n", domain_uuid); virBufferAddLit(&buf, " </domain>\n"); -- 1.7.0.4

3 4

[libvirt] [PATCH] command: Fix compilation on FreeBSD
by Matthias Bolte 14 May '11

14 May '11

kill, SIGTERM and SIGKILL require additional headers. --- src/util/command.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index b488d55..b2a873b 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -22,9 +22,11 @@ #include <config.h> #include <poll.h> +#include <signal.h> #include <stdarg.h> #include <stdlib.h> #include <sys/stat.h> +#include <sys/types.h> #include <sys/wait.h> #include "command.h" -- 1.7.0.4

4 5

[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