[libvirt] [PATCH] usbFindBusByVendor: don't leak a DIR buffer and FD
by Jim Meyering
I know better than to say this is so simple/obvious that
it doesn't need review, but I'm tempted...
>From 02c7dfa830544bbafd62867fc70b3aba7cc07385 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 25 Jan 2010 16:36:07 +0100
Subject: [PATCH] usbFindBusByVendor: don't leak a DIR buffer and FD
* src/util/hostusb.c (usbFindBusByVendor): Don't leak a DIR buffer
and file descriptor.
---
src/util/hostusb.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/util/hostusb.c b/src/util/hostusb.c
index 8fbb486..9a37103 100644
--- a/src/util/hostusb.c
+++ b/src/util/hostusb.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009 Red Hat, Inc.
+ * Copyright (C) 2009-2010 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -151,6 +151,7 @@ static int usbFindBusByVendor(virConnectPtr conn,
ret = 0;
error:
+ closedir (dir);
return ret;
}
--
1.7.0.rc0.127.gab8271
14 years, 10 months
[libvirt] [PATCH] Eliminate use of large buffer on stack in virFileMakePath.
by Laine Stump
virFileMakePath is a recursive function that was creates a buffer
PATH_MAX bytes long for each recursion (one recursion for each element
in the path). This changes it to have no buffers on the stack, and to
allocate just one buffer total, no matter how many elements are in the
path. Because the modified algorithm requires a char* to be passed in
rather than const char *, it is now 2 functions - a toplevel API
function that remains identical in function, and a 2nd helper function
called for the recursions, which 1) doesn't allocate anything, and 2)
takes a char* arg, so it can modify the contents.
src/util/util.c: rewrite virFileMakePath
---
src/util/util.c | 58 +++++++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 46 insertions(+), 12 deletions(-)
diff --git a/src/util/util.c b/src/util/util.c
index 08d74a3..0ce5026 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1443,34 +1443,68 @@ int virDirCreate(const char *path, mode_t mode,
}
#endif
-int virFileMakePath(const char *path)
-{
+static int virFileMakePathHelper(char *path) {
struct stat st;
- char parent[PATH_MAX];
- char *p;
+ char *p = NULL;
int err;
if (stat(path, &st) >= 0)
return 0;
- if (virStrcpyStatic(parent, path) == NULL)
- return EINVAL;
-
- if (!(p = strrchr(parent, '/')))
+ if ((p = strrchr(path, '/')) == NULL)
return EINVAL;
- if (p != parent) {
+ if (p != path) {
*p = '\0';
- if ((err = virFileMakePath(parent)))
+ err = virFileMakePathHelper(path);
+ *p = '/';
+ if (err != 0)
return err;
}
- if (mkdir(path, 0777) < 0 && errno != EEXIST)
+ if (mkdir(path, 0777) < 0 && errno != EEXIST) {
return errno;
-
+ }
return 0;
}
+int virFileMakePath(const char *path)
+{
+ struct stat st;
+ char *parent = NULL;
+ char *p;
+ int err = 0;
+
+ if (stat(path, &st) >= 0)
+ goto cleanup;
+
+ if ((parent = strdup(path)) == NULL) {
+ err = ENOMEM;
+ goto cleanup;
+ }
+
+ if ((p = strrchr(parent, '/')) == NULL) {
+ err = EINVAL;
+ goto cleanup;
+ }
+
+ if (p != parent) {
+ *p = '\0';
+ if ((err = virFileMakePathHelper(parent)) != 0) {
+ goto cleanup;
+ }
+ }
+
+ if (mkdir(path, 0777) < 0 && errno != EEXIST) {
+ err = errno;
+ goto cleanup;
+ }
+
+cleanup:
+ VIR_FREE(parent);
+ return err;
+}
+
/* Build up a fully qualfiied path for a config file to be
* associated with a persistent guest or network */
int virFileBuildPath(const char *dir,
--
1.6.6
14 years, 10 months
[libvirt] [PATCH] Fix two instances of misspelled 'pseudo'
by Chris Lalancette
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
docs/sitemap.html.in | 2 +-
src/xen/xs_internal.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/docs/sitemap.html.in b/docs/sitemap.html.in
index 424897f..76d8faa 100644
--- a/docs/sitemap.html.in
+++ b/docs/sitemap.html.in
@@ -134,7 +134,7 @@
</li>
<li>
<a href="drvtest.html">Test</a>
- <span>Psuedo-driver simulating APIs in memory for test suites</span>
+ <span>Pseudo-driver simulating APIs in memory for test suites</span>
</li>
<li>
<a href="drvremote.html">Remote</a>
diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c
index 8a64d4e..1ca3da0 100644
--- a/src/xen/xs_internal.c
+++ b/src/xen/xs_internal.c
@@ -853,7 +853,7 @@ int xenStoreDomainGetVNCPort(virConnectPtr conn, int domid) {
* @conn: the hypervisor connection
* @domid: id of the domain
*
- * Return the path to the psuedo TTY on which the guest domain's
+ * Return the path to the pseudo TTY on which the guest domain's
* serial console is attached.
*
* Returns the path to the serial console. It is the callers
--
1.6.6
14 years, 10 months
[libvirt] Live Migration with non-shared storage for kvm
by Kenneth Nagin
On January 5 I asked for comments on adding libvirt support for live
migration with non-shared storage on kvm. I did not get any comments so I
am repeating my request:
Support for live migration between hosts that do not share storage was
added to qemu-kvm release 0.12.1.
It supports two flags:
-b migration without shared storage with full disk copy
-i migration without shared storage with incremental copy (same base image
shared between source and destination).
I suggest adding these flags to virDomainMigrate.
If I'm not mistaken qemuMonitorTextMigrate is the function that actually
invokes the kvm monitor.
Thus, it would be necessary to pass the flags to qemuMonitorTextMigrate..
But qemuMonitorTextMigrate does not have a flag input parameter. I think
the least disruptive way to support the new flags is use the existing
"background" parameter
to pass the flags. Of course this would require some changes to the
upstream functions that are invoked for migration.
What do you think?
Kenneth Nagin
14 years, 10 months
[libvirt] [PATCH] Fix a crash when restarting libvirtd.
by Chris Lalancette
If you shutdown libvirtd while a domain with PCI
devices is running, then try to restart libvirtd,
libvirtd will crash.
This happens because qemuUpdateActivePciHostdevs() is calling
pciDeviceListSteal() with a dev of 0x0 (NULL), and then trying
to dereference it. This patch fixes it up so that
qemuUpdateActivePciHostdevs() steals the devices after first
Get()'ting them, avoiding the crash.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
src/qemu/qemu_driver.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 55550ef..bbdbe33 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2147,6 +2147,7 @@ qemuUpdateActivePciHostdevs(struct qemud_driver *driver,
virDomainDefPtr def)
{
pciDeviceList *pcidevs;
+ int i;
int ret = -1;
if (!def->nhostdevs)
@@ -2155,8 +2156,9 @@ qemuUpdateActivePciHostdevs(struct qemud_driver *driver,
if (!(pcidevs = qemuGetPciHostDeviceList(NULL, def)))
return -1;
- while (pciDeviceListCount(pcidevs) > 0) {
- pciDevice *dev = pciDeviceListSteal(NULL, pcidevs, 0);
+ for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
+ pciDevice *dev = pciDeviceListGet(pcidevs, i);
+ pciDeviceListSteal(NULL, pcidevs, dev);
if (pciDeviceListAdd(NULL,
driver->activePciHostdevs,
dev) < 0) {
--
1.6.6
14 years, 10 months
[libvirt] [PATCH] Revert "Fix libvirtd restart for domains with PCI passthrough devices"
by Chris Lalancette
This reverts commit cdc42d0a4865199a941d330dbb6ca1ef426323ae.
As DanB pointed out, this patch is actually wrong. The real
bug that was causing me to see this problem is a bug
introduced in a RHEL-5 libvirt snapshot, and I'm going to
fix the real bug there.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
src/conf/domain_conf.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6d0e2dc..e548d1d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2876,7 +2876,7 @@ static int
virDomainHostdevSubsysPciDefParseXML(virConnectPtr conn,
const xmlNodePtr node,
virDomainHostdevDefPtr def,
- int flags ATTRIBUTE_UNUSED) {
+ int flags) {
int ret = -1;
xmlNodePtr cur;
@@ -2890,7 +2890,8 @@ virDomainHostdevSubsysPciDefParseXML(virConnectPtr conn,
if (virDomainDevicePCIAddressParseXML(conn, cur, addr) < 0)
goto out;
- } else if (xmlStrEqual(cur->name, BAD_CAST "state")) {
+ } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) &&
+ xmlStrEqual(cur->name, BAD_CAST "state")) {
/* Legacy back-compat. Don't add any more attributes here */
char *devaddr = virXMLPropString(cur, "devaddr");
if (devaddr &&
--
1.6.6
14 years, 10 months
[libvirt] [PATCH] qemuMonitorTextAttachDrive: avoid two leaks
by Jim Meyering
Coverity reported the first leak: return-(-1)-would-leak-"safe_str".
I spotted the 2nd.
>From 6326ab7b52f3550f24015c27407c1d3c2a1fd64d Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 25 Jan 2010 20:19:01 +0100
Subject: [PATCH] qemuMonitorTextAttachDrive: avoid two leaks
* src/qemu/qemu_monitor_text.c (qemuMonitorTextAttachDrive): Most other
failures in this function would "goto cleanup", but one mistakenly
returned directly, skipping the cleanup and resulting in a leak.
In addition, iterating the "try_command" loop would clobber, and
thus leak, the "cmd" allocated on the first iteration,
so be careful to free it in addition to "reply" beforehand.
---
src/qemu/qemu_monitor_text.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index ce5349b..380bcdc 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1881,13 +1881,13 @@ try_command:
if (qemudParseDriveAddReply(reply, driveAddr) < 0) {
if (!tryOldSyntax && strstr(reply, "invalid char in expression")) {
VIR_FREE(reply);
+ VIR_FREE(cmd);
tryOldSyntax = 1;
goto try_command;
}
qemudReportError (NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
_("adding %s disk failed: %s"), drivestr, reply);
- VIR_FREE(reply);
- return -1;
+ goto cleanup;
}
ret = 0;
--
1.7.0.rc0.127.gab8271
14 years, 10 months
[libvirt] [PATCH] qemu: Search binaries in PATH instead of hardcoding /usr/bin
by Matthias Bolte
---
src/qemu/qemu_conf.c | 137 ++++++++++++++++++++++++++++++--------------------
1 files changed, 82 insertions(+), 55 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 92a9348..6141ec6 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -379,20 +379,20 @@ static const struct qemu_feature_flags const arch_info_x86_64_flags [] = {
/* The archicture tables for supported QEMU archs */
static const struct qemu_arch_info const arch_info_hvm[] = {
- { "i686", 32, NULL, "/usr/bin/qemu",
- "/usr/bin/qemu-system-x86_64", arch_info_i686_flags, 4 },
- { "x86_64", 64, NULL, "/usr/bin/qemu-system-x86_64",
+ { "i686", 32, NULL, "qemu",
+ "qemu-system-x86_64", arch_info_i686_flags, 4 },
+ { "x86_64", 64, NULL, "qemu-system-x86_64",
NULL, arch_info_x86_64_flags, 2 },
- { "arm", 32, NULL, "/usr/bin/qemu-system-arm", NULL, NULL, 0 },
- { "mips", 32, NULL, "/usr/bin/qemu-system-mips", NULL, NULL, 0 },
- { "mipsel", 32, NULL, "/usr/bin/qemu-system-mipsel", NULL, NULL, 0 },
- { "sparc", 32, NULL, "/usr/bin/qemu-system-sparc", NULL, NULL, 0 },
- { "ppc", 32, NULL, "/usr/bin/qemu-system-ppc", NULL, NULL, 0 },
+ { "arm", 32, NULL, "qemu-system-arm", NULL, NULL, 0 },
+ { "mips", 32, NULL, "qemu-system-mips", NULL, NULL, 0 },
+ { "mipsel", 32, NULL, "qemu-system-mipsel", NULL, NULL, 0 },
+ { "sparc", 32, NULL, "qemu-system-sparc", NULL, NULL, 0 },
+ { "ppc", 32, NULL, "qemu-system-ppc", NULL, NULL, 0 },
};
static const struct qemu_arch_info const arch_info_xen[] = {
- { "i686", 32, "xenner", "/usr/bin/xenner", NULL, arch_info_i686_flags, 4 },
- { "x86_64", 64, "xenner", "/usr/bin/xenner", NULL, arch_info_x86_64_flags, 2 },
+ { "i686", 32, "xenner", "xenner", NULL, arch_info_i686_flags, 4 },
+ { "x86_64", 64, "xenner", "xenner", NULL, arch_info_x86_64_flags, 2 },
};
@@ -768,8 +768,8 @@ qemudCapsInitGuest(virCapsPtr caps,
int i;
int haskvm = 0;
int haskqemu = 0;
- const char *kvmbin = NULL;
- const char *binary = NULL;
+ char *kvmbin = NULL;
+ char *binary = NULL;
time_t binary_mtime;
virCapsGuestMachinePtr *machines = NULL;
int nmachines = 0;
@@ -779,10 +779,15 @@ qemudCapsInitGuest(virCapsPtr caps,
/* Check for existance of base emulator, or alternate base
* which can be used with magic cpu choice
*/
- if (access(info->binary, X_OK) == 0)
- binary = info->binary;
- else if (info->altbinary && access(info->altbinary, X_OK) == 0)
- binary = info->altbinary;
+ binary = virFindFileInPath(info->binary);
+
+ if (binary == NULL || access(binary, X_OK) != 0) {
+ VIR_FREE(binary);
+ binary = virFindFileInPath(info->altbinary);
+
+ if (binary != NULL && access(binary, X_OK) != 0)
+ VIR_FREE(binary);
+ }
/* Can use acceleration for KVM/KQEMU if
* - host & guest arches match
@@ -792,16 +797,30 @@ qemudCapsInitGuest(virCapsPtr caps,
*/
if (STREQ(info->arch, hostmachine) ||
(STREQ(hostmachine, "x86_64") && STREQ(info->arch, "i686"))) {
- const char *const kvmbins[] = { "/usr/bin/qemu-kvm", /* Fedora */
- "/usr/bin/kvm" }; /* Upstream .spec */
+ if (access("/dev/kvm", F_OK) == 0) {
+ const char *const kvmbins[] = { "qemu-kvm", /* Fedora */
+ "kvm" }; /* Upstream .spec */
+
+ for (i = 0; i < ARRAY_CARDINALITY(kvmbins); ++i) {
+ kvmbin = virFindFileInPath(kvmbins[i]);
+
+ if (kvmbin == NULL || access(kvmbin, X_OK) != 0) {
+ VIR_FREE(kvmbin);
+ continue;
+ }
- for (i = 0; i < ARRAY_CARDINALITY(kvmbins); ++i) {
- if (access(kvmbins[i], X_OK) == 0 &&
- access("/dev/kvm", F_OK) == 0) {
haskvm = 1;
- kvmbin = kvmbins[i];
- if (!binary)
- binary = kvmbin;
+
+ if (binary == NULL) {
+ binary = strdup(kvmbin);
+
+ if (binary == NULL) {
+ virReportOOMError(NULL);
+ VIR_FREE(kvmbin);
+ return -1;
+ }
+ }
+
break;
}
}
@@ -826,21 +845,18 @@ qemudCapsInitGuest(virCapsPtr caps,
virCapsGuestMachinePtr machine;
if (VIR_ALLOC(machine) < 0) {
- virReportOOMError(NULL);
- return -1;
+ goto no_memory;
}
if (!(machine->name = strdup(info->machine))) {
- virReportOOMError(NULL);
VIR_FREE(machine);
- return -1;
+ goto no_memory;
}
if (VIR_ALLOC_N(machines, 1) < 0) {
- virReportOOMError(NULL);
VIR_FREE(machine->name);
VIR_FREE(machine);
- return -1;
+ goto no_memory;
}
machines[0] = machine;
@@ -853,7 +869,7 @@ qemudCapsInitGuest(virCapsPtr caps,
old_caps, &machines, &nmachines);
if (probe &&
qemudProbeMachineTypes(binary, &machines, &nmachines) < 0)
- return -1;
+ goto error;
}
/* We register kvm as the base emulator too, since we can
@@ -865,21 +881,18 @@ qemudCapsInitGuest(virCapsPtr caps,
binary,
NULL,
nmachines,
- machines)) == NULL) {
- for (i = 0; i < nmachines; i++) {
- VIR_FREE(machines[i]->name);
- VIR_FREE(machines[i]);
- }
- VIR_FREE(machines);
- return -1;
- }
+ machines)) == NULL)
+ goto error;
+
+ machines = NULL;
+ nmachines = 0;
guest->arch.defaultInfo.emulator_mtime = binary_mtime;
if (qemudProbeCPUModels(binary, info->arch, &ncpus, NULL) == 0
&& ncpus > 0
&& !virCapabilitiesAddGuestFeature(guest, "cpuselection", 1, 0))
- return -1;
+ goto error;
if (hvm) {
if (virCapabilitiesAddGuestDomain(guest,
@@ -888,7 +901,7 @@ qemudCapsInitGuest(virCapsPtr caps,
NULL,
0,
NULL) == NULL)
- return -1;
+ goto error;
if (haskqemu &&
virCapabilitiesAddGuestDomain(guest,
@@ -897,7 +910,7 @@ qemudCapsInitGuest(virCapsPtr caps,
NULL,
0,
NULL) == NULL)
- return -1;
+ goto error;
if (haskvm) {
virCapsGuestDomainPtr dom;
@@ -911,9 +924,6 @@ qemudCapsInitGuest(virCapsPtr caps,
binary_mtime = 0;
}
- machines = NULL;
- nmachines = 0;
-
if (!STREQ(binary, kvmbin)) {
int probe = 1;
if (old_caps && binary_mtime)
@@ -922,7 +932,7 @@ qemudCapsInitGuest(virCapsPtr caps,
old_caps, &machines, &nmachines);
if (probe &&
qemudProbeMachineTypes(kvmbin, &machines, &nmachines) < 0)
- return -1;
+ goto error;
}
if ((dom = virCapabilitiesAddGuestDomain(guest,
@@ -931,14 +941,12 @@ qemudCapsInitGuest(virCapsPtr caps,
NULL,
nmachines,
machines)) == NULL) {
- for (i = 0; i < nmachines; i++) {
- VIR_FREE(machines[i]->name);
- VIR_FREE(machines[i]);
- }
- VIR_FREE(machines);
- return -1;
+ goto error;
}
+ machines = NULL;
+ nmachines = 0;
+
dom->info.emulator_mtime = binary_mtime;
}
} else {
@@ -948,7 +956,7 @@ qemudCapsInitGuest(virCapsPtr caps,
NULL,
0,
NULL) == NULL)
- return -1;
+ goto error;
}
if (info->nflags) {
@@ -957,11 +965,24 @@ qemudCapsInitGuest(virCapsPtr caps,
info->flags[i].name,
info->flags[i].default_on,
info->flags[i].toggle) == NULL)
- return -1;
+ goto error;
}
}
+ VIR_FREE(binary);
+ VIR_FREE(kvmbin);
+
return 0;
+
+no_memory:
+ virReportOOMError(NULL);
+
+error:
+ VIR_FREE(binary);
+ VIR_FREE(kvmbin);
+ virCapabilitiesFreeMachines(machines, nmachines);
+
+ return -1;
}
@@ -1011,6 +1032,7 @@ virCapsPtr qemudCapsInit(virCapsPtr old_caps) {
struct utsname utsname;
virCapsPtr caps;
int i;
+ char *xenner = NULL;
/* Really, this never fails - look at the man-page. */
uname (&utsname);
@@ -1051,7 +1073,9 @@ virCapsPtr qemudCapsInit(virCapsPtr old_caps) {
goto no_memory;
/* Then possibly the Xen paravirt guests (ie Xenner */
- if (access("/usr/bin/xenner", X_OK) == 0 &&
+ xenner = virFindFileInPath("xenner");
+
+ if (xenner != NULL && access(xenner, X_OK) == 0 &&
access("/dev/kvm", F_OK) == 0) {
for (i = 0 ; i < ARRAY_CARDINALITY(arch_info_xen) ; i++)
/* Allow Xen 32-on-32, 32-on-64 and 64-on-64 */
@@ -1065,12 +1089,15 @@ virCapsPtr qemudCapsInit(virCapsPtr old_caps) {
}
}
+ VIR_FREE(xenner);
+
/* QEMU Requires an emulator in the XML */
virCapabilitiesSetEmulatorRequired(caps);
return caps;
no_memory:
+ VIR_FREE(xenner);
virCapabilitiesFree(caps);
return NULL;
}
--
1.6.3.3
14 years, 10 months
[libvirt] [PATCH] Look in /usr/libexec for the qemu-kvm binary.
by Chris Lalancette
On RHEL-5 the qemu-kvm binary is located in /usr/libexec.
To reduce confusion for people trying to run upstream libvirt
on RHEL-5 machines, make the qemu driver look in /usr/libexec
for the qemu-kvm binary.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
src/qemu/qemu_conf.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index c227fe1..2ba8366 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -797,7 +797,8 @@ qemudCapsInitGuest(virCapsPtr caps,
*/
if (STREQ(info->arch, hostmachine) ||
(STREQ(hostmachine, "x86_64") && STREQ(info->arch, "i686"))) {
- const char *const kvmbins[] = { "/usr/bin/qemu-kvm", /* Fedora */
+ const char *const kvmbins[] = { "/usr/libexec/qemu-kvm", /* RHEL */
+ "/usr/bin/qemu-kvm", /* Fedora */
"/usr/bin/kvm" }; /* Upstream .spec */
for (i = 0; i < ARRAY_CARDINALITY(kvmbins); ++i) {
--
1.6.6
14 years, 10 months