Re: [Qemu-devel] [PATCH 13/20] oslib: add a wrapper for mmap/munmap

[Paolo Bonzini <pbonzini@redhat.com>]

----- Messaggio originale -----
> Da: "Eric Blake" <eblake@redhat.com>
> A: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: qemu-devel@nongnu.org, kwolf@redhat.com, stefanha@redhat.com, jcody@redhat.com
> Inviato: Venerdì, 14 dicembre 2012 23:54:31
> Oggetto: Re: [PATCH 13/20] oslib: add a wrapper for mmap/munmap
> 
> On 12/12/2012 06:46 AM, Paolo Bonzini wrote:
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >  osdep.h       | 10 ++++++++++
> >  oslib-posix.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> >  oslib-win32.c | 59
> >  +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 116 insertions(+)
> > 
> 
> > +int qemu_mmap_alloc(QEMUMmapArea *mm, const char *path, size_t
> > size)
> > +{
> > +    int fd = -1;
> > +    char *mem = NULL;
> > +    int save_errno;
> > +
> > +    fd = qemu_open(path, O_RDWR | O_CREAT, 0666);
> 
> Do you want O_EXCL and/or O_TRUNC here as well?
> Or are you okay with using this to read existing contents and for the
> size to possibly discard the tail of a file?  (Hmm, thinking of how
> you plan on using persistent bitmaps, it sounds like you WANT to open
> pre-existing files; but then the caller has to be careful to pass in
> the right size).

Yes.  The caller will copy the transient bitmap to the persistent one
if it is creating a new file, and vice versa when loading.

> You're blindly unlinking here

Right, the unlink should be in the caller.

> ; sounds like you either want O_EXCL on the
> open, or need to skip the unlink() if the file was pre-existing.  No
> error checking that unlink() succeeded?

No error checking because the mmap error is more important to pass up.

> You unlink()d the file on failure during the initial map, but nowhere
> else.  Is that intentional?

Yes.

Paolo