Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind

2011-10-30 Thread Alexander Graf

On 30.10.2011, at 13:07, Stefan Weil wrote:

> Valgrind is a tool which can automatically detect many kinds of bugs.
> 
> Running QEMU on Valgrind with x86_64 hosts was not possible because
> Valgrind aborts when memalign is called with an alignment larger than
> 1 MiB. QEMU normally uses 2 MiB on Linux x86_64.
> 
> Now the alignment is reduced to the page size when QEMU is running on
> Valgrind.
> 
> valgrind.h is a copy from Valgrind svn trunk r12226 with trailing
> whitespace stripped but otherwise unmodified, so it still raises lots
> of errors when checked with scripts/checkpatch.pl.

Can't we just require valgrind header files to be around when kvm is enabled? I 
would rather not copy code from other projects. Alternatively you could take 
the header and shrink it down to maybe 5 lines of inline asm code that would be 
a lot more readable :). You're #ifdef'ing on x86_64 already anyways.

> 
> It is included here to avoid a dependency on Valgrind.
> 
> Signed-off-by: Stefan Weil 
> ---
> oslib-posix.c |8 +-
> valgrind.h| 4060 +
> 2 files changed, 4066 insertions(+), 2 deletions(-)
> create mode 100644 valgrind.h
> 
> diff --git a/oslib-posix.c b/oslib-posix.c
> index dbc8ee8..e503e58 100644
> --- a/oslib-posix.c
> +++ b/oslib-posix.c
> @@ -36,10 +36,14 @@ extern int daemon(int, int);
> #endif
> 
> #if defined(__linux__) && defined(__x86_64__)
> -   /* Use 2MB alignment so transparent hugepages can be used by KVM */
> +   /* Use 2 MiB alignment so transparent hugepages can be used by KVM.
> +  Valgrind does not support alignments larger than 1 MiB,
> +  therefore we need special code which handles running on Valgrind. */
> #  define QEMU_VMALLOC_ALIGN (512 * 4096)
> +#  include "valgrind.h" /* RUNNING_ON_VALGRIND */

I would prefer to just have a global variable we keep the alignment in that 
gets populated on initialization. That way we don't have to query valgrind or 
potentially query the kernel on every memalign and keep everything on a single 
spot.


Alex




Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind

2011-10-30 Thread Stefan Weil

Am 30.10.2011 14:41, schrieb Alexander Graf:

On 30.10.2011, at 13:07, Stefan Weil wrote:

Valgrind is a tool which can automatically detect many kinds of bugs.

Running QEMU on Valgrind with x86_64 hosts was not possible because
Valgrind aborts when memalign is called with an alignment larger than
1 MiB. QEMU normally uses 2 MiB on Linux x86_64.

Now the alignment is reduced to the page size when QEMU is running on
Valgrind.

valgrind.h is a copy from Valgrind svn trunk r12226 with trailing
whitespace stripped but otherwise unmodified, so it still raises lots
of errors when checked with scripts/checkpatch.pl.


Can't we just require valgrind header files to be around when kvm is 
enabled? I would rather not copy code from other projects. 
Alternatively you could take the header and shrink it down to maybe 5 
lines of inline asm code that would be a lot more readable :). You're 
#ifdef'ing on x86_64 already anyways.


The patch is currently required for x86_64 hosts running Linux.
I estimate that this is one of the most frequently used QEMU host platforms,
and in most cases, KVM will be configured because this is the default
and also because it is reasonable for this platform.

How many of these hosts will have the Valgrind header around?
I estimate less than 20 %, so configure would have to test whether
valgrind.h is available or not. I think providing valgrind.h is
a much better (and simpler) solution.

Stripping valgrind.h is not a good idea: the file is specially designed
to be included in other projects like QEMU. As soon as the 2 MiB alignment
is used for other hosts (ppc64, ...), you would have to take more and more
from the original code. The file was not designed to be readable.
Although it contains lots of comments which improve readability,
there remains code which is less easy to read. I cite one of those
comments:

/* The following defines the magic code sequences which the JITter
   spots and handles magically.  Don't look too closely at them as
   they will rot your brain.

Instead of rotting my brain, I prefer using a copy of the original code.





It is included here to avoid a dependency on Valgrind.

Signed-off-by: Stefan Weil 
---
oslib-posix.c | 8 +-
valgrind.h | 4060 
+

2 files changed, 4066 insertions(+), 2 deletions(-)
create mode 100644 valgrind.h

diff --git a/oslib-posix.c b/oslib-posix.c
index dbc8ee8..e503e58 100644
--- a/oslib-posix.c
+++ b/oslib-posix.c
@@ -36,10 +36,14 @@ extern int daemon(int, int);
#endif

#if defined(__linux__) && defined(__x86_64__)
- /* Use 2MB alignment so transparent hugepages can be used by KVM */
+ /* Use 2 MiB alignment so transparent hugepages can be used by KVM.
+ Valgrind does not support alignments larger than 1 MiB,
+ therefore we need special code which handles running on Valgrind. */
# define QEMU_VMALLOC_ALIGN (512 * 4096)
+# include "valgrind.h" /* RUNNING_ON_VALGRIND */


I would prefer to just have a global variable we keep the alignment in 
that gets populated on initialization. That way we don't have to query 
valgrind or potentially query the kernel on every memalign and keep 
everything on a single spot.


As long as this is the only use of a Valgrind interface,
I don't know which other single spot would be better.
For a short time I thought about adding something to
qemu-common.h, but I don't think that would be a good idea.

memalign is not time critical. The extra code added by 
RUNNING_ON_VALGRIND is small
(a few machine instructions, see comment in valgrind.h), and it is only 
executed when
a large memory block is allocated. How many calls of qemu_vmalloc with 
at least 2 MiB

do you expect?

There remains an issue with my patch which I don't like:
I dislike the large number of files in QEMU's root directory
and that I was going to add one more. That's something
which should be addressed in a different patch as soon
as there is a consensus that many files in a directory
are a problem and which project directory structure would
be better.


Alex


Cheers,
Stefan




Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind

2011-10-30 Thread Alexander Graf

On 30.10.2011, at 15:30, Stefan Weil wrote:

> Am 30.10.2011 14:41, schrieb Alexander Graf:
>> On 30.10.2011, at 13:07, Stefan Weil wrote:
>>> Valgrind is a tool which can automatically detect many kinds of bugs.
>>> 
>>> Running QEMU on Valgrind with x86_64 hosts was not possible because
>>> Valgrind aborts when memalign is called with an alignment larger than
>>> 1 MiB. QEMU normally uses 2 MiB on Linux x86_64.
>>> 
>>> Now the alignment is reduced to the page size when QEMU is running on
>>> Valgrind.
>>> 
>>> valgrind.h is a copy from Valgrind svn trunk r12226 with trailing
>>> whitespace stripped but otherwise unmodified, so it still raises lots
>>> of errors when checked with scripts/checkpatch.pl.
>> 
>> Can't we just require valgrind header files to be around when kvm is 
>> enabled? I would rather not copy code from other projects. Alternatively you 
>> could take the header and shrink it down to maybe 5 lines of inline asm code 
>> that would be a lot more readable :). You're #ifdef'ing on x86_64 already 
>> anyways.
> 
> The patch is currently required for x86_64 hosts running Linux.
> I estimate that this is one of the most frequently used QEMU host platforms,
> and in most cases, KVM will be configured because this is the default
> and also because it is reasonable for this platform.
> 
> How many of these hosts will have the Valgrind header around?
> I estimate less than 20 %, so configure would have to test whether
> valgrind.h is available or not. I think providing valgrind.h is
> a much better (and simpler) solution.

Hrm. I see your point.

> Stripping valgrind.h is not a good idea: the file is specially designed
> to be included in other projects like QEMU. As soon as the 2 MiB alignment
> is used for other hosts (ppc64, ...), you would have to take more and more
> from the original code. The file was not designed to be readable.
> Although it contains lots of comments which improve readability,
> there remains code which is less easy to read. I cite one of those
> comments:
> 
> /* The following defines the magic code sequences which the JITter
>   spots and handles magically.  Don't look too closely at them as
>   they will rot your brain.
> 
> Instead of rotting my brain, I prefer using a copy of the original code.

Could we maybe use a git submodule to point to the valgrind repo and fetch it 
from there?

> 
>> 
>>> 
>>> It is included here to avoid a dependency on Valgrind.
>>> 
>>> Signed-off-by: Stefan Weil 
>>> ---
>>> oslib-posix.c | 8 +-
>>> valgrind.h | 4060 +
>>> 2 files changed, 4066 insertions(+), 2 deletions(-)
>>> create mode 100644 valgrind.h
>>> 
>>> diff --git a/oslib-posix.c b/oslib-posix.c
>>> index dbc8ee8..e503e58 100644
>>> --- a/oslib-posix.c
>>> +++ b/oslib-posix.c
>>> @@ -36,10 +36,14 @@ extern int daemon(int, int);
>>> #endif
>>> 
>>> #if defined(__linux__) && defined(__x86_64__)
>>> - /* Use 2MB alignment so transparent hugepages can be used by KVM */
>>> + /* Use 2 MiB alignment so transparent hugepages can be used by KVM.
>>> + Valgrind does not support alignments larger than 1 MiB,
>>> + therefore we need special code which handles running on Valgrind. */
>>> # define QEMU_VMALLOC_ALIGN (512 * 4096)
>>> +# include "valgrind.h" /* RUNNING_ON_VALGRIND */
>> 
>> I would prefer to just have a global variable we keep the alignment in that 
>> gets populated on initialization. That way we don't have to query valgrind 
>> or potentially query the kernel on every memalign and keep everything on a 
>> single spot.
> 
> As long as this is the only use of a Valgrind interface,
> I don't know which other single spot would be better.
> For a short time I thought about adding something to
> qemu-common.h, but I don't think that would be a good idea.

Oh, I'm not talking about the #include. What I meant was the QEMU_MALLOC_ALIGN 
constant. I don't see why that should be a define. Why not make it:

size_t vmalloc_align;

void some_init_func() {
vmalloc_align = getpagesize();

#if defined(__linux__) && defined(__x86_64__)
/* comment about 2MB page size, THP and valgrind */
if (!RUNNING_ON_VALGRIND) {
vmalloc_align = (512 * 4096);
}
#endif
}

Then this piece of code would be the only one that cares about the 
initialization of the alignment. During runtime, every user (admittedly, there 
is only one, but it still is nicer separation) of the alignment constant would 
just use the global and be done. No checks.

> 
> memalign is not time critical. The extra code added by RUNNING_ON_VALGRIND is 
> small
> (a few machine instructions, see comment in valgrind.h), and it is only 
> executed when
> a large memory block is allocated. How many calls of qemu_vmalloc with at 
> least 2 MiB
> do you expect?

I'm not thinking of time critical, but complexity. It's a lot easier to read 
qemu_vmalloc if we don't have valgrind special cases in there.

> 
> There remains an issue with my patch which I don't lik

Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind

2011-10-30 Thread Markus Armbruster
Alexander Graf  writes:

> On 30.10.2011, at 13:07, Stefan Weil wrote:
>
>> Valgrind is a tool which can automatically detect many kinds of bugs.
>> 
>> Running QEMU on Valgrind with x86_64 hosts was not possible because
>> Valgrind aborts when memalign is called with an alignment larger than
>> 1 MiB. QEMU normally uses 2 MiB on Linux x86_64.
>> 
>> Now the alignment is reduced to the page size when QEMU is running on
>> Valgrind.
>> 
>> valgrind.h is a copy from Valgrind svn trunk r12226 with trailing
>> whitespace stripped but otherwise unmodified, so it still raises lots
>> of errors when checked with scripts/checkpatch.pl.
>
> Can't we just require valgrind header files to be around when kvm is enabled? 
> I would rather not copy code from other projects. Alternatively you could 
> take the header and shrink it down to maybe 5 lines of inline asm code that 
> would be a lot more readable :). You're #ifdef'ing on x86_64 already anyways.
>
>> 
>> It is included here to avoid a dependency on Valgrind.

Our usual way to avoid a hard dependency on something we want is to
detect it in configure, then do something like

#ifdef CONFIG_VALGRIND
#include "valgrind.h"
#else
#define RUNNING_ON_VALGRIND 0
#endif

[...]



Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind

2011-10-30 Thread Markus Armbruster
Alexander Graf  writes:

> On 30.10.2011, at 15:30, Stefan Weil wrote:
>
>> Am 30.10.2011 14:41, schrieb Alexander Graf:
>>> On 30.10.2011, at 13:07, Stefan Weil wrote:
 Valgrind is a tool which can automatically detect many kinds of bugs.
 
 Running QEMU on Valgrind with x86_64 hosts was not possible because
 Valgrind aborts when memalign is called with an alignment larger than
 1 MiB. QEMU normally uses 2 MiB on Linux x86_64.
 
 Now the alignment is reduced to the page size when QEMU is running on
 Valgrind.
 
 valgrind.h is a copy from Valgrind svn trunk r12226 with trailing
 whitespace stripped but otherwise unmodified, so it still raises lots
 of errors when checked with scripts/checkpatch.pl.
>>> 
>>> Can't we just require valgrind header files to be around when kvm is 
>>> enabled? I would rather not copy code from other projects. Alternatively 
>>> you could take the header and shrink it down to maybe 5 lines of inline asm 
>>> code that would be a lot more readable :). You're #ifdef'ing on x86_64 
>>> already anyways.
>> 
>> The patch is currently required for x86_64 hosts running Linux.
>> I estimate that this is one of the most frequently used QEMU host platforms,
>> and in most cases, KVM will be configured because this is the default
>> and also because it is reasonable for this platform.
>> 
>> How many of these hosts will have the Valgrind header around?
>> I estimate less than 20 %, so configure would have to test whether
>> valgrind.h is available or not. I think providing valgrind.h is
>> a much better (and simpler) solution.
>
> Hrm. I see your point.
>
>> Stripping valgrind.h is not a good idea: the file is specially designed
>> to be included in other projects like QEMU. As soon as the 2 MiB alignment
>> is used for other hosts (ppc64, ...), you would have to take more and more
>> from the original code. The file was not designed to be readable.
>> Although it contains lots of comments which improve readability,
>> there remains code which is less easy to read. I cite one of those
>> comments:
>> 
>> /* The following defines the magic code sequences which the JITter
>>   spots and handles magically.  Don't look too closely at them as
>>   they will rot your brain.
>> 
>> Instead of rotting my brain, I prefer using a copy of the original code.

Yes, copies are evil.

> Could we maybe use a git submodule to point to the valgrind repo and fetch it 
> from there?

Anyone sophisticated enough to make use of valgrind should be able to
install valgrind.h just fine.  It's not rocket science:

# yum provides \*/valgrind.h
[...]
# yum install valgrind-devel

[...]



Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind

2011-10-31 Thread Stefan Weil

Am 31.10.2011 07:38, schrieb Markus Armbruster:

Alexander Graf  writes:

On 30.10.2011, at 13:07, Stefan Weil wrote:

Valgrind is a tool which can automatically detect many kinds of bugs.

Running QEMU on Valgrind with x86_64 hosts was not possible because
Valgrind aborts when memalign is called with an alignment larger than
1 MiB. QEMU normally uses 2 MiB on Linux x86_64.

Now the alignment is reduced to the page size when QEMU is running on
Valgrind.

valgrind.h is a copy from Valgrind svn trunk r12226 with trailing
whitespace stripped but otherwise unmodified, so it still raises lots
of errors when checked with scripts/checkpatch.pl.


Can't we just require valgrind header files to be around when kvm is 
enabled? I would rather not copy code from other projects. 
Alternatively you could take the header and shrink it down to maybe 5 
lines of inline asm code that would be a lot more readable :). You're 
#ifdef'ing on x86_64 already anyways.




It is included here to avoid a dependency on Valgrind.

Our usual way to avoid a hard dependency on something we want is to
detect it in configure, then do something like

#ifdef CONFIG_VALGRIND
#include "valgrind.h"
#else
#define RUNNING_ON_VALGRIND 0
#endif

[...]


Markus, you obviously did not read my last mail.
I know how configure works, so there is no need to teach me.

I wrote that I decided against the configure solution because
it is not adequate here. Adding a copy of valgrind.h which
is explicitly made for being copied is simpler and better:

* It avoids code in configure. There are already so many
  checks in configure that it takes a rather long time to run,
  and additional checks don't improve maintainability.

* It adds Valgrind support for any x86_64 QEMU binary
  without enforcing a build dependency on Valgrind.
  This is useful for QEMU packages in distributions.

You said that copies are evil without explaining why you
think so. What about the other copies in QEMU? There are
lots of them, and some (e.g. the Linux headers) were added
recently.

Cheers,
Stefan Weil




Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind

2011-10-31 Thread Anthony Liguori

On 10/30/2011 07:07 AM, Stefan Weil wrote:

Valgrind is a tool which can automatically detect many kinds of bugs.

Running QEMU on Valgrind with x86_64 hosts was not possible because
Valgrind aborts when memalign is called with an alignment larger than
1 MiB. QEMU normally uses 2 MiB on Linux x86_64.

Now the alignment is reduced to the page size when QEMU is running on
Valgrind.

valgrind.h is a copy from Valgrind svn trunk r12226 with trailing
whitespace stripped but otherwise unmodified, so it still raises lots
of errors when checked with scripts/checkpatch.pl.

It is included here to avoid a dependency on Valgrind.

Signed-off-by: Stefan Weil


How about we just fix valgrind?

Regards,

Anthony Liguori



Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind

2011-10-31 Thread Daniel P. Berrange
On Sun, Oct 30, 2011 at 01:07:26PM +0100, Stefan Weil wrote:
> Valgrind is a tool which can automatically detect many kinds of bugs.
> 
> Running QEMU on Valgrind with x86_64 hosts was not possible because
> Valgrind aborts when memalign is called with an alignment larger than
> 1 MiB. QEMU normally uses 2 MiB on Linux x86_64.
> 
> Now the alignment is reduced to the page size when QEMU is running on
> Valgrind.
> 
> valgrind.h is a copy from Valgrind svn trunk r12226 with trailing
> whitespace stripped but otherwise unmodified, so it still raises lots
> of errors when checked with scripts/checkpatch.pl.
> 
> It is included here to avoid a dependency on Valgrind.

In libvirt we do the following fun hack to avoid a build dep on valgrind:

const char *ld = getenv("LD_PRELOAD");
if (ld && strstr(ld, "vgpreload")) {
fprintf(stderr, "Running under valgrind, disabling driver\n");
return 0;
}

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind

2011-10-31 Thread Markus Armbruster
Stefan Weil  writes:

> Am 31.10.2011 07:38, schrieb Markus Armbruster:
>> Alexander Graf  writes:
>>> On 30.10.2011, at 13:07, Stefan Weil wrote:
 Valgrind is a tool which can automatically detect many kinds of bugs.

 Running QEMU on Valgrind with x86_64 hosts was not possible because
 Valgrind aborts when memalign is called with an alignment larger than
 1 MiB. QEMU normally uses 2 MiB on Linux x86_64.

 Now the alignment is reduced to the page size when QEMU is running on
 Valgrind.

 valgrind.h is a copy from Valgrind svn trunk r12226 with trailing
 whitespace stripped but otherwise unmodified, so it still raises lots
 of errors when checked with scripts/checkpatch.pl.
>>>
>>> Can't we just require valgrind header files to be around when kvm
>>> is enabled? I would rather not copy code from other
>>> projects. Alternatively you could take the header and shrink it
>>> down to maybe 5 lines of inline asm code that would be a lot more
>>> readable :). You're #ifdef'ing on x86_64 already anyways.
>>>

 It is included here to avoid a dependency on Valgrind.
>> Our usual way to avoid a hard dependency on something we want is to
>> detect it in configure, then do something like
>>
>> #ifdef CONFIG_VALGRIND
>> #include "valgrind.h"
>> #else
>> #define RUNNING_ON_VALGRIND 0
>> #endif
>>
>> [...]
>
> Markus, you obviously did not read my last mail.
> I know how configure works, so there is no need to teach me.

Hope I didn't offend you; sorry if I did.

> I wrote that I decided against the configure solution because
> it is not adequate here. Adding a copy of valgrind.h which
> is explicitly made for being copied is simpler and better:
>
> * It avoids code in configure. There are already so many
>   checks in configure that it takes a rather long time to run,
>   and additional checks don't improve maintainability.

Configure does what it needs to do.  If it's slow or hard to maintain,
we can discuss how to better solve the problem.  Avoiding to solve parts
of the problem doesn't seem such a good idea, though.

> * It adds Valgrind support for any x86_64 QEMU binary
>   without enforcing a build dependency on Valgrind.
>   This is useful for QEMU packages in distributions.

I can't speak for other distributions, but over here we very much prefer
build dependencies over copies.  Anyone who ever searched a bunch of
packages for copies containing a hot security hole will understand why.

> You said that copies are evil without explaining why you
> think so. What about the other copies in QEMU? There are
> lots of them, and some (e.g. the Linux headers) were added
> recently.

Copies are evil because we need to pick the one version that's right for
all our users.  Repeatedly.

That's okay (sort of) when there's a tight coupling, and there's really
only one admissible version.

Or it may be a lesser evil when the thing copied isn't readily available
(not packaged in common distros).

I can't see either of that for valgrind.h.



Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind

2011-10-31 Thread Stefan Weil

Am 31.10.2011 19:22, schrieb Daniel P. Berrange:

On Sun, Oct 30, 2011 at 01:07:26PM +0100, Stefan Weil wrote:

Valgrind is a tool which can automatically detect many kinds of bugs.

Running QEMU on Valgrind with x86_64 hosts was not possible because
Valgrind aborts when memalign is called with an alignment larger than
1 MiB. QEMU normally uses 2 MiB on Linux x86_64.

Now the alignment is reduced to the page size when QEMU is running on
Valgrind.

valgrind.h is a copy from Valgrind svn trunk r12226 with trailing
whitespace stripped but otherwise unmodified, so it still raises lots
of errors when checked with scripts/checkpatch.pl.

It is included here to avoid a dependency on Valgrind.


In libvirt we do the following fun hack to avoid a build dep on valgrind:

const char *ld = getenv("LD_PRELOAD");
if (ld && strstr(ld, "vgpreload")) {
fprintf(stderr, "Running under valgrind, disabling driver\n");
return 0;
}

Regards,
Daniel


Thanks, Daniel.

That works, although it is not the official way and it would fail
if vgpreload were renamed.

It is much slower than the offical macro, so the test would
have to be done once and save the result in a static variable.

As it solves the current problem with QEMU on Valgrind,
this solution would be better than no solution, so if more
people agree, it could be done like this.

From other mails, I expect that the 2 MiB alignment will
be used in more scenarios (any host and operating system
which supports KVM). As far as I know, Valgrind runs
on ARM, PPC, S390, BSD, ..., too, and latest valgrind.h
is designed to support all those scenarios. I have no
idea whether the vgpreload hack also works everywhere.

Regards,
Stefan




Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind

2011-10-31 Thread Stefan Weil

Am 31.10.2011 19:30, schrieb Markus Armbruster:

Copies are evil because we need to pick the one version that's right for
all our users. Repeatedly.

That's okay (sort of) when there's a tight coupling, and there's really
only one admissible version.

Or it may be a lesser evil when the thing copied isn't readily available
(not packaged in common distros).

I can't see either of that for valgrind.h.


Let me add some piece of information for the further discussion.

valgrind.h is an official interface. It is designed to be compatible
across versions.

Initially, I planned to add a valgrind.h copy from my Linux distro.
When I looked at the Valgrind repository, I noticed that the latest
version of valgrind.h had added support for more hosts (ppc, s390)
which I expect will be needed for QEMU soon, so I took this version.

So the decision which copy should be taken is simple: any copy
works as long as it supports your host.

Cheers,
Stefan Weil





Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind

2011-10-31 Thread Stefan Weil

Am 31.10.2011 19:13, schrieb Anthony Liguori:

On 10/30/2011 07:07 AM, Stefan Weil wrote:

Valgrind is a tool which can automatically detect many kinds of bugs.

Running QEMU on Valgrind with x86_64 hosts was not possible because
Valgrind aborts when memalign is called with an alignment larger than
1 MiB. QEMU normally uses 2 MiB on Linux x86_64.

Now the alignment is reduced to the page size when QEMU is running on
Valgrind.

valgrind.h is a copy from Valgrind svn trunk r12226 with trailing
whitespace stripped but otherwise unmodified, so it still raises lots
of errors when checked with scripts/checkpatch.pl.

It is included here to avoid a dependency on Valgrind.

Signed-off-by: Stefan Weil


How about we just fix valgrind?

Regards,

Anthony Liguori



Do you think that Valgrind will be fixed before tests of QEMU 1.0 start?
I don't, and I think that using Valgrind should be part of these tests!

I'd add the patch now. As soon as Valgrind is fixed, it can be reverted.
Or we add another patch with the Valgrind hack from libvirt.

Regards,
Stefan Weil




Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind

2011-10-31 Thread Anthony Liguori

On 10/31/2011 02:03 PM, Stefan Weil wrote:

Am 31.10.2011 19:13, schrieb Anthony Liguori:

On 10/30/2011 07:07 AM, Stefan Weil wrote:

Valgrind is a tool which can automatically detect many kinds of bugs.

Running QEMU on Valgrind with x86_64 hosts was not possible because
Valgrind aborts when memalign is called with an alignment larger than
1 MiB. QEMU normally uses 2 MiB on Linux x86_64.

Now the alignment is reduced to the page size when QEMU is running on
Valgrind.

valgrind.h is a copy from Valgrind svn trunk r12226 with trailing
whitespace stripped but otherwise unmodified, so it still raises lots
of errors when checked with scripts/checkpatch.pl.

It is included here to avoid a dependency on Valgrind.

Signed-off-by: Stefan Weil


How about we just fix valgrind?

Regards,

Anthony Liguori



Do you think that Valgrind will be fixed before tests of QEMU 1.0 start?
I don't, and I think that using Valgrind should be part of these tests!

I'd add the patch now. As soon as Valgrind is fixed, it can be reverted.
Or we add another patch with the Valgrind hack from libvirt.


I definitely don't want to pull in a valgrind header.  The LD_PRELOAD check 
seems a bit ugly but I'd rather carry that as an intermediate solution.


Regards,

Anthony Liguori



Regards,
Stefan Weil