[libvirt] [PATCH] usbGetDevice: don't leak a "usbDevice" buffer on failure path
by Jim Meyering
Here's another leak fix, although this one is only on a failure path:
>From a89551ecfcebfc3d45eff38923824a9590fe9f76 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 25 Jan 2010 16:44:13 +0100
Subject: [PATCH] usbGetDevice: don't leak a "usbDevice" buffer on failure path
* src/util/hostusb.c (usbGetDevice): Free "dev" when returning NULL.
---
src/util/hostusb.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/src/util/hostusb.c b/src/util/hostusb.c
index 9a37103..f635ce5 100644
--- a/src/util/hostusb.c
+++ b/src/util/hostusb.c
@@ -171,8 +171,10 @@ usbGetDevice(virConnectPtr conn,
if (vendor) {
/* Look up bus.dev by vendor:product */
- if (usbFindBusByVendor(conn, vendor, product, &bus, &devno) < 0)
+ if (usbFindBusByVendor(conn, vendor, product, &bus, &devno) < 0) {
+ VIR_FREE(dev);
return NULL;
+ }
}
dev->bus = bus;
--
1.7.0.rc0.127.gab8271
14 years, 9 months
[libvirt] [PATCH] qemuMonitorTextGetMemoryStats: plug a leak on an error path
by Jim Meyering
Coverity complained about a leak via this return -1
in qemu_monitor_text.c:
int qemuMonitorTextGetMemoryStats(qemuMonitorPtr mon,
virDomainMemoryStatPtr stats,
unsigned int nr_stats)
{
char *reply = NULL;
int ret = 0;
char *offset;
if (qemuMonitorCommand(mon, "info balloon", &reply) < 0) {
qemudReportError(NULL, NULL, NULL, VIR_ERR_OPERATION_FAILED,
"%s", _("could not query memory balloon statistics"));
return -1;
}
That can happen because
qemuMonitorCommand calls
qemuMonitorCommandWithFd which calls
qemuMonitorCommandWithHandler, which does this:
218 ret = qemuMonitorSend(mon, &msg);
...
228 if (msg.rxBuffer) {
229 *reply = msg.rxBuffer;
230 } else {
231 *reply = strdup("");
232 if (!*reply) {
233 virReportOOMError(NULL);
234 return -1;
235 }
236 }
237
238 if (ret < 0)
239 virReportSystemError(NULL, msg.lastErrno,
240 _("cannot send monitor command '%s'"), cmd);
241
242 return ret;
243 }
That function breaks contract by failing to free *reply when it
returns a negative value. Here's the fix:
>From 3b44df075f9d4330ec27d59eddaa0a32c20d7ac1 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Wed, 20 Jan 2010 18:24:47 +0100
Subject: [PATCH] qemuMonitorTextGetMemoryStats: plug a leak on an error path
* src/qemu/qemu_monitor_text.c (qemuMonitorCommandWithHandler):
Always free *reply, upon failure.
---
src/qemu/qemu_monitor_text.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index c3848b5..d921c7e 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -1,7 +1,7 @@
/*
* qemu_monitor_text.c: interaction with QEMU monitor console
*
- * Copyright (C) 2006-2009 Red Hat, Inc.
+ * Copyright (C) 2006-2010 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -235,9 +235,11 @@ qemuMonitorCommandWithHandler(qemuMonitorPtr mon,
}
}
- if (ret < 0)
+ if (ret < 0) {
virReportSystemError(NULL, msg.lastErrno,
_("cannot send monitor command '%s'"), cmd);
+ VIR_FREE(*reply);
+ }
return ret;
}
--
1.6.6.516.gb72f
14 years, 9 months
[libvirt] [PATCH 1/n] modernizing configure
by Eric Blake
I just built libvirt for the first time today, and noticed that
configure fails on the first missing dependency, rather than trying
harder to make it through and list all missing dependencies at the end.
I ended up restarting configure multiple times. In the process, I also
noticed that configure is somewhat out of date.
So, what is the oldest version of autoconf and automake that libvirt
will insist on supporting? For example, is it time to require automake
1.11.1 (the latest stable release) because of its security patch to
avoid potential problems in 'make dist'? Or is it too soon to expect
all distros to have an automake that new pre-installed, and more effort
be put into supporting automake 1.9.6 and autoconf 2.59 (those being the
implicit minimum requirements due to the use of gnulib)? Or somewhere
in between?
Meanwhile, how about this patch, for me getting my feet wet in helping
to modernize the autotools usage, and hopefully leading up to future
patches to make configure list all missing dependencies at once, rather
than requiring multiple restarts.
14 years, 9 months
[libvirt] [PATCH] Fix libvirtd restart for domains with PCI passthrough devices
by Chris Lalancette
When libvirtd shuts down, it places a <state/> tag in the XML
state file it writes out for guests with PCI passthrough
devices. For devices that are attached at bootup time, the
state tag is empty. However, at libvirtd startup time, it
ignores anything with a <state/> tag in the XML, effectively
hiding the guest.
I can think of at least 3 ways to fix this:
1) Don't throw an error on "unknown" tags in
virDomainHostdevSubsysPciDefParseXML().
2) Have virDomainLoadAllConfigs() pass the
VIR_DOMAIN_XML_INTERNAL_STATUS flag when parsing the domain
XML.
3) Remove the check for VIR_DOMAIN_XML_INTERNAL_STATUS
when parsing the XML.
I chose approach 3). My reasoning for this is that the
<state> tag is a legitimate part of the XML, so we should
always offer to parse it. This fixes the problem with
reconnecting to domains that have PCI passthrough devices.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
src/conf/domain_conf.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 74c2337..595c46c 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) {
+ int flags ATTRIBUTE_UNUSED) {
int ret = -1;
xmlNodePtr cur;
@@ -2890,8 +2890,7 @@ virDomainHostdevSubsysPciDefParseXML(virConnectPtr conn,
if (virDomainDevicePCIAddressParseXML(conn, cur, addr) < 0)
goto out;
- } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) &&
- xmlStrEqual(cur->name, BAD_CAST "state")) {
+ } else if (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, 9 months
[libvirt] [PATCH] allow (only) surrounding whitespace in uuid
by Dan Kenigsberg
Please consider something along these lines. Without it, pretty-printed
domxml is rejected due to the whitespace before uuid, and long long
string of hexadecimal digits is accepted.
---
src/util/uuid.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/src/util/uuid.c b/src/util/uuid.c
index 002a64d..0f2ca96 100644
--- a/src/util/uuid.c
+++ b/src/util/uuid.c
@@ -145,9 +145,13 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid) {
/*
* do a liberal scan allowing '-' and ' ' anywhere between character
- * pairs as long as there is 32 of them in the end.
+ * pairs, and surrounding whitespace, as long as there are exactly
+ * 32 hexadecimal digits the end.
*/
cur = uuidstr;
+ while (c_isspace(*cur))
+ cur++;
+
for (i = 0;i < VIR_UUID_BUFLEN;) {
uuid[i] = 0;
if (*cur == 0)
@@ -170,6 +174,12 @@ virUUIDParse(const char *uuidstr, unsigned char *uuid) {
cur++;
}
+ while (*cur) {
+ if (!c_isspace(*cur))
+ goto error;
+ cur++;
+ }
+
return 0;
error:
--
1.6.5.2
14 years, 9 months
[libvirt] [PATCH] libvirt-publican copyright tags
by David Jorm
I don't have commiter access on this. Can someone else please push it or give me access?
--- a/en-US/Legal_Notice.xml
+++ b/en-US/Legal_Notice.xml
@@ -3,10 +3,10 @@
]>
<legalnotice>
<para>
- Copyright <trademark class="copyright"></trademark> &YEAR; &HOLDER;
+ Copyright <trademark class="copyright"></trademark> &YEAR; &HOLDER;
</para>
<para>
- Copyright © 2009 Red Hat, Inc. and others.
+ Copyright © 2010 Red Hat, Inc. and others.
</para>
<para>
The text of and illustrations in this document are licensed by Red Hat
14 years, 9 months
[libvirt] esx: networkPtr not initialized
by Nimal I
hy,
I'm tring to add a new API for getting the network info from ESX. I found
out that I dont get a valid pointer for the networkPtr while
initializing.(whereas DomainPtr is initialozed and gettting a valid pointer
on a successfull connect itself). Can somebody let me know, what to do to
initialize this pointer and further add a API for a netconfig.
Thanks in Advance,
--
Nimal.I
14 years, 9 months
[libvirt] PCI Passthrough
by Aaron Clausen
I'm trying once again to get PCI passthrough working (KVM 84 on Ubuntu
9.10), and I'm getting this error :
LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
/usr/bin/kvm -S -M pc-0.11 -m 4096 -smp 4 -name mailserver -uuid
76a83471-e94a-3658-fa61-8eceaa74ffc2 -monitor
unix:/var/run/libvirt/qemu/mailserver.monitor,server,nowait -localtime
-boot c -drive file=,if=ide,media=cdrom,index=2 -drive
file=/var/lib/libvirt/images/mailserver.img,if=virtio,index=0,boot=on
-drive file=/var/lib/libvirt/images/mailserver-2.img,if=virtio,index=1
-net nic,macaddr=54:52:00:1b:b2:56,vlan=0,model=virtio,name=virtio.0
-net tap,fd=17,vlan=0,name=tap.0 -serial pty -parallel none -usb
-usbdevice tablet -vnc 127.0.0.1:0 -k en-us -vga cirrus -pcidevice
host=0a:01.0
char device redirected to /dev/pts/0
get_real_device: /sys/bus/pci/devices/0000:0a:01.0/config: Permission denied
init_assigned_device: Error: Couldn't get real device (0a:01.0)!
Failed to initialize assigned device host=0a:01.0
Any thoughts?
--
Aaron Clausen
mightymartianca(a)gmail.com
14 years, 9 months
[libvirt] [PATCH] qemu: Fix race between device rebind and kvm cleanup
by Chris Lalancette
Certain hypervisors (like qemu/kvm) map the PCI bar(s) on
the host when doing device passthrough. This can lead to a race
condition where the hypervisor is still cleaning up the device while
libvirt is trying to re-attach it to the host device driver. To avoid
this situation, we look through /proc/iomem, and if the hypervisor is
still holding onto the bar (denoted by the string in the matcher variable),
then we can wait around a bit for that to clear up.
v2: Thanks to review by DV, make sure we wait the full timeout per-device
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 19 ++++++---
src/util/pci.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++
src/util/pci.h | 1 +
4 files changed, 112 insertions(+), 6 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c912d81..e42c090 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -439,6 +439,7 @@ pciGetDevice;
pciFreeDevice;
pciDettachDevice;
pciReAttachDevice;
+pciWaitForDeviceCleanup;
pciResetDevice;
pciDeviceSetManaged;
pciDeviceGetManaged;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e14ed8f..55550ef 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2277,12 +2277,19 @@ qemuDomainReAttachHostDevices(virConnectPtr conn,
for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
pciDevice *dev = pciDeviceListGet(pcidevs, i);
- if (pciDeviceGetManaged(dev) &&
- pciReAttachDevice(NULL, dev) < 0) {
- virErrorPtr err = virGetLastError();
- VIR_ERROR(_("Failed to re-attach PCI device: %s"),
- err ? err->message : "");
- virResetError(err);
+ int retries = 100;
+ if (pciDeviceGetManaged(dev)) {
+ while (pciWaitForDeviceCleanup(dev, "kvm_assigned_device")
+ && retries) {
+ usleep(100*1000);
+ retries--;
+ }
+ if (pciReAttachDevice(NULL, dev) < 0) {
+ virErrorPtr err = virGetLastError();
+ VIR_ERROR(_("Failed to re-attach PCI device: %s"),
+ err ? err->message : "");
+ virResetError(err);
+ }
}
}
diff --git a/src/util/pci.c b/src/util/pci.c
index 0defbfe..09535bd 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -883,6 +883,103 @@ pciReAttachDevice(virConnectPtr conn, pciDevice *dev)
return pciUnBindDeviceFromStub(conn, dev, driver);
}
+/* Certain hypervisors (like qemu/kvm) map the PCI bar(s) on
+ * the host when doing device passthrough. This can lead to a race
+ * condition where the hypervisor is still cleaning up the device while
+ * libvirt is trying to re-attach it to the host device driver. To avoid
+ * this situation, we look through /proc/iomem, and if the hypervisor is
+ * still holding onto the bar (denoted by the string in the matcher variable),
+ * then we can wait around a bit for that to clear up.
+ *
+ * A typical /proc/iomem looks like this (snipped for brevity):
+ * 00010000-0008efff : System RAM
+ * 0008f000-0008ffff : reserved
+ * ...
+ * 00100000-cc9fcfff : System RAM
+ * 00200000-00483d3b : Kernel code
+ * 00483d3c-005c88df : Kernel data
+ * cc9fd000-ccc71fff : ACPI Non-volatile Storage
+ * ...
+ * d0200000-d02fffff : PCI Bus #05
+ * d0200000-d021ffff : 0000:05:00.0
+ * d0200000-d021ffff : e1000e
+ * d0220000-d023ffff : 0000:05:00.0
+ * d0220000-d023ffff : e1000e
+ * ...
+ * f0000000-f0003fff : 0000:00:1b.0
+ * f0000000-f0003fff : kvm_assigned_device
+ *
+ * Returns 0 if we are clear to continue, and 1 if the hypervisor is still
+ * holding onto the resource.
+ */
+int
+pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher)
+{
+ FILE *fp;
+ char line[160];
+ unsigned long long start, end;
+ int consumed;
+ char *rest;
+ unsigned long long domain;
+ int bus, slot, function;
+ int in_matching_device;
+ int ret;
+ size_t match_depth;
+
+ fp = fopen("/proc/iomem", "r");
+ if (!fp) {
+ /* If we failed to open iomem, we just basically ignore the error. The
+ * unbind might succeed anyway, and besides, it's very likely we have
+ * no way to report the error
+ */
+ VIR_DEBUG0("Failed to open /proc/iomem, trying to continue anyway");
+ return 0;
+ }
+
+ ret = 0;
+ in_matching_device = 0;
+ match_depth = 0;
+ while (fgets(line, sizeof(line), fp) != 0) {
+ /* the logic here is a bit confusing. For each line, we look to
+ * see if it matches the domain:bus:slot.function we were given.
+ * If this line matches the DBSF, then any subsequent lines indented
+ * by 2 spaces are the PCI regions for this device. It's also
+ * possible that none of the PCI regions are currently mapped, in
+ * which case we have no indented regions. This code handles all
+ * of these situations
+ */
+ if (in_matching_device && (strspn(line, " ") == (match_depth + 2))) {
+ if (sscanf(line, "%Lx-%Lx : %n", &start, &end, &consumed) != 2)
+ continue;
+
+ rest = line + consumed;
+ if (STRPREFIX(rest, matcher)) {
+ ret = 1;
+ break;
+ }
+ }
+ else {
+ in_matching_device = 0;
+ if (sscanf(line, "%Lx-%Lx : %n", &start, &end, &consumed) != 2)
+ continue;
+
+ rest = line + consumed;
+ if (sscanf(rest, "%Lx:%x:%x.%x", &domain, &bus, &slot, &function) != 4)
+ continue;
+
+ if (domain != dev->domain || bus != dev->bus || slot != dev->slot ||
+ function != dev->function)
+ continue;
+ in_matching_device = 1;
+ match_depth = strspn(line, " ");
+ }
+ }
+
+ fclose(fp);
+
+ return ret;
+}
+
static char *
pciReadDeviceID(pciDevice *dev, const char *id_name)
{
diff --git a/src/util/pci.h b/src/util/pci.h
index a1a19e7..e6ab137 100644
--- a/src/util/pci.h
+++ b/src/util/pci.h
@@ -81,5 +81,6 @@ int pciDeviceFileIterate(virConnectPtr conn,
int pciDeviceIsAssignable(virConnectPtr conn,
pciDevice *dev,
int strict_acs_check);
+int pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher);
#endif /* __VIR_PCI_H__ */
--
1.6.6
14 years, 9 months
