Re: [Qemu-devel] [PATCH v2 3/8] block: Make bdrv_file_open() static

["Benoît Canet" <benoit.canet@irqsave.net>]

Le Saturday 08 Feb 2014 à 18:39:14 (+0100), Max Reitz a écrit :
> Add the bdrv_open() option BDRV_O_PROTOCOL which results in passing the
> call to bdrv_file_open(). Additionally, make bdrv_file_open() static and
> therefore bdrv_open() the only way to call it.
> 
> Consequently, all existing calls to bdrv_file_open() have to be adjusted
> to use bdrv_open() with the BDRV_O_PROTOCOL flag instead.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block.c               | 17 ++++++++++++-----
>  block/cow.c           |  6 +++---
>  block/qcow.c          |  6 +++---
>  block/qcow2.c         |  5 +++--
>  block/qed.c           |  5 +++--
>  block/sheepdog.c      |  8 +++++---
>  block/vhdx.c          |  5 +++--
>  block/vmdk.c          | 11 +++++++----
>  include/block/block.h |  6 +++---
>  qemu-io.c             |  4 +++-
>  10 files changed, 45 insertions(+), 28 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 3a32c37..432d659 100644
> --- a/block.c
> +++ b/block.c
> @@ -953,9 +953,9 @@ free_and_fail:
>   * after the call (even on failure), so if the caller intends to reuse the
>   * dictionary, it needs to use QINCREF() before calling bdrv_file_open.
>   */
> -int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> -                   const char *reference, QDict *options, int flags,
> -                   Error **errp)
> +static int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> +                          const char *reference, QDict *options, int flags,
> +                          Error **errp)
>  {
>      BlockDriverState *bs = NULL;
>      BlockDriver *drv;
> @@ -1201,8 +1201,9 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
>  
>          ret = bdrv_open(pbs, filename, NULL, image_options, flags, NULL, errp);
>      } else {
> -        ret = bdrv_file_open(pbs, filename, reference, image_options, flags,
> -                             errp);
> +        *pbs = NULL;
> +        ret = bdrv_open(pbs, filename, reference, image_options,
> +                        flags | BDRV_O_PROTOCOL, NULL, errp);
>      }
>  
>  done:
> @@ -1234,6 +1235,12 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
>  
>      assert(pbs);
>  
> +    if (flags & BDRV_O_PROTOCOL) {
> +        assert(!drv);
> +        return bdrv_file_open(pbs, filename, reference, options,
> +                              flags & ~BDRV_O_PROTOCOL, errp);
> +    }
> +
>      if (reference) {
>          bool options_non_empty = options ? qdict_size(options) : false;
>          QDECREF(options);
> diff --git a/block/cow.c b/block/cow.c
> index 7fc0b12..f0748a2 100644
> --- a/block/cow.c
> +++ b/block/cow.c
> @@ -332,7 +332,7 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
>      const char *image_filename = NULL;
>      Error *local_err = NULL;
>      int ret;
> -    BlockDriverState *cow_bs;
> +    BlockDriverState *cow_bs = NULL;
>  
>      /* Read out options */
>      while (options && options->name) {
> @@ -351,8 +351,8 @@ static int cow_create(const char *filename, QEMUOptionParameter *options,
>          return ret;
>      }
>  
> -    ret = bdrv_file_open(&cow_bs, filename, NULL, NULL, BDRV_O_RDWR,
> -                         &local_err);
> +    ret = bdrv_open(&cow_bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
>      if (ret < 0) {
>          qerror_report_err(local_err);
>          error_free(local_err);
> diff --git a/block/qcow.c b/block/qcow.c
> index 948b0c5..4881d84 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -670,7 +670,7 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
>      int flags = 0;
>      Error *local_err = NULL;
>      int ret;
> -    BlockDriverState *qcow_bs;
> +    BlockDriverState *qcow_bs = NULL;
>  
>      /* Read out options */
>      while (options && options->name) {
> @@ -691,8 +691,8 @@ static int qcow_create(const char *filename, QEMUOptionParameter *options,
>          return ret;
>      }
>  
> -    ret = bdrv_file_open(&qcow_bs, filename, NULL, NULL, BDRV_O_RDWR,
> -                         &local_err);
> +    ret = bdrv_open(&qcow_bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
>      if (ret < 0) {
>          qerror_report_err(local_err);
>          error_free(local_err);
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 6996276..bd72172 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1481,7 +1481,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
>       * 2 GB for 64k clusters, and we don't want to have a 2 GB initial file
>       * size for any qcow2 image.
>       */
> -    BlockDriverState* bs;
> +    BlockDriverState *bs = NULL;
>      QCowHeader *header;
>      uint8_t* refcount_table;
>      Error *local_err = NULL;
> @@ -1493,7 +1493,8 @@ static int qcow2_create2(const char *filename, int64_t total_size,
>          return ret;
>      }
>  
> -    ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
> +    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +                    NULL, &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
>          return ret;
> diff --git a/block/qed.c b/block/qed.c
> index b9ca7ac..33fbce1 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -571,8 +571,9 @@ static int qed_create(const char *filename, uint32_t cluster_size,
>          return ret;
>      }
>  
> -    ret = bdrv_file_open(&bs, filename, NULL, NULL,
> -                         BDRV_O_RDWR | BDRV_O_CACHE_WB, &local_err);
> +    ret = bdrv_open(&bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL, NULL,
> +                    &local_err);
>      if (ret < 0) {
>          qerror_report_err(local_err);
>          error_free(local_err);
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 672b9c9..6f2ec57 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -1534,7 +1534,8 @@ static int sd_prealloc(const char *filename)
>      Error *local_err = NULL;
>      int ret;
>  
> -    ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
> +    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +                    NULL, &local_err);
>      if (ret < 0) {
>          qerror_report_err(local_err);
>          error_free(local_err);
> @@ -1683,7 +1684,7 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
>      }
>  
>      if (backing_file) {
> -        BlockDriverState *bs;
> +        BlockDriverState *bs = NULL;
>          BDRVSheepdogState *base;
>          BlockDriver *drv;
>  
> @@ -1695,7 +1696,8 @@ static int sd_create(const char *filename, QEMUOptionParameter *options,
>              goto out;
>          }
>  
> -        ret = bdrv_file_open(&bs, backing_file, NULL, NULL, 0, &local_err);
> +        ret = bdrv_open(&bs, backing_file, NULL, NULL, BDRV_O_PROTOCOL, NULL,
> +                        &local_err);
>          if (ret < 0) {
>              qerror_report_err(local_err);
>              error_free(local_err);
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 55689cf..38d949c 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -1724,7 +1724,7 @@ static int vhdx_create(const char *filename, QEMUOptionParameter *options,
>  
>      gunichar2 *creator = NULL;
>      glong creator_items;
> -    BlockDriverState *bs;
> +    BlockDriverState *bs = NULL;
>      const char *type = NULL;
>      VHDXImageType image_type;
>      Error *local_err = NULL;
> @@ -1797,7 +1797,8 @@ static int vhdx_create(const char *filename, QEMUOptionParameter *options,
>          goto exit;
>      }
>  
> -    ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
> +    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +                    NULL, &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
>          goto exit;
> diff --git a/block/vmdk.c b/block/vmdk.c
> index e007793..70bf011 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -776,8 +776,9 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
>  
>          path_combine(extent_path, sizeof(extent_path),
>                  desc_file_path, fname);
> -        ret = bdrv_file_open(&extent_file, extent_path, NULL, NULL,
> -                             bs->open_flags, errp);
> +        extent_file = NULL;
> +        ret = bdrv_open(&extent_file, extent_path, NULL, NULL,
> +                        bs->open_flags | BDRV_O_PROTOCOL, NULL, errp);
>          if (ret) {
>              return ret;
>          }
> @@ -1493,7 +1494,8 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
>          goto exit;
>      }
>  
> -    ret = bdrv_file_open(&bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
> +    ret = bdrv_open(&bs, filename, NULL, NULL, BDRV_O_RDWR | BDRV_O_PROTOCOL,
> +                    NULL, &local_err);
>      if (ret < 0) {
>          error_propagate(errp, local_err);
>          goto exit;
> @@ -1831,7 +1833,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options,
>              goto exit;
>          }
>      }
> -    ret = bdrv_file_open(&new_bs, filename, NULL, NULL, BDRV_O_RDWR, &local_err);
> +    ret = bdrv_open(&new_bs, filename, NULL, NULL,
> +                    BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
>      if (ret < 0) {
>          error_setg_errno(errp, -ret, "Could not write description");
>          goto exit;
> diff --git a/include/block/block.h b/include/block/block.h
> index a421041..bf78db5 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -102,6 +102,9 @@ typedef enum {
>  #define BDRV_O_CHECK       0x1000  /* open solely for consistency check */
>  #define BDRV_O_ALLOW_RDWR  0x2000  /* allow reopen to change from r/o to r/w */
>  #define BDRV_O_UNMAP       0x4000  /* execute guest UNMAP/TRIM operations */
> +#define BDRV_O_PROTOCOL    0x8000  /* if no block driver is explicitly given:
> +                                      select an appropriate protocol driver,
> +                                      ignoring the format layer */
>  
>  #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
>  
> @@ -183,9 +186,6 @@ void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
>  void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
>  int bdrv_parse_cache_flags(const char *mode, int *flags);
>  int bdrv_parse_discard_flags(const char *mode, int *flags);
> -int bdrv_file_open(BlockDriverState **pbs, const char *filename,
> -                   const char *reference, QDict *options, int flags,
> -                   Error **errp);
>  int bdrv_open_image(BlockDriverState **pbs, const char *filename,
>                      QDict *options, const char *bdref_key, int flags,
>                      bool force_raw, bool allow_none, Error **errp);
> diff --git a/qemu-io.c b/qemu-io.c
> index 61d54c0..71d7806 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -59,7 +59,9 @@ static int openfile(char *name, int flags, int growable, QDict *opts)
>      }
>  
>      if (growable) {
> -        if (bdrv_file_open(&qemuio_bs, name, NULL, opts, flags, &local_err)) {
> +        if (bdrv_open(&qemuio_bs, name, NULL, opts, flags | BDRV_O_PROTOCOL,
> +                      NULL, &local_err))
> +        {
>              fprintf(stderr, "%s: can't open device %s: %s\n", progname, name,
>                      error_get_pretty(local_err));
>              error_free(local_err);
> -- 
> 1.8.5.4
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>