From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46763) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZlnsZ-0004Zi-UE for qemu-devel@nongnu.org; Mon, 12 Oct 2015 20:55:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZlnsW-0004J9-Km for qemu-devel@nongnu.org; Mon, 12 Oct 2015 20:55:47 -0400 Date: Mon, 12 Oct 2015 20:55:24 -0400 From: Jeff Cody Message-ID: <20151013005524.GI11943@localhost.localdomain> References: <1444392941-28704-1-git-send-email-kwolf@redhat.com> <1444392941-28704-3-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1444392941-28704-3-git-send-email-kwolf@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 02/16] vmdk: Use BdrvChild instead of BDS for references to extents List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: berto@igalia.com, qemu-block@nongnu.org, qemu-devel@nongnu.org, armbru@redhat.com, stefanha@redhat.com, famz@redhat.com, mreitz@redhat.com On Fri, Oct 09, 2015 at 02:15:27PM +0200, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf > Reviewed-by: Max Reitz > Reviewed-by: Alberto Garcia > Reviewed-by: Fam Zheng > --- > 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