Re: [Qemu-devel] Re: [PATCH v7] Introduce qemu_madvise()

[Blue Swirl <blauwirbel@gmail.com>]

On Fri, Sep 24, 2010 at 6:08 PM, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 20.09.2010 um 22:33 schrieb Blue Swirl:
>
>> On Sun, Sep 19, 2010 at 10:11 AM, Andreas Färber <andreas.faerber@web.de>
>> wrote:
>>>
>>> From: Andreas Färber <afaerber@opensolaris.org>
>>>
>>> vl.c has a Sun-specific hack to supply a prototype for madvise(),
>>> but the call site has apparently moved to arch_init.c.
>>>
>>> Haiku doesn't implement madvise() in favor of posix_madvise().
>>> OpenBSD and Solaris 10 don't implement posix_madvise() but madvise().
>>>
>>> Check for madvise() and posix_madvise() in configure and supply
>>> qemu_madvise()
>>> as wrapper. Prefer madvise() over posix_madvise() due to flag
>>> availability.
>>> Convert all callers to use qemu_madvise() and QEMU_MADV_*.
>>>
>>> Note that on Solaris the warning is fixed by moving the madvise()
>>> prototype,
>>> not by qemu_madvise() itself. It helps with porting though, and it
>>> simplifies
>>> most call sites.
>>>
>>> v6 -> v7:
>>> * Adopt madvise() rather than posix_madvise() semantics for returning
>>> errors.
>>> * Use EINVAL in place of ENOTSUP.
>>>
>>> v5 -> v6:
>>> * Replace two leftover instances of POSIX_MADV_NORMAL with
>>> QEMU_MADV_INVALID.
>>>  Spotted by Blue Swirl.
>>>
>>> v4 -> v5:
>>> * Introduce QEMU_MADV_INVALID, suggested by Alexander Graf.
>>>  Note that this relies on -1 not being a valid advice value.
>>>
>>> v3 -> v4:
>>> * Eliminate #ifdefs at qemu_advise() call sites. Requested by Blue Swirl.
>>>  This will currently break the check in kvm-all.c by calling madvise()
>>> with
>>>  a supported flag, which will not fail. Ideas/patches welcome.
>>>
>>> v2 -> v3:
>>> * Reuse the *_MADV_* defines for QEMU_MADV_*. Suggested by Alexander
>>> Graf.
>>> * Add configure check for madvise(), too.
>>>  Add defines to Makefile, not QEMU_CFLAGS.
>>>  Convert all callers, untested. Suggested by Blue Swirl.
>>> * Keep Solaris' madvise() prototype around. Pointed out by Alexander
>>> Graf.
>>> * Display configure check results.
>>>
>>> v1 -> v2:
>>> * Don't rely on posix_madvise() availability, add qemu_madvise().
>>>  Suggested by Blue Swirl.
>>>
>>> Signed-off-by: Andreas Färber <afaerber@opensolaris.org>
>>> Cc: Blue Swirl <blauwirbel@gmail.com>
>>> Cc: Alexander Graf <agraf@suse.de>
>>> Cc: Andrea Arcangeli <aarcange@redhat.com>
>>> ---
>>>  arch_init.c         |    2 +-
>>>  configure           |   33 +++++++++++++++++++++++++++++++++
>>>  exec.c              |    8 ++------
>>>  hw/virtio-balloon.c |    4 ++--
>>>  kvm-all.c           |   12 ++++--------
>>>  osdep.c             |   20 ++++++++++++++++++++
>>>  osdep.h             |   35 +++++++++++++++++++++++++++++++++++
>>>  vl.c                |    3 ---
>>>  8 files changed, 97 insertions(+), 20 deletions(-)
>
>>> diff --git a/osdep.c b/osdep.c
>>> index 30426ff..56c6944 100644
>>> --- a/osdep.c
>>> +++ b/osdep.c
>>> @@ -28,6 +28,7 @@
>>>  #include <errno.h>
>>>  #include <unistd.h>
>>>  #include <fcntl.h>
>>> +#include <sys/mman.h>
>>
>> With the patch applied, I get a warning here with mingw from Debian
>> stable:
>>  CC    osdep.o
>> /src/qemu/osdep.c:31:22: error: sys/mman.h: No such file or directory
>>
>> For some reason, it doesn't happen with newer mingw from Debian testing.
>
> Any suggestions what to do about that? Would it work without, i.e. could we
> enclose it in #ifndef _WIN32?

There's no madvise in mingw, so either that or #if
defined(CONFIG_MADVISE) || defined(CONFIG_POSIX_MADVISE).