[libvirt] [PATCH] phyp: avoid a crash
by Eric Blake
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
14 years, 1 month
[libvirt] Potential problems in virFDStreamClose
by Matthias Bolte
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
14 years, 1 month
[libvirt] [PATCH] time_t is not a long on FreeBSD, need to add casts
by Matthias Bolte
---
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
14 years, 1 month
[libvirt] [PATCH] command: Fix compilation on FreeBSD
by Matthias Bolte
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
14 years, 1 month
[libvirt] [PATCH] virsh: Fix uninitialized variable warning
by Matthias Bolte
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
14 years, 1 month
[libvirt] [PATCH] apparmor: Fix compilation by removing remains from virCommand conversion
by Matthias Bolte
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
14 years, 1 month
[libvirt] [PATCH] configure: Fix mpath check on non-Linux systems
by Matthias Bolte
---
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
14 years, 1 month
[libvirt] [PATCH] screenshot: Expose the new API in virsh
by Michal Privoznik
* 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
14 years, 1 month
[libvirt] [PATCH 1/2] libxl: fix typos in previous patch
by Eric Blake
* 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
14 years, 1 month
[libvirt] [PATCH 0/7 v3] events: Add helpers for driver dispatching
by Cole Robinson
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
14 years, 1 month