May 2011 - Devel - libvirt List Archives

[libvirt] [PATCH] domain: Require <init> for container guests
by Cole Robinson 16 May '11

16 May '11

Use capabilities to allow a driver to register a default <init> if none is specified in the XML. Openvz was already open-coding this to be /sbin/init LXC currently falls over if no init is specified, so an explicit error is an improvement IMO. (Side note: I don't think we can set a default value for LXC. If we use /sbin/init but the user doesn't specify a separate root FS for their guest, the container will rerun the host's init which can be traumatic :). For virt-install I'm thinking of defaulting to /sbin/init if a root FS has been specified, otherwise require the user to manually specify <init>) Signed-off-by: Cole Robinson <crobinso(a)redhat.com> --- src/conf/capabilities.h | 1 + src/conf/domain_conf.c | 12 ++++++++++++ src/openvz/openvz_conf.c | 4 +++- src/openvz/openvz_driver.c | 14 -------------- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h index 96bf0a2..e2fa1d6 100644 --- a/src/conf/capabilities.h +++ b/src/conf/capabilities.h @@ -150,6 +150,7 @@ struct _virCaps { int (*privateDataXMLFormat)(virBufferPtr, void *); int (*privateDataXMLParse)(xmlXPathContextPtr, void *); bool hasWideScsiBus; + const char *defaultInitPath; virDomainXMLNamespace ns; }; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9ab9a5a..f7e4959 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5708,6 +5708,18 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, if (STREQ(def->os.type, "exe")) { def->os.init = virXPathString("string(./os/init[1])", ctxt); + if (!def->os.init) { + if (caps->defaultInitPath) { + def->os.init = strdup(caps->defaultInitPath); + if (!def->os.init) { + goto no_memory; + } + } else { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("init binary must be specified")); + goto error; + } + } } if (STREQ(def->os.type, "xen") || diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 88cd4c8..45bc398 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -174,8 +174,10 @@ virCapsPtr openvzCapsInit(void) 0, NULL) == NULL) goto no_memory; - return caps; + caps->defaultInitPath = "/sbin/init"; + + return caps; no_memory: virCapabilitiesFree(caps); return NULL; diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 60f2dc2..6af4f75 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -857,13 +857,6 @@ openvzDomainDefineXML(virConnectPtr conn, const char *xml) VIR_DOMAIN_XML_INACTIVE)) == NULL) goto cleanup; - if (vmdef->os.init == NULL) { - if (!(vmdef->os.init = strdup("/sbin/init"))) { - virReportOOMError(); - goto cleanup; - } - } - vm = virDomainFindByName(&driver->domains, vmdef->name); if (vm) { openvzError(VIR_ERR_OPERATION_FAILED, @@ -943,13 +936,6 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, VIR_DOMAIN_XML_INACTIVE)) == NULL) goto cleanup; - if (vmdef->os.init == NULL) { - if (!(vmdef->os.init = strdup("/sbin/init"))) { - virReportOOMError(); - goto cleanup; - } - } - vm = virDomainFindByName(&driver->domains, vmdef->name); if (vm) { openvzError(VIR_ERR_OPERATION_FAILED, -- 1.7.4.4

3 4

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

15 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 15 May '11

15 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 15 May '11

15 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 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 14 May '11

14 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