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

[Andreas Färber]

Am 20.09.2010 um 22:33 schrieb Blue Swirl:

> On Sun, Sep 19, 2010 at 10:11 AM, Andreas Färber <address@hidden 
> > wrote:
> > From: Andreas Färber <address@hidden>
> >
> > 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 <address@hidden>
> > Cc: Blue Swirl <address@hidden>
> > Cc: Alexander Graf <address@hidden>
> > Cc: Andrea Arcangeli <address@hidden>
> > ---
> >  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?

Andreas