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

[Luiz Capitulino <lcapitulino@redhat.com>]

On Thu, 14 Jun 2012 11:55:04 -0400
Corey Bryant <coreyb@linux.vnet.ibm.com> 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.

Very minor comment, but I'd exchange this with the previous patch. Because
this change is not only related to the /dev/fd/X feature and also because
as soon as you introduce the /dev/fd/X it will work for all file formats.

> 
> Signed-off-by: Corey Bryant <coreyb@linux.vnet.ibm.com>
> ---
> v2:
>  -Convert calls to qemu_open instead of file_open (kwolf@redhat.com)
>  -Mention introduction of O_CLOEXEC (kwolf@redhat.com)
> 
> v3:
>  -No changes
> 
>  block/raw-posix.c |   18 +++++++++---------
>  block/raw-win32.c |    4 ++--
>  block/vdi.c       |    5 +++--
>  block/vmdk.c      |   21 +++++++++------------
>  block/vpc.c       |    2 +-
>  block/vvfat.c     |   21 +++++++++++----------
>  6 files changed, 35 insertions(+), 36 deletions(-)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 03fcfcc..d8eff2f 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -568,8 +568,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
>          options++;
>      }
>  
> -    fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
> -              0644);
> +    fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
> +                   0644);
>      if (fd < 0) {
>          result = -errno;
>      } else {
> @@ -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);
>              if (fd < 0) {
>                  bsdPath[strlen(bsdPath)-1] = '1';
>              } else {
> @@ -798,7 +798,7 @@ static int fd_open(BlockDriverState *bs)
>  #endif
>              return -EIO;
>          }
> -        s->fd = open(bs->filename, s->open_flags & ~O_NONBLOCK);
> +        s->fd = qemu_open(bs->filename, s->open_flags & ~O_NONBLOCK);
>          if (s->fd < 0) {
>              s->fd_error_time = get_clock();
>              s->fd_got_error = 1;
> @@ -872,7 +872,7 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options)
>          options++;
>      }
>  
> -    fd = open(filename, O_WRONLY | O_BINARY);
> +    fd = qemu_open(filename, O_WRONLY | O_BINARY);
>      if (fd < 0)
>          return -errno;
>  
> @@ -950,7 +950,7 @@ static int floppy_probe_device(const char *filename)
>      if (strstart(filename, "/dev/fd", NULL))
>          prio = 50;
>  
> -    fd = open(filename, O_RDONLY | O_NONBLOCK);
> +    fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
>      if (fd < 0) {
>          goto out;
>      }
> @@ -1003,7 +1003,7 @@ static void floppy_eject(BlockDriverState *bs, bool eject_flag)
>          close(s->fd);
>          s->fd = -1;
>      }
> -    fd = open(bs->filename, s->open_flags | O_NONBLOCK);
> +    fd = qemu_open(bs->filename, s->open_flags | O_NONBLOCK);
>      if (fd >= 0) {
>          if (ioctl(fd, FDEJECT, 0) < 0)
>              perror("FDEJECT");
> @@ -1053,7 +1053,7 @@ static int cdrom_probe_device(const char *filename)
>      int prio = 0;
>      struct stat st;
>  
> -    fd = open(filename, O_RDONLY | O_NONBLOCK);
> +    fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
>      if (fd < 0) {
>          goto out;
>      }
> @@ -1177,7 +1177,7 @@ static int cdrom_reopen(BlockDriverState *bs)
>       */
>      if (s->fd >= 0)
>          close(s->fd);
> -    fd = open(bs->filename, s->open_flags, 0644);
> +    fd = qemu_open(bs->filename, s->open_flags, 0644);
>      if (fd < 0) {
>          s->fd = -1;
>          return -EIO;
> diff --git a/block/raw-win32.c b/block/raw-win32.c
> index e4b0b75..8d7838d 100644
> --- a/block/raw-win32.c
> +++ b/block/raw-win32.c
> @@ -255,8 +255,8 @@ static int raw_create(const char *filename, QEMUOptionParameter *options)
>          options++;
>      }
>  
> -    fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
> -              0644);
> +    fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
> +                   0644);
>      if (fd < 0)
>          return -EIO;
>      set_sparse(fd);
> diff --git a/block/vdi.c b/block/vdi.c
> index 119d3c7..6183fdf 100644
> --- a/block/vdi.c
> +++ 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,
> +                   0644);
>      if (fd < 0) {
>          return -errno;
>      }
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 18e9b4c..557dc1b 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1161,10 +1161,9 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
>      VMDK4Header header;
>      uint32_t tmp, magic, grains, gd_size, gt_size, gt_count;
>  
> -    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,
> +                   0644);
>      if (fd < 0) {
>          return -errno;
>      }
> @@ -1484,15 +1483,13 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
>              (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
>              total_size / (int64_t)(63 * 16 * 512));
>      if (split || flat) {
> -        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,
> +                       0644);
>      } else {
> -        fd = open(
> -                filename,
> -                O_WRONLY | O_BINARY | O_LARGEFILE,
> -                0644);
> +        fd = qemu_open(filename,
> +                       O_WRONLY | O_BINARY | O_LARGEFILE,
> +                       0644);
>      }
>      if (fd < 0) {
>          return -errno;
> diff --git a/block/vpc.c b/block/vpc.c
> index 5cd13d1..60ebf5a 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -678,7 +678,7 @@ static int vpc_create(const char *filename, QEMUOptionParameter *options)
>      }
>  
>      /* Create the file */
> -    fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
> +    fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644);
>      if (fd < 0) {
>          return -EIO;
>      }
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 0fd3367..81c3a19 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -1152,16 +1152,17 @@ static inline mapping_t* find_mapping_for_cluster(BDRVVVFATState* s,int cluster_
>  static int open_file(BDRVVVFATState* s,mapping_t* mapping)
>  {
>      if(!mapping)
> -	return -1;
> +        return -1;
>      if(!s->current_mapping ||
> -	    strcmp(s->current_mapping->path,mapping->path)) {
> -	/* open file */
> -	int fd = open(mapping->path, O_RDONLY | O_BINARY | O_LARGEFILE);
> -	if(fd<0)
> -	    return -1;
> -	vvfat_close_current_file(s);
> -	s->current_fd = fd;
> -	s->current_mapping = mapping;
> +            strcmp(s->current_mapping->path, mapping->path)) {
> +        /* open file */
> +        int fd = qemu_open(mapping->path, O_RDONLY | O_BINARY | O_LARGEFILE);
> +        if (fd < 0) {
> +            return -1;
> +        }
> +        vvfat_close_current_file(s);
> +        s->current_fd = fd;
> +        s->current_mapping = mapping;
>      }
>      return 0;
>  }
> @@ -2215,7 +2216,7 @@ static int commit_one_file(BDRVVVFATState* s,
>      for (i = s->cluster_size; i < offset; i += s->cluster_size)
>  	c = modified_fat_get(s, c);
>  
> -    fd = open(mapping->path, O_RDWR | O_CREAT | O_BINARY, 0666);
> +    fd = qemu_open(mapping->path, O_RDWR | O_CREAT | O_BINARY, 0666);
>      if (fd < 0) {
>  	fprintf(stderr, "Could not open %s... (%s, %d)\n", mapping->path,
>  		strerror(errno), errno);