From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:53059) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RKvMw-0007xj-9p for qemu-devel@nongnu.org; Mon, 31 Oct 2011 13:09:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RKvMv-00015j-6d for qemu-devel@nongnu.org; Mon, 31 Oct 2011 13:09:54 -0400 Received: from v220110690675601.yourvserver.net ([78.47.199.172]:37292) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RKvMv-00015V-0o for qemu-devel@nongnu.org; Mon, 31 Oct 2011 13:09:53 -0400 Message-ID: <4EAED62D.3080602@weilnetz.de> Date: Mon, 31 Oct 2011 18:09:01 +0100 From: Stefan Weil MIME-Version: 1.0 References: <1319976446-12602-1-git-send-email-sw@weilnetz.de> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Support running QEMU on Valgrind List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Avi Kivity , Alexander Graf , qemu-devel@nongnu.org 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