Am 02.06.2015 um 12:50 hat Alberto Garcia geschrieben: > On Tue 02 Jun 2015 12:05:59 PM CEST, Kevin Wolf <kw...@redhat.com> wrote: > > >> +#include <sys/mman.h> > >> #include "block/block_int.h" > >> #include "qemu-common.h" > >> +#include "qemu/osdep.h" > >> #include "qcow2.h" > >> #include "trace.h" > > > > This breaks the mingw build: > > > > /mnt/qemu/block/qcow2-cache.c:25:22: fatal error: sys/mman.h: No such file > > or directory > > #include <sys/mman.h> > > Ok, I can use #if defined(CONFIG_MADVISE) || defined(CONFIG_POSIX_MADVISE) > as it's done in util/osdep.c for the same header. > > >> +#if QEMU_MADV_DONTNEED != QEMU_MADV_INVALID > >> + BDRVQcowState *s = bs->opaque; > >> + void *t = qcow2_cache_get_table_addr(bs, c, i); > >> + long align = sysconf(_SC_PAGESIZE); > > > > It seems that getpagesize() is usually used in qemu. > > The getpagesize() manual page actually recommends using sysconf() for > portability reasons. But other than that I don't have a problem with > getpagesize() if it's the preferred choice in QEMU.
There is a Windows implementation for getpagesize() in qemu, so I guess it's actually more portable in our context. Now, as you'll #ifdef this code out for Windows, that probably doesn't matter, so it's just about consistency. > >> + size_t mem_size = (size_t) s->cluster_size * num_tables; > >> + size_t offset = QEMU_ALIGN_UP((uintptr_t) t, align) - (uintptr_t) t; > >> + size_t length = QEMU_ALIGN_DOWN(mem_size - offset, align); > > > > Instead of all the aligning here, shouldn't we just make sure that the > > tables are created with the right alignment? > > The tables should have the right alignment and their size should be a > multiple of the page size. The latter condition is not necessarily true > (a cluster can be smaller than one page) so I'd keep that code in any > case. We could adjust the size to MAX(size, pagesize), but perhaps that wastes a bit too much memory indeed. Kevin