qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v3 02/16] vmdk: Use BdrvChild instead of BDS for


From: Jeff Cody
Subject: Re: [Qemu-block] [PATCH v3 02/16] vmdk: Use BdrvChild instead of BDS for references to extents
Date: Mon, 12 Oct 2015 20:55:24 -0400
User-agent: Mutt/1.5.21 (2010-09-15)

On Fri, Oct 09, 2015 at 02:15:27PM +0200, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <address@hidden>
> Reviewed-by: Max Reitz <address@hidden>
> Reviewed-by: Alberto Garcia <address@hidden>
> Reviewed-by: Fam Zheng <address@hidden>
> ---
>  block/vmdk.c | 99 
> +++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 51 insertions(+), 48 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index be0d640..9702132 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -87,7 +87,7 @@ typedef struct {
>  #define L2_CACHE_SIZE 16
>  
>  typedef struct VmdkExtent {
> -    BlockDriverState *file;
> +    BdrvChild *file;
>      bool flat;
>      bool compressed;
>      bool has_marker;
> @@ -221,8 +221,8 @@ static void vmdk_free_extents(BlockDriverState *bs)
>          g_free(e->l2_cache);
>          g_free(e->l1_backup_table);
>          g_free(e->type);
> -        if (e->file != bs->file) {
> -            bdrv_unref(e->file);
> +        if (e->file != bs->file_child) {
> +            bdrv_unref_child(bs, e->file);
>          }
>      }
>      g_free(s->extents);
> @@ -367,7 +367,7 @@ static int vmdk_parent_open(BlockDriverState *bs)
>  /* Create and append extent to the extent array. Return the added VmdkExtent
>   * address. return NULL if allocation failed. */
>  static int vmdk_add_extent(BlockDriverState *bs,
> -                           BlockDriverState *file, bool flat, int64_t 
> sectors,
> +                           BdrvChild *file, bool flat, int64_t sectors,
>                             int64_t l1_offset, int64_t l1_backup_offset,
>                             uint32_t l1_size,
>                             int l2_size, uint64_t cluster_sectors,
> @@ -392,7 +392,7 @@ static int vmdk_add_extent(BlockDriverState *bs,
>          return -EFBIG;
>      }
>  
> -    nb_sectors = bdrv_nb_sectors(file);
> +    nb_sectors = bdrv_nb_sectors(file->bs);
>      if (nb_sectors < 0) {
>          return nb_sectors;
>      }
> @@ -439,14 +439,14 @@ static int vmdk_init_tables(BlockDriverState *bs, 
> VmdkExtent *extent,
>          return -ENOMEM;
>      }
>  
> -    ret = bdrv_pread(extent->file,
> +    ret = bdrv_pread(extent->file->bs,
>                       extent->l1_table_offset,
>                       extent->l1_table,
>                       l1_size);
>      if (ret < 0) {
>          error_setg_errno(errp, -ret,
>                           "Could not read l1 table from extent '%s'",
> -                         extent->file->filename);
> +                         extent->file->bs->filename);
>          goto fail_l1;
>      }
>      for (i = 0; i < extent->l1_size; i++) {
> @@ -459,14 +459,14 @@ static int vmdk_init_tables(BlockDriverState *bs, 
> VmdkExtent *extent,
>              ret = -ENOMEM;
>              goto fail_l1;
>          }
> -        ret = bdrv_pread(extent->file,
> +        ret = bdrv_pread(extent->file->bs,
>                           extent->l1_backup_table_offset,
>                           extent->l1_backup_table,
>                           l1_size);
>          if (ret < 0) {
>              error_setg_errno(errp, -ret,
>                               "Could not read l1 backup table from extent 
> '%s'",
> -                             extent->file->filename);
> +                             extent->file->bs->filename);
>              goto fail_l1b;
>          }
>          for (i = 0; i < extent->l1_size; i++) {
> @@ -485,7 +485,7 @@ static int vmdk_init_tables(BlockDriverState *bs, 
> VmdkExtent *extent,
>  }
>  
>  static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
> -                                 BlockDriverState *file,
> +                                 BdrvChild *file,
>                                   int flags, Error **errp)
>  {
>      int ret;
> @@ -493,11 +493,11 @@ static int vmdk_open_vmfs_sparse(BlockDriverState *bs,
>      VMDK3Header header;
>      VmdkExtent *extent;
>  
> -    ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
> +    ret = bdrv_pread(file->bs, sizeof(magic), &header, sizeof(header));
>      if (ret < 0) {
>          error_setg_errno(errp, -ret,
>                           "Could not read header from file '%s'",
> -                         file->filename);
> +                         file->bs->filename);
>          return ret;
>      }
>      ret = vmdk_add_extent(bs, file, false,
> @@ -559,7 +559,7 @@ static char *vmdk_read_desc(BlockDriverState *file, 
> uint64_t desc_offset,
>  }
>  
>  static int vmdk_open_vmdk4(BlockDriverState *bs,
> -                           BlockDriverState *file,
> +                           BdrvChild *file,
>                             int flags, QDict *options, Error **errp)
>  {
>      int ret;
> @@ -570,17 +570,17 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
>      BDRVVmdkState *s = bs->opaque;
>      int64_t l1_backup_offset = 0;
>  
> -    ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
> +    ret = bdrv_pread(file->bs, sizeof(magic), &header, sizeof(header));
>      if (ret < 0) {
>          error_setg_errno(errp, -ret,
>                           "Could not read header from file '%s'",
> -                         file->filename);
> +                         file->bs->filename);
>          return -EINVAL;
>      }
>      if (header.capacity == 0) {
>          uint64_t desc_offset = le64_to_cpu(header.desc_offset);
>          if (desc_offset) {
> -            char *buf = vmdk_read_desc(file, desc_offset << 9, errp);
> +            char *buf = vmdk_read_desc(file->bs, desc_offset << 9, errp);
>              if (!buf) {
>                  return -EINVAL;
>              }
> @@ -620,7 +620,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
>              } QEMU_PACKED eos_marker;
>          } QEMU_PACKED footer;
>  
> -        ret = bdrv_pread(file,
> +        ret = bdrv_pread(file->bs,
>              bs->file->total_sectors * 512 - 1536,
>              &footer, sizeof(footer));
>          if (ret < 0) {
> @@ -675,7 +675,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
>      if (le32_to_cpu(header.flags) & VMDK4_FLAG_RGD) {
>          l1_backup_offset = le64_to_cpu(header.rgd_offset) << 9;
>      }
> -    if (bdrv_nb_sectors(file) < le64_to_cpu(header.grain_offset)) {
> +    if (bdrv_nb_sectors(file->bs) < le64_to_cpu(header.grain_offset)) {
>          error_setg(errp, "File truncated, expecting at least %" PRId64 " 
> bytes",
>                     (int64_t)(le64_to_cpu(header.grain_offset)
>                               * BDRV_SECTOR_SIZE));
> @@ -739,8 +739,7 @@ static int vmdk_parse_description(const char *desc, const 
> char *opt_name,
>  }
>  
>  /* Open an extent file and append to bs array */
> -static int vmdk_open_sparse(BlockDriverState *bs,
> -                            BlockDriverState *file, int flags,
> +static int vmdk_open_sparse(BlockDriverState *bs, BdrvChild *file, int flags,
>                              char *buf, QDict *options, Error **errp)
>  {
>      uint32_t magic;
> @@ -773,10 +772,11 @@ static int vmdk_parse_extents(const char *desc, 
> BlockDriverState *bs,
>      int64_t sectors = 0;
>      int64_t flat_offset;
>      char *extent_path;
> -    BlockDriverState *extent_file;
> +    BdrvChild *extent_file;
>      BDRVVmdkState *s = bs->opaque;
>      VmdkExtent *extent;
>      char extent_opt_prefix[32];
> +    Error *local_err = NULL;
>  
>      while (*p) {
>          /* parse extent line in one of below formats:
> @@ -825,16 +825,16 @@ static int vmdk_parse_extents(const char *desc, 
> BlockDriverState *bs,
>  
>          extent_path = g_malloc0(PATH_MAX);
>          path_combine(extent_path, PATH_MAX, desc_file_path, fname);
> -        extent_file = NULL;
>  
>          ret = snprintf(extent_opt_prefix, 32, "extents.%d", s->num_extents);
>          assert(ret < 32);
>  
> -        ret = bdrv_open_image(&extent_file, extent_path, options,
> -                              extent_opt_prefix, bs, &child_file, false, 
> errp);
> +        extent_file = bdrv_open_child(extent_path, options, 
> extent_opt_prefix,
> +                                      bs, &child_file, false, &local_err);
>          g_free(extent_path);
> -        if (ret) {
> -            return ret;
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return -EINVAL;
>          }
>  
>          /* save to extents array */
> @@ -844,13 +844,13 @@ static int vmdk_parse_extents(const char *desc, 
> BlockDriverState *bs,
>              ret = vmdk_add_extent(bs, extent_file, true, sectors,
>                              0, 0, 0, 0, 0, &extent, errp);
>              if (ret < 0) {
> -                bdrv_unref(extent_file);
> +                bdrv_unref_child(bs, extent_file);
>                  return ret;
>              }
>              extent->flat_start_offset = flat_offset << 9;
>          } else if (!strcmp(type, "SPARSE") || !strcmp(type, "VMFSSPARSE")) {
>              /* SPARSE extent and VMFSSPARSE extent are both "COWD" sparse 
> file*/
> -            char *buf = vmdk_read_desc(extent_file, 0, errp);
> +            char *buf = vmdk_read_desc(extent_file->bs, 0, errp);
>              if (!buf) {
>                  ret = -EINVAL;
>              } else {
> @@ -859,13 +859,13 @@ static int vmdk_parse_extents(const char *desc, 
> BlockDriverState *bs,
>              }
>              g_free(buf);
>              if (ret) {
> -                bdrv_unref(extent_file);
> +                bdrv_unref_child(bs, extent_file);
>                  return ret;
>              }
>              extent = &s->extents[s->num_extents - 1];
>          } else {
>              error_setg(errp, "Unsupported extent type '%s'", type);
> -            bdrv_unref(extent_file);
> +            bdrv_unref_child(bs, extent_file);
>              return -ENOTSUP;
>          }
>          extent->type = g_strdup(type);
> @@ -927,7 +927,8 @@ static int vmdk_open(BlockDriverState *bs, QDict 
> *options, int flags,
>      switch (magic) {
>          case VMDK3_MAGIC:
>          case VMDK4_MAGIC:
> -            ret = vmdk_open_sparse(bs, bs->file, flags, buf, options, errp);
> +            ret = vmdk_open_sparse(bs, bs->file_child, flags, buf, options,
> +                                   errp);
>              s->desc_offset = 0x200;
>              break;
>          default:
> @@ -1028,7 +1029,7 @@ static int get_whole_cluster(BlockDriverState *bs,
>                  goto exit;
>              }
>          }
> -        ret = bdrv_write(extent->file, cluster_sector_num, whole_grain,
> +        ret = bdrv_write(extent->file->bs, cluster_sector_num, whole_grain,
>                           skip_start_sector);
>          if (ret < 0) {
>              ret = VMDK_ERROR;
> @@ -1046,7 +1047,7 @@ static int get_whole_cluster(BlockDriverState *bs,
>                  goto exit;
>              }
>          }
> -        ret = bdrv_write(extent->file, cluster_sector_num + skip_end_sector,
> +        ret = bdrv_write(extent->file->bs, cluster_sector_num + 
> skip_end_sector,
>                           whole_grain + (skip_end_sector << BDRV_SECTOR_BITS),
>                           extent->cluster_sectors - skip_end_sector);
>          if (ret < 0) {
> @@ -1066,7 +1067,7 @@ static int vmdk_L2update(VmdkExtent *extent, 
> VmdkMetaData *m_data,
>      offset = cpu_to_le32(offset);
>      /* update L2 table */
>      if (bdrv_pwrite_sync(
> -                extent->file,
> +                extent->file->bs,
>                  ((int64_t)m_data->l2_offset * 512)
>                      + (m_data->l2_index * sizeof(offset)),
>                  &offset, sizeof(offset)) < 0) {
> @@ -1076,7 +1077,7 @@ static int vmdk_L2update(VmdkExtent *extent, 
> VmdkMetaData *m_data,
>      if (extent->l1_backup_table_offset != 0) {
>          m_data->l2_offset = extent->l1_backup_table[m_data->l1_index];
>          if (bdrv_pwrite_sync(
> -                    extent->file,
> +                    extent->file->bs,
>                      ((int64_t)m_data->l2_offset * 512)
>                          + (m_data->l2_index * sizeof(offset)),
>                      &offset, sizeof(offset)) < 0) {
> @@ -1166,7 +1167,7 @@ static int get_cluster_offset(BlockDriverState *bs,
>      }
>      l2_table = extent->l2_cache + (min_index * extent->l2_size);
>      if (bdrv_pread(
> -                extent->file,
> +                extent->file->bs,
>                  (int64_t)l2_offset * 512,
>                  l2_table,
>                  extent->l2_size * sizeof(uint32_t)
> @@ -1274,7 +1275,7 @@ static int64_t coroutine_fn 
> vmdk_co_get_block_status(BlockDriverState *bs,
>          break;
>      case VMDK_OK:
>          ret = BDRV_BLOCK_DATA;
> -        if (extent->file == bs->file && !extent->compressed) {
> +        if (extent->file == bs->file_child && !extent->compressed) {
>              ret |= BDRV_BLOCK_OFFSET_VALID | offset;
>          }
>  
> @@ -1320,7 +1321,7 @@ static int vmdk_write_extent(VmdkExtent *extent, 
> int64_t cluster_offset,
>          write_len = buf_len + sizeof(VmdkGrainMarker);
>      }
>      write_offset = cluster_offset + offset_in_cluster,
> -    ret = bdrv_pwrite(extent->file, write_offset, write_buf, write_len);
> +    ret = bdrv_pwrite(extent->file->bs, write_offset, write_buf, write_len);
>  
>      write_end_sector = DIV_ROUND_UP(write_offset + write_len, 
> BDRV_SECTOR_SIZE);
>  
> @@ -1355,7 +1356,7 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t 
> cluster_offset,
>  
>  
>      if (!extent->compressed) {
> -        ret = bdrv_pread(extent->file,
> +        ret = bdrv_pread(extent->file->bs,
>                            cluster_offset + offset_in_cluster,
>                            buf, nb_sectors * 512);
>          if (ret == nb_sectors * 512) {
> @@ -1369,7 +1370,7 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t 
> cluster_offset,
>      buf_bytes = cluster_bytes * 2;
>      cluster_buf = g_malloc(buf_bytes);
>      uncomp_buf = g_malloc(cluster_bytes);
> -    ret = bdrv_pread(extent->file,
> +    ret = bdrv_pread(extent->file->bs,
>                  cluster_offset,
>                  cluster_buf, buf_bytes);
>      if (ret < 0) {
> @@ -2035,7 +2036,7 @@ static coroutine_fn int vmdk_co_flush(BlockDriverState 
> *bs)
>      int ret = 0;
>  
>      for (i = 0; i < s->num_extents; i++) {
> -        err = bdrv_co_flush(s->extents[i].file);
> +        err = bdrv_co_flush(s->extents[i].file->bs);
>          if (err < 0) {
>              ret = err;
>          }
> @@ -2055,10 +2056,10 @@ static int64_t 
> vmdk_get_allocated_file_size(BlockDriverState *bs)
>          return ret;
>      }
>      for (i = 0; i < s->num_extents; i++) {
> -        if (s->extents[i].file == bs->file) {
> +        if (s->extents[i].file == bs->file_child) {
>              continue;
>          }
> -        r = bdrv_get_allocated_file_size(s->extents[i].file);
> +        r = bdrv_get_allocated_file_size(s->extents[i].file->bs);
>          if (r < 0) {
>              return r;
>          }
> @@ -2076,7 +2077,7 @@ static int vmdk_has_zero_init(BlockDriverState *bs)
>       * return 0. */
>      for (i = 0; i < s->num_extents; i++) {
>          if (s->extents[i].flat) {
> -            if (!bdrv_has_zero_init(s->extents[i].file)) {
> +            if (!bdrv_has_zero_init(s->extents[i].file->bs)) {
>                  return 0;
>              }
>          }
> @@ -2089,7 +2090,7 @@ static ImageInfo *vmdk_get_extent_info(VmdkExtent 
> *extent)
>      ImageInfo *info = g_new0(ImageInfo, 1);
>  
>      *info = (ImageInfo){
> -        .filename         = g_strdup(extent->file->filename),
> +        .filename         = g_strdup(extent->file->bs->filename),
>          .format           = g_strdup(extent->type),
>          .virtual_size     = extent->sectors * BDRV_SECTOR_SIZE,
>          .compressed       = extent->compressed,
> @@ -2135,7 +2136,9 @@ static int vmdk_check(BlockDriverState *bs, 
> BdrvCheckResult *result,
>                      PRId64 "\n", sector_num);
>              break;
>          }
> -        if (ret == VMDK_OK && cluster_offset >= 
> bdrv_getlength(extent->file)) {
> +        if (ret == VMDK_OK &&
> +            cluster_offset >= bdrv_getlength(extent->file->bs))
> +        {
>              fprintf(stderr,
>                      "ERROR: cluster offset for sector %"
>                      PRId64 " points after EOF\n", sector_num);
> @@ -2211,7 +2214,7 @@ static void vmdk_detach_aio_context(BlockDriverState 
> *bs)
>      int i;
>  
>      for (i = 0; i < s->num_extents; i++) {
> -        bdrv_detach_aio_context(s->extents[i].file);
> +        bdrv_detach_aio_context(s->extents[i].file->bs);
>      }
>  }
>  
> @@ -2222,7 +2225,7 @@ static void vmdk_attach_aio_context(BlockDriverState 
> *bs,
>      int i;
>  
>      for (i = 0; i < s->num_extents; i++) {
> -        bdrv_attach_aio_context(s->extents[i].file, new_context);
> +        bdrv_attach_aio_context(s->extents[i].file->bs, new_context);
>      }
>  }
>  
> -- 
> 1.8.3.1
>

Reviewed-by: Jeff Cody <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]