Re: [Qemu-devel] [PATCH v3 4/5] block: Convert open calls to qemu_open

[Corey Bryant]

On 06/15/2012 11:21 AM, Eric Blake wrote:
> On 06/14/2012 09:55 AM, Corey Bryant wrote:
> > This patch converts all block layer open calls to qemu_open.  This
> > enables all block layer open paths to dup(X) a pre-opened file
> > descriptor if the filename is of the format /dev/fd/X.  This is
> > useful if QEMU is restricted from opening certain files.
> >
> > Note that this adds the O_CLOEXEC flag to the changed open paths
> > when the O_CLOEXEC macro is defined.
>
> > @@ -741,7 +741,7 @@ static int hdev_open(BlockDriverState *bs, const char 
> > *filename, int flags)
> >           if ( bsdPath[ 0 ] != '\0' ) {
> >               strcat(bsdPath,"s0");
> >               /* some CDs don't have a partition 0 */
> > -            fd = open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
> > +            fd = qemu_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE);
>
> Why are we even bothering with O_LARGEFILE?  Shouldn't we be compiling
> with large file support always enabled, so that we are always calling
> open64 in the cases where it matters, without having to explicitly add
> the O_LARGEFILE flag ourselves?

That might be a no-op too since large file support looks to already be 
set at compile time:

257 QEMU_CFLAGS="-D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
-D_LARGEFILE_SOURCE $QEMU_CFLAGS"

> > +++ b/block/vdi.c
> > @@ -648,8 +648,9 @@ static int vdi_create(const char *filename, 
> > QEMUOptionParameter *options)
> >           options++;
> >       }
> >
> > -    fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
> > -              0644);
> > +    fd = qemu_open(filename,
> > +                   O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
>
> More instances.  In fact, scrubbing O_LARGEFILE, is probably worth a
> separate patch.

I'm happy to do it.  Kevin or anyone else want to weigh in first?

--
Regards,
Corey