[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 05/16] block: Convert bs->file to BdrvChild
From: |
Jeff Cody |
Subject: |
Re: [Qemu-block] [PATCH v3 05/16] block: Convert bs->file to BdrvChild |
Date: |
Mon, 12 Oct 2015 21:33:06 -0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, Oct 09, 2015 at 02:15:30PM +0200, Kevin Wolf wrote:
> This patch removes the temporary duplication between bs->file and
> bs->file_child by converting everything to BdrvChild.
>
> 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.c | 63
> ++++++++++++++++++++++-------------------------
> block/blkdebug.c | 32 +++++++++++++-----------
> block/blkverify.c | 33 ++++++++++++++-----------
> block/bochs.c | 8 +++---
> block/cloop.c | 10 ++++----
> block/dmg.c | 20 +++++++--------
> block/io.c | 50 ++++++++++++++++++-------------------
> block/parallels.c | 38 ++++++++++++++--------------
> block/qapi.c | 2 +-
> block/qcow.c | 42 ++++++++++++++++---------------
> block/qcow2-cache.c | 11 +++++----
> block/qcow2-cluster.c | 38 ++++++++++++++++------------
> block/qcow2-refcount.c | 45 +++++++++++++++++----------------
> block/qcow2-snapshot.c | 30 +++++++++++-----------
> block/qcow2.c | 62 ++++++++++++++++++++++++----------------------
> block/qed-table.c | 4 +--
> block/qed.c | 32 ++++++++++++------------
> block/raw_bsd.c | 40 +++++++++++++++---------------
> block/snapshot.c | 12 ++++-----
> block/vdi.c | 17 +++++++------
> block/vhdx-log.c | 25 ++++++++++---------
> block/vhdx.c | 36 ++++++++++++++-------------
> block/vmdk.c | 27 ++++++++++----------
> block/vpc.c | 34 +++++++++++++------------
> include/block/block.h | 8 +++++-
> include/block/block_int.h | 3 +--
> 26 files changed, 378 insertions(+), 344 deletions(-)
>
[snip lots of code]
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index bc247f4..117fce6 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -427,10 +427,10 @@ static int blkdebug_open(BlockDriverState *bs, QDict
> *options, int flags,
> s->state = 1;
>
> /* Open the backing file */
> - assert(bs->file == NULL);
> - ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-image"), options,
> "image",
> - bs, &child_file, false, &local_err);
> - if (ret < 0) {
> + bs->file = bdrv_open_child(qemu_opt_get(opts, "x-image"), options,
> "image",
> + bs, &child_file, false, &local_err);
> + if (local_err) {
> + ret = -EINVAL;
> error_propagate(errp, local_err);
> goto out;
> }
> @@ -449,7 +449,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict
> *options, int flags,
> goto out;
>
> fail_unref:
> - bdrv_unref(bs->file);
> + bdrv_unref(bs->file->bs);
Would it be better to use bdrv_unref_child() here?
[snip lots of code]
> diff --git a/block/qcow.c b/block/qcow.c
> index 6e35db1..4d20cd5 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -100,7 +100,7 @@ static int qcow_open(BlockDriverState *bs, QDict
> *options, int flags,
> int ret;
> QCowHeader header;
>
> - ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
> + ret = bdrv_pread(bs->file->bs, 0, &header, sizeof(header));
> if (ret < 0) {
> goto fail;
> }
> @@ -193,7 +193,7 @@ static int qcow_open(BlockDriverState *bs, QDict
> *options, int flags,
> goto fail;
> }
>
> - ret = bdrv_pread(bs->file, s->l1_table_offset, s->l1_table,
> + ret = bdrv_pread(bs->file->bs, s->l1_table_offset, s->l1_table,
> s->l1_size * sizeof(uint64_t));
> if (ret < 0) {
> goto fail;
> @@ -205,7 +205,7 @@ static int qcow_open(BlockDriverState *bs, QDict
> *options, int flags,
>
> /* alloc L2 cache (max. 64k * 16 * 8 = 8 MB) */
> s->l2_cache =
> - qemu_try_blockalign(bs->file,
> + qemu_try_blockalign(bs->file->bs,
> s->l2_size * L2_CACHE_SIZE * sizeof(uint64_t));
> if (s->l2_cache == NULL) {
> error_setg(errp, "Could not allocate L2 table cache");
> @@ -224,7 +224,7 @@ static int qcow_open(BlockDriverState *bs, QDict
> *options, int flags,
> ret = -EINVAL;
> goto fail;
> }
> - ret = bdrv_pread(bs->file, header.backing_file_offset,
> + ret = bdrv_pread(bs->file->bs, header.backing_file_offset,
> bs->backing_file, len);
> if (ret < 0) {
> goto fail;
> @@ -369,13 +369,13 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
> if (!allocate)
> return 0;
> /* allocate a new l2 entry */
> - l2_offset = bdrv_getlength(bs->file);
> + l2_offset = bdrv_getlength(bs->file->bs);
> /* round to cluster size */
> l2_offset = (l2_offset + s->cluster_size - 1) & ~(s->cluster_size -
> 1);
> /* update the L1 entry */
> s->l1_table[l1_index] = l2_offset;
> tmp = cpu_to_be64(l2_offset);
> - if (bdrv_pwrite_sync(bs->file,
> + if (bdrv_pwrite_sync(bs->file->bs,
> s->l1_table_offset + l1_index * sizeof(tmp),
> &tmp, sizeof(tmp)) < 0)
> return 0;
> @@ -405,11 +405,12 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
> l2_table = s->l2_cache + (min_index << s->l2_bits);
> if (new_l2_table) {
> memset(l2_table, 0, s->l2_size * sizeof(uint64_t));
> - if (bdrv_pwrite_sync(bs->file, l2_offset, l2_table,
> + if (bdrv_pwrite_sync(bs->file->bs, l2_offset, l2_table,
> s->l2_size * sizeof(uint64_t)) < 0)
> return 0;
> } else {
> - if (bdrv_pread(bs->file, l2_offset, l2_table, s->l2_size *
> sizeof(uint64_t)) !=
> + if (bdrv_pread(bs->file->bs, l2_offset, l2_table,
> + s->l2_size * sizeof(uint64_t)) !=
> s->l2_size * sizeof(uint64_t))
> return 0;
> }
> @@ -430,20 +431,21 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
> overwritten */
> if (decompress_cluster(bs, cluster_offset) < 0)
> return 0;
> - cluster_offset = bdrv_getlength(bs->file);
> + cluster_offset = bdrv_getlength(bs->file->bs);
> cluster_offset = (cluster_offset + s->cluster_size - 1) &
> ~(s->cluster_size - 1);
> /* write the cluster content */
> - if (bdrv_pwrite(bs->file, cluster_offset, s->cluster_cache,
> s->cluster_size) !=
> + if (bdrv_pwrite(bs->file->bs, cluster_offset, s->cluster_cache,
> + s->cluster_size) !=
> s->cluster_size)
> return -1;
> } else {
> - cluster_offset = bdrv_getlength(bs->file);
> + cluster_offset = bdrv_getlength(bs->file->bs);
> if (allocate == 1) {
> /* round to cluster size */
> cluster_offset = (cluster_offset + s->cluster_size - 1) &
> ~(s->cluster_size - 1);
> - bdrv_truncate(bs->file, cluster_offset + s->cluster_size);
> + bdrv_truncate(bs->file->bs, cluster_offset +
> s->cluster_size);
> /* if encrypted, we must initialize the cluster
> content which won't be written */
> if (bs->encrypted &&
> @@ -463,7 +465,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
> errno = EIO;
> return -1;
> }
> - if (bdrv_pwrite(bs->file, cluster_offset + i *
> 512,
> + if (bdrv_pwrite(bs->file->bs, cluster_offset + i
> * 512,
This exceeds 80 characters... but qcow.c has numerous style issues, so
I am not bothered by it.
[snip lots of code]
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 6ede629..7844f8e 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -72,7 +72,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t
> min_size,
> #endif
>
> new_l1_size2 = sizeof(uint64_t) * new_l1_size;
> - new_l1_table = qemu_try_blockalign(bs->file,
> + new_l1_table = qemu_try_blockalign(bs->file->bs,
> align_offset(new_l1_size2, 512));
> if (new_l1_table == NULL) {
> return -ENOMEM;
> @@ -105,7 +105,8 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t
> min_size,
> BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_WRITE_TABLE);
> for(i = 0; i < s->l1_size; i++)
> new_l1_table[i] = cpu_to_be64(new_l1_table[i]);
> - ret = bdrv_pwrite_sync(bs->file, new_l1_table_offset, new_l1_table,
> new_l1_size2);
> + ret = bdrv_pwrite_sync(bs->file->bs, new_l1_table_offset,
> + new_l1_table, new_l1_size2);
> if (ret < 0)
> goto fail;
> for(i = 0; i < s->l1_size; i++)
> @@ -115,7 +116,8 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t
> min_size,
> BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_ACTIVATE_TABLE);
> cpu_to_be32w((uint32_t*)data, new_l1_size);
> stq_be_p(data + 4, new_l1_table_offset);
> - ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, l1_size),
> data,sizeof(data));
> + ret = bdrv_pwrite_sync(bs->file->bs, offsetof(QCowHeader, l1_size),
> + data, sizeof(data));
> if (ret < 0) {
> goto fail;
> }
> @@ -182,8 +184,9 @@ int qcow2_write_l1_entry(BlockDriverState *bs, int
> l1_index)
> }
>
> BLKDBG_EVENT(bs->file, BLKDBG_L1_UPDATE);
> - ret = bdrv_pwrite_sync(bs->file, s->l1_table_offset + 8 * l1_start_index,
> - buf, sizeof(buf));
> + ret = bdrv_pwrite_sync(bs->file->bs,
> + s->l1_table_offset + 8 * l1_start_index,
> + buf, sizeof(buf));
> if (ret < 0) {
> return ret;
> }
> @@ -440,7 +443,8 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs,
> }
>
> BLKDBG_EVENT(bs->file, BLKDBG_COW_WRITE);
> - ret = bdrv_co_writev(bs->file, (cluster_offset >> 9) + n_start, n,
> &qiov);
> + ret = bdrv_co_writev(bs->file->bs, (cluster_offset >> 9) + n_start, n,
> + &qiov);
> if (ret < 0) {
> goto out;
> }
> @@ -817,7 +821,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs,
> QCowL2Meta *m)
>
> /*
> * If this was a COW, we need to decrease the refcount of the old
> cluster.
> - * Also flush bs->file to get the right order for L2 and refcount update.
> + * Also flush bs->file->bs to get the right order for L2 and refcount
> update.
Over by 1 character :). Up to you if you want to fix it.
[snip lots of code]
> diff --git a/include/block/block.h b/include/block/block.h
> index 2dd6630..7ebb35d 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -585,7 +585,13 @@ typedef enum {
> BLKDBG_EVENT_MAX,
> } BlkDebugEvent;
>
> -#define BLKDBG_EVENT(bs, evt) bdrv_debug_event(bs, evt)
> +#define BLKDBG_EVENT(child, evt) \
> + do { \
> + if (child) { \
> + bdrv_debug_event(child->bs, evt); \
> + } \
> + } while(0)
According to style guidelines, this should have a space: while (0).
Again, your choice if you want to fix it or not.
- [Qemu-block] [PATCH v3 00/16] block: Get rid of bdrv_swap(), Kevin Wolf, 2015/10/09
- [Qemu-block] [PATCH v3 10/16] block/io: Make bdrv_requests_pending() public, Kevin Wolf, 2015/10/09
- [Qemu-block] [PATCH v3 11/16] block-backend: Add blk_set_bs(), Kevin Wolf, 2015/10/09
- [Qemu-block] [PATCH v3 14/16] blockjob: Store device name at job creation, Kevin Wolf, 2015/10/09
- [Qemu-block] [PATCH v3 01/16] block: Introduce BDS.file_child, Kevin Wolf, 2015/10/09