[libvirt] [PATCH] threads: check for failure to set thread-local value
by Eric Blake
We had a memory leak on a very arcane OOM situation (unlikely to ever
hit in practice, but who knows if libvirt.so would ever be linked
into some other program that exhausts all thread-local storage keys?).
I found it by code inspection, while analyzing a valgrind report
generated by Alex Jia.
* src/util/threads.h (virThreadLocalSet): Alter signature.
* src/util/threads-pthread.c (virThreadHelper): Reduce allocation
lifetime.
(virThreadLocalSet): Detect failure.
* src/util/threads-win32.c (virThreadLocalSet): Likewise.
(virCondWait): Fix caller.
* src/util/virterror.c (virLastErrorObject): Likewise.
---
src/util/threads-pthread.c | 14 +++++++++++---
src/util/threads-win32.c | 9 ++++++---
src/util/threads.h | 2 +-
src/util/virterror.c | 5 +++--
4 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/src/util/threads-pthread.c b/src/util/threads-pthread.c
index 5b8fd5b..ea64887 100644
--- a/src/util/threads-pthread.c
+++ b/src/util/threads-pthread.c
@@ -154,8 +154,11 @@ struct virThreadArgs {
static void *virThreadHelper(void *data)
{
struct virThreadArgs *args = data;
- args->func(args->opaque);
+ struct virThreadArgs local = *args;
+
+ /* Free args early, rather than tying it up during the entire thread. */
VIR_FREE(args);
+ local.func(local.opaque);
return NULL;
}
@@ -249,7 +252,12 @@ void *virThreadLocalGet(virThreadLocalPtr l)
return pthread_getspecific(l->key);
}
-void virThreadLocalSet(virThreadLocalPtr l, void *val)
+int virThreadLocalSet(virThreadLocalPtr l, void *val)
{
- pthread_setspecific(l->key, val);
+ int err = pthread_setspecific(l->key, val);
+ if (err) {
+ errno = err;
+ return -1;
+ }
+ return 0;
}
diff --git a/src/util/threads-win32.c b/src/util/threads-win32.c
index 63f699b..1c33826 100644
--- a/src/util/threads-win32.c
+++ b/src/util/threads-win32.c
@@ -155,7 +155,10 @@ int virCondWait(virCondPtr c, virMutexPtr m)
if (!event) {
return -1;
}
- virThreadLocalSet(&virCondEvent, event);
+ if (virThreadLocalSet(&virCondEvent, event) < 0) {
+ CloseHandle(event);
+ return -1;
+ }
}
virMutexLock(&c->lock);
@@ -376,7 +379,7 @@ void *virThreadLocalGet(virThreadLocalPtr l)
return TlsGetValue(l->key);
}
-void virThreadLocalSet(virThreadLocalPtr l, void *val)
+int virThreadLocalSet(virThreadLocalPtr l, void *val)
{
- TlsSetValue(l->key, val);
+ return TlsSetValue(l->key, val) == 0 ? -1 : 0;
}
diff --git a/src/util/threads.h b/src/util/threads.h
index e52f3a9..e5000ea 100644
--- a/src/util/threads.h
+++ b/src/util/threads.h
@@ -104,7 +104,7 @@ typedef void (*virThreadLocalCleanup)(void *);
int virThreadLocalInit(virThreadLocalPtr l,
virThreadLocalCleanup c) ATTRIBUTE_RETURN_CHECK;
void *virThreadLocalGet(virThreadLocalPtr l);
-void virThreadLocalSet(virThreadLocalPtr l, void*);
+int virThreadLocalSet(virThreadLocalPtr l, void*) ATTRIBUTE_RETURN_CHECK;
# ifdef WIN32
# include "threads-win32.h"
diff --git a/src/util/virterror.c b/src/util/virterror.c
index 380dc56..8205516 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -267,7 +267,8 @@ virLastErrorObject(void)
if (!err) {
if (VIR_ALLOC(err) < 0)
return NULL;
- virThreadLocalSet(&virLastErr, err);
+ if (virThreadLocalSet(&virLastErr, err) < 0)
+ VIR_FREE(err);
}
return err;
}
@@ -612,7 +613,7 @@ virDispatchError(virConnectPtr conn)
virErrorFunc handler = virErrorHandler;
void *userData = virUserData;
- /* Should never happen, but doesn't hurt to check */
+ /* Can only happen on OOM. */
if (!err)
return;
--
1.7.7.4
13 years
Re: [libvirt] possible 0.9.8 regression?
by Jim Fehlig
xhu wrote:
> On 12/16/2011 11:33 AM, Jim Fehlig wrote:
>> Hi All,
>>
>> I've noticed a regression in libvirt 0.9.8 on some of my kvm test machines
>>
>> # virsh start opensuse12
>> error: Failed to start domain opensuse12
>> error: Cannot open network interface control socket: Permission denied
> For I can't reproduce it on my machine with 0.9.8, can you provide me
> the detailed steps?
Nothing special, basic domain config using file-backed disk and
connecting to a bridge.
> Also your os, libvirt, qemu-kvm and kernel version?
Yeah, it has something to do with the kernel, glibc, or other such
component. qemu-kvm isn't the problem as the error occurs before it is
invoked.
kernel 3.1.0, glibc 2.14.1 (openSUSE12.1):
With libvirt 0.9.7, starting the domain works. This version of libvirt
opens control socket with 'socket(AF_INET, SOCK_STREAM, 0)'. With
libvirt 0.9.8, the domain does not start. In this version, the control
socket is opened with 'socket(AF_PACKET, SOCK_DGRAM, 0)', which fails
with EACCES.
kernel 3.0.13, glibc 2.11.3 (SLES11 SP2):
Regression between libvirt 0.9.7 and 0.9.8 not observed.
Initially, I assumed the bug was in glibc. But I can open packet(7)
sockets in a test program running as uid=euid=0, just not within
libvirtd running with same privileges.
Regards,
Jim
> Thanks!
>> Opening a control socket for setting MAC addr, etc. failed with EACCES.
>> In 0.9.7, the socket was opened with domain AF_INET, type SOCK_STREAM,
>> which of course works on this system. In 0.9.8, the socket is opened
>> with AF_PACKET, SOCK_DGRAM. Interestingly, a small test program calling
>> 'socket(AF_PACKET, SOCK_DGRAM, 0)' works on this system.
>>
>> libvirt is built with '--without-capng --without-apparmor
>> --without-selinux' and libvirtd is running with uid=euid=0.
>>
>> I'm really baffled why this fails in libvirtd but works otherwise. Any
>> ideas?
>>
>> Thanks,
>> Jim
>>
>>
>>
>>
>> --
>> libvir-list mailing list
>> libvir-list(a)redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>
13 years
[libvirt] [PATCH] docs: improve documents for domxml-from-native and domxml-to-native commands
by ajia＠redhat.com
From: Alex Jia <ajia(a)redhat.com>
* tools/virsh.pod: improve virsh man page for domxml-from-native and
domxml-to-native commands.
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
tools/virsh.pod | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.pod b/tools/virsh.pod
index dbe5165..ca0b993 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -670,12 +670,16 @@ seconds elapsed since the control interface entered its current state.
=item B<domxml-from-native> I<format> I<config>
Convert the file I<config> in the native guest configuration format
-named by I<format> to a domain XML format.
+named by I<format> to a domain XML format. For QEMU/KVM hypervisor,
+the I<format> argument must be B<qemu-argv>. For Xen hypervisor, the
+I<format> argument may be B<xen-xm> or B<xen-sxpr>.
=item B<domxml-to-native> I<format> I<xml>
Convert the file I<xml> in domain XML format to the native guest
-configuration format named by I<format>.
+configuration format named by I<format>. For QEMU/KVM hypervisor,
+the I<format> argument must be B<qemu-argv>. For Xen hypervisor, the
+I<format> argument may be B<xen-xm> or B<xen-sxpr>.
=item B<dump> I<domain-id> I<corefilepath> [I<--bypass-cache>]
{ [I<--live>] | [I<--crash>] | [I<--reset>] }
--
1.7.1
13 years
[libvirt] [PATCH] virsh: plug memory leaks on cmdDomXMLFromNative and cmdDomXMLToNative
by ajia＠redhat.com
From: Alex Jia <ajia(a)redhat.com>
Detected by valgrind. Leaks introduced in commit 4d5383f.
* tools/virsh.c: fix memory leaks on cmdDomXMLFromNative and cmdDomXMLToNative.
* how to reproduce?
% virsh dumpxml ${guest} > foo.xml
% valgrind -v --leak-check=full virsh domxml-from-native qemu-argv foo.xml
% valgrind -v --leak-check=full virsh domxml-to-native qemu-argv foo.xml
* actual valgrind results:
==9724== 8,193 bytes in 1 blocks are definitely lost in loss record 31 of 33
==9724== at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==9724== by 0x4A06167: realloc (vg_replace_malloc.c:525)
==9724== by 0x4C7510B: virReallocN (memory.c:161)
==9724== by 0x4C84679: virFileReadLimFD (util.c:394)
==9724== by 0x4C84815: virFileReadAll (util.c:455)
==9724== by 0x41A89F: cmdDomXMLFromNative (virsh.c:5532)
==9724== by 0x414872: vshCommandRun (virsh.c:16464)
==9724== by 0x425623: main (virsh.c:17971)
==9724==
==9724== LEAK SUMMARY:
==9724== definitely lost: 8,193 bytes in 1 blocks
==9724== indirectly lost: 0 bytes in 0 blocks
==9724== possibly lost: 0 bytes in 0 blocks
==9724== still reachable: 127,128 bytes in 1,347 blocks
==7409== 8,193 bytes in 1 blocks are definitely lost in loss record 31 of 33
==7409== at 0x4A05FDE: malloc (vg_replace_malloc.c:236)
==7409== by 0x4A06167: realloc (vg_replace_malloc.c:525)
==7409== by 0x4C7510B: virReallocN (memory.c:161)
==7409== by 0x4C84679: virFileReadLimFD (util.c:394)
==7409== by 0x4C84815: virFileReadAll (util.c:455)
==7409== by 0x41A7AF: cmdDomXMLToNative (virsh.c:5578)
==7409== by 0x414892: vshCommandRun (virsh.c:16463)
==7409== by 0x425633: main (virsh.c:17970)
==7409==
==7409== LEAK SUMMARY:
==7409== definitely lost: 8,193 bytes in 1 blocks
==7409== indirectly lost: 0 bytes in 0 blocks
==7409== possibly lost: 0 bytes in 0 blocks
==7409== still reachable: 127,128 bytes in 1,347 blocks
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
tools/virsh.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 583ec6d..784e4f2 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -5540,6 +5540,7 @@ cmdDomXMLFromNative(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
+ VIR_FREE(configData);
return ret;
}
@@ -5586,6 +5587,7 @@ cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
ret = false;
}
+ VIR_FREE(xmlData);
return ret;
}
--
1.7.1
13 years
[libvirt] [PATCHv2] console: plug memory leaks
by ajia＠redhat.com
From: Alex Jia <ajia(a)redhat.com>
Using 'virReallocN' to allocate memory on virConsoleEventOnStdin,
virConsoleEventOnStdout and virConsoleEventOnStream, however, the
cleanup function virConsoleShutdown hasn't released these memory.
* tools/console.c: fix memory leaks on virConsoleShutdown.
https://bugzilla.redhat.com/show_bug.cgi?id=767488
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
tools/console.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/tools/console.c b/tools/console.c
index e6118a0..63e51ce 100644
--- a/tools/console.c
+++ b/tools/console.c
@@ -101,6 +101,10 @@ virConsoleShutdown(virConsolePtr con)
virStreamAbort(con->st);
virStreamFree(con->st);
}
+ if (con->streamToTerminal.data)
+ VIR_FREE(con->streamToTerminal.data);
+ if (con->terminalToStream.data)
+ VIR_FREE(con->terminalToStream.data);
if (con->stdinWatch != -1)
virEventRemoveHandle(con->stdinWatch);
if (con->stdoutWatch != -1)
--
1.7.1
13 years
[libvirt] [PATCH] qemu: Add a capability flag for -no-acpi
by Michael Ellerman
Currently non-x86 guests must have <acpi/> defined in <features> to
prevent libvirt from running qemu with -no-acpi. Although it works, it
is a hack.
Instead add a capability flag which indicates whether qemu understands
the -no-acpi option. Use it to control whether libvirt emits -no-acpi.
Current versions of qemu always display -no-acpi in their help output,
so this patch has no effect. However the development version of qemu
has been modified such that -no-acpi is only displayed when it is
actually supported.
Signed-off-by: Michael Ellerman <michael(a)ellerman.id.au>
---
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 6 ++++--
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e69d601..de2bc13 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -142,6 +142,7 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST,
"cache-unsafe", /* 75 */
"rombar",
"ich9-ahci",
+ "no-acpi",
);
struct qemu_feature_flags {
@@ -1066,6 +1067,8 @@ qemuCapsComputeCmdFlags(const char *help,
qemuCapsSet(flags, QEMU_CAPS_RTC_TD_HACK);
if (strstr(help, "-no-hpet"))
qemuCapsSet(flags, QEMU_CAPS_NO_HPET);
+ if (strstr(help, "-no-acpi"))
+ qemuCapsSet(flags, QEMU_CAPS_NO_ACPI);
if (strstr(help, "-no-kvm-pit-reinjection"))
qemuCapsSet(flags, QEMU_CAPS_NO_KVM_PIT);
if (strstr(help, "-tdf"))
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 00e5214..08d8457 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -115,6 +115,7 @@ enum qemuCapsFlags {
QEMU_CAPS_DRIVE_CACHE_UNSAFE = 75, /* Is cache=unsafe supported? */
QEMU_CAPS_PCI_ROMBAR = 76, /* -device rombar=0|1 */
QEMU_CAPS_ICH9_AHCI = 77, /* -device ich9-ahci */
+ QEMU_CAPS_NO_ACPI = 78, /* -no-acpi */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 92a5955..b241147 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4035,8 +4035,10 @@ qemuBuildCommandLine(virConnectPtr conn,
if (monitor_json && qemuCapsGet(qemuCaps, QEMU_CAPS_NO_SHUTDOWN))
virCommandAddArg(cmd, "-no-shutdown");
- if (!(def->features & (1 << VIR_DOMAIN_FEATURE_ACPI)))
- virCommandAddArg(cmd, "-no-acpi");
+ if (qemuCapsGet(qemuCaps, QEMU_CAPS_NO_ACPI)) {
+ if (!(def->features & (1 << VIR_DOMAIN_FEATURE_ACPI)))
+ virCommandAddArg(cmd, "-no-acpi");
+ }
if (!def->os.bootloader) {
/*
--
1.7.7.3
13 years
[libvirt] [PATCHv3 0/8] add command numatune
by Hu Tao
This series adds a new command numatune to get/set numatune parameters.
Besides libnuma, cpuset cgroup parameters are also set according to
numatune parameters. But for now, only cpuset.mems is supported.
Hu Tao (8):
Add functions to set/get cgroup cpuset parameters
use cpuset to manage numa
add new API virDomain{G,S}etNumaParameters
Implement main entries of virDomain{G,S}etNumaParameters
Add virDomain{G,S}etNumaParameters support to the remote driver
Implement virDomain{G,S}etNumaParameters for the qemu driver
add new command numatune to virsh
Add document for new command numatune.
daemon/remote.c | 64 ++++++++
include/libvirt/libvirt.h.in | 23 +++
python/generator.py | 2 +
src/driver.h | 15 ++
src/libvirt.c | 113 ++++++++++++++
src/libvirt_private.syms | 2 +
src/libvirt_public.syms | 6 +
src/qemu/qemu_cgroup.c | 19 +++
src/qemu/qemu_driver.c | 334 ++++++++++++++++++++++++++++++++++++++++++
src/remote/remote_driver.c | 50 +++++++
src/remote/remote_protocol.x | 24 +++-
src/remote_protocol-structs | 16 ++
src/util/cgroup.c | 32 ++++
src/util/cgroup.h | 3 +
tools/virsh.c | 181 +++++++++++++++++++++++
tools/virsh.pod | 13 ++
16 files changed, 896 insertions(+), 1 deletions(-)
--
1.7.3.1
13 years
[libvirt] [PATCH] console: plug memory leaks
by ajia＠redhat.com
From: Alex Jia <ajia(a)redhat.com>
Using 'virReallocN' to allocate memory on virConsoleEventOnStdin,
virConsoleEventOnStdout and virConsoleEventOnStream, however, the
cleanup function virConsoleShutdown hasn't released these memory.
* tools/console.c: fix memory leaks on virConsoleShutdown.
https://bugzilla.redhat.com/show_bug.cgi?id=767488
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
tools/console.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/tools/console.c b/tools/console.c
index e6118a0..86376e4 100644
--- a/tools/console.c
+++ b/tools/console.c
@@ -101,6 +101,12 @@ virConsoleShutdown(virConsolePtr con)
virStreamAbort(con->st);
virStreamFree(con->st);
}
+ if (con->streamToTerminal.data) {
+ VIR_FREE(con->streamToTerminal.data);
+ }
+ if (con->terminalToStream.data) {
+ VIR_FREE(con->terminalToStream.data);
+ }
if (con->stdinWatch != -1)
virEventRemoveHandle(con->stdinWatch);
if (con->stdoutWatch != -1)
--
1.7.1
13 years
[libvirt] [PATCHv4 0/6] add numatune command
by Eric Blake
Not quite polished - see my complaints that will require a v5:
https://www.redhat.com/archives/libvir-list/2011-December/msg00843.html
I'm hoping Hu can help cover the remaining points, as I've already
spent quite a bit of time cleaning it up to here.
Hu Tao (6):
Add functions to set/get cgroup cpuset parameters
use cpuset to manage numa
add new API virDomain{G, S}etNumaParameters
Add virDomain{G, S}etNumaParameters support to the remote driver
Implement virDomain{G, S}etNumaParameters for the qemu driver
add new command numatune to virsh
daemon/remote.c | 64 ++++++++++
include/libvirt/libvirt.h.in | 39 ++++++
python/generator.py | 2 +
src/conf/domain_conf.h | 8 --
src/driver.h | 15 +++
src/libvirt.c | 129 +++++++++++++++++++++
src/libvirt_private.syms | 2 +
src/libvirt_public.syms | 6 +
src/qemu/qemu_cgroup.c | 19 +++
src/qemu/qemu_driver.c | 262 ++++++++++++++++++++++++++++++++++++++++++
src/remote/remote_driver.c | 50 ++++++++
src/remote/remote_protocol.x | 24 ++++-
src/remote_protocol-structs | 22 ++++
src/util/cgroup.c | 32 +++++
src/util/cgroup.h | 3 +
tools/virsh.c | 159 +++++++++++++++++++++++++
tools/virsh.pod | 19 +++
17 files changed, 846 insertions(+), 9 deletions(-)
--
1.7.7.4
13 years
[libvirt] [PATCH] virsh: Fix a regression of detaching device
by Osier Yang
Since for node <address>, the values for attrs "domain", "bus", "slot",
"function" are all converted to heximal when parsing the XML. However
commit ea7182c29 tries to examine if the device represented by the XML
from user exists in the domain XML earlier before the XML parsing, and
thus if the user use different base to represent the device address, e.g.
<address type='pci' domain='0' bus='0' slot='7'/>
vshCompleteXMLFromDomain won't find the device, and quit with error
"no such device" even if the device does exist.
---
tools/virsh.c | 41 +++++++++++++++++++++++++++++++++++------
1 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 3654589..5c78995 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -12107,12 +12107,14 @@ vshNodeIsSuperset(xmlNodePtr n1, xmlNodePtr n2)
{
xmlNodePtr child1, child2;
xmlAttrPtr attr;
- char *prop1, *prop2;
+ char *prop1 = NULL;
+ char *prop2 = NULL;
bool found;
bool visited;
bool ret = false;
long n1_child_size, n2_child_size, n1_iter;
- virBitmapPtr bitmap;
+ virBitmapPtr bitmap = NULL;
+ unsigned int prop1_ui, prop2_ui;
if (!n1 && !n2)
return true;
@@ -12129,13 +12131,36 @@ vshNodeIsSuperset(xmlNodePtr n1, xmlNodePtr n2)
if (attr->type == XML_ATTRIBUTE_NODE) {
prop1 = virXMLPropString(n1, (const char *) attr->name);
prop2 = virXMLPropString(n2, (const char *) attr->name);
- if (STRNEQ_NULLABLE(prop1, prop2)) {
- xmlFree(prop1);
- xmlFree(prop2);
- return false;
+
+ if (STREQ((const char *)n2->name, "address") &&
+ prop1 && prop2 &&
+ (STREQ((const char *)attr->name, "domain") ||
+ STREQ((const char *)attr->name, "bus") ||
+ STREQ((const char *)attr->name, "slot") ||
+ STREQ((const char *)attr->name, "function"))) {
+ if (virStrToLong_ui(prop1, NULL, 0, &prop1_ui) < 0) {
+ vshError(NULL, _("can't parse value for %s"), attr->name);
+ goto cleanup;
+ }
+
+ if (virStrToLong_ui(prop2, NULL, 0, &prop2_ui) < 0) {
+ vshError(NULL, _("can't parse value for %s"), attr->name);
+ goto cleanup;
+ }
+
+ if (prop1_ui != prop2_ui) {
+ goto cleanup;
+ }
+ } else {
+ if (STRNEQ_NULLABLE(prop1, prop2)) {
+ goto cleanup;
+ }
}
+
xmlFree(prop1);
+ prop1 = NULL;
xmlFree(prop2);
+ prop2 = NULL;
}
attr = attr->next;
}
@@ -12207,6 +12232,10 @@ vshNodeIsSuperset(xmlNodePtr n1, xmlNodePtr n2)
ret = true;
cleanup:
+ if (prop1)
+ xmlFree(prop1);
+ if (prop2)
+ xmlFree(prop2);
virBitmapFree(bitmap);
return ret;
}
--
1.7.7.3
13 years