From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50574) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tjnhq-0001Cy-D3 for qemu-devel@nongnu.org; Sat, 15 Dec 2012 04:06:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tjnhp-00010U-33 for qemu-devel@nongnu.org; Sat, 15 Dec 2012 04:06:50 -0500 Received: from mx3-phx2.redhat.com ([209.132.183.24]:33469) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tjnho-00010K-Ra for qemu-devel@nongnu.org; Sat, 15 Dec 2012 04:06:49 -0500 Date: Sat, 15 Dec 2012 04:06:48 -0500 (EST) From: Paolo Bonzini Message-ID: <1454883727.24990537.1355562408421.JavaMail.root@redhat.com> In-Reply-To: <50CBAE27.1050102@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 13/20] oslib: add a wrapper for mmap/munmap List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, jcody@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com ----- Messaggio originale ----- > Da: "Eric Blake" > A: "Paolo Bonzini" > Cc: qemu-devel@nongnu.org, kwolf@redhat.com, stefanha@redhat.com, jcody@r= edhat.com > Inviato: Venerd=C3=AC, 14 dicembre 2012 23:54:31 > Oggetto: Re: [PATCH 13/20] oslib: add a wrapper for mmap/munmap >=20 > On 12/12/2012 06:46 AM, Paolo Bonzini wrote: > > Signed-off-by: Paolo Bonzini > > --- > > osdep.h | 10 ++++++++++ > > oslib-posix.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ > > oslib-win32.c | 59 > > +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 116 insertions(+) > >=20 >=20 > > +int qemu_mmap_alloc(QEMUMmapArea *mm, const char *path, size_t > > size) > > +{ > > + int fd =3D -1; > > + char *mem =3D NULL; > > + int save_errno; > > + > > + fd =3D qemu_open(path, O_RDWR | O_CREAT, 0666); >=20 > 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