Re: [Qemu-devel] [PATCH 2/3] block: Add support to "open" /dev/fd/X filenames

[Kevin Wolf]

Am 04.06.2012 15:10, schrieb Corey Bryant:
> The main goal of this patch series is to enable isolation of guest
> images that are stored on the same NFS mount.  This can be achieved
> if the management application opens files for QEMU, and QEMU is
> restricted from opening files.
> 
> This patch adds support to the block layer open paths to dup(X) a
> pre-opened file descriptor if the filename is of the format
> /dev/fd/X.
> 
> One nice thing about this approach is that no new SELinux policy is
> required to prevent open of NFS files (files with type nfs_t).  The
> virt_use_nfs boolean type simply needs to be set to false, and open
> will be prevented (yet dup will be allowed).  For example:
> 
>   # setsebool virt_use_nfs 0
>   # getsebool virt_use_nfs
>   virt_use_nfs --> off
> 
> Signed-off-by: Corey Bryant <address@hidden>
> ---
>  block.c           |   15 +++++++++++++++
>  block/raw-posix.c |   20 ++++++++++----------
>  block/raw-win32.c |    4 ++--
>  block/vdi.c       |    5 +++--
>  block/vmdk.c      |   21 +++++++++------------
>  block/vpc.c       |    2 +-
>  block/vvfat.c     |   21 +++++++++++----------
>  block_int.h       |   13 +++++++++++++
>  8 files changed, 64 insertions(+), 37 deletions(-)
> 
> diff --git a/block.c b/block.c
> index af2ab4f..8416313 100644
> --- a/block.c
> +++ b/block.c
> @@ -102,6 +102,21 @@ static BlockDriverState *bs_snapshots;
>  /* If non-zero, use only whitelisted block drivers */
>  static int use_bdrv_whitelist;
>  
> +int file_open(const char *filename, int flags, mode_t mode)
> +{
> +#ifndef _WIN32
> +    int fd;
> +    const char *p;
> +
> +    if (strstart(filename, "/dev/fd/", &p)) {
> +        fd = atoi(p);
> +        return dup(fd);
> +    }
> +#endif
> +
> +    return qemu_open(filename, flags, mode);
> +}

What's the reason for having separate file_open() and qemu_open()
functions? Are there places where you don't want allow to use /dev/fd?
Otherwise I would have expected that we just extend qemu_open().

I'd have the remaining open -> qemu_open conversions as a separate patch
then. They do not only add fd support as advertised in the commit
message, but also add O_CLOEXEC. I don't think that's bad, these are
likely bonus bug fixes, but they should be mentioned in the commit message.

Kevin