[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 13/14] commit: Use BlockBackend for I/O
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH 13/14] commit: Use BlockBackend for I/O |
Date: |
Fri, 13 May 2016 17:36:57 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 |
On 04.05.2016 11:39, Kevin Wolf wrote:
> This changes the commit block job to use the job's BlockBackend for
> performing its I/O. job->bs isn't used by the commit code any more
> afterwards.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> block/commit.c | 42 +++++++++++++++++++++++++++---------------
> 1 file changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/block/commit.c b/block/commit.c
> index f308c8c..863e059 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -36,28 +36,34 @@ typedef struct CommitBlockJob {
> BlockJob common;
> RateLimit limit;
> BlockDriverState *active;
> - BlockDriverState *top;
> - BlockDriverState *base;
> + BlockBackend *top;
> + BlockBackend *base;
> BlockdevOnError on_error;
> int base_flags;
> int orig_overlay_flags;
> char *backing_file_str;
> } CommitBlockJob;
>
> -static int coroutine_fn commit_populate(BlockDriverState *bs,
> - BlockDriverState *base,
> +static int coroutine_fn commit_populate(BlockBackend *bs, BlockBackend *base,
> int64_t sector_num, int nb_sectors,
> void *buf)
> {
> int ret = 0;
> + QEMUIOVector qiov;
> + struct iovec iov = {
> + .iov_base = (void *)buf,
Why the cast?
> + .iov_len = nb_sectors * BDRV_SECTOR_SIZE,
> + };
>
> - ret = bdrv_read(bs, sector_num, buf, nb_sectors);
> - if (ret) {
> + qemu_iovec_init_external(&qiov, &iov, 1);
> +
> + ret = blk_co_readv(bs, sector_num, nb_sectors, &qiov);
> + if (ret < 0) {
> return ret;
> }
>
> - ret = bdrv_write(base, sector_num, buf, nb_sectors);
> - if (ret) {
> + ret = blk_co_writev(base, sector_num, nb_sectors, &qiov);
> + if (ret < 0) {
> return ret;
> }
>
> @@ -73,8 +79,8 @@ static void commit_complete(BlockJob *job, void *opaque)
> CommitBlockJob *s = container_of(job, CommitBlockJob, common);
> CommitCompleteData *data = opaque;
> BlockDriverState *active = s->active;
> - BlockDriverState *top = s->top;
> - BlockDriverState *base = s->base;
> + BlockDriverState *top = blk_bs(s->top);
> + BlockDriverState *base = blk_bs(s->base);
> BlockDriverState *overlay_bs;
> int ret = data->ret;
>
> @@ -94,6 +100,8 @@ static void commit_complete(BlockJob *job, void *opaque)
> bdrv_reopen(overlay_bs, s->orig_overlay_flags, NULL);
> }
> g_free(s->backing_file_str);
> + blk_unref(s->top);
> + blk_unref(s->base);
> block_job_completed(&s->common, ret);
> g_free(data);
> }
> @@ -102,8 +110,8 @@ static void coroutine_fn commit_run(void *opaque)
> {
> CommitBlockJob *s = opaque;
> CommitCompleteData *data;
> - BlockDriverState *top = s->top;
> - BlockDriverState *base = s->base;
> + BlockDriverState *top = blk_bs(s->top);
> + BlockDriverState *base = blk_bs(s->base);
I think for this function it would definitely be nicer to drop these
variables (or make them BlockBackend pointers) and use them with the
blk_* functions. The only function where this is not possible is
bdrv_is_allocated_above(), and I think it won't be too ugly to use
blk_bs() in its parameter list there.
> int64_t sector_num, end;
> int ret = 0;
> int n = 0;
> @@ -158,7 +166,7 @@ wait:
> goto wait;
> }
> }
> - ret = commit_populate(top, base, sector_num, n, buf);
> + ret = commit_populate(s->top, s->base, sector_num, n, buf);
> bytes_written += n * BDRV_SECTOR_SIZE;
> }
> if (ret < 0) {
> @@ -253,8 +261,12 @@ void commit_start(BlockDriverState *bs, BlockDriverState
> *base,
> return;
> }
>
> - s->base = base;
> - s->top = top;
> + s->base = blk_new(&error_abort);
> + blk_insert_bs(s->base, base);
> +
> + s->top = blk_new(&error_abort);
> + blk_insert_bs(s->top, top);
> +
But again, this is why I'm hesitant to give an R-b for this patch.
Max
> s->active = bs;
>
> s->base_flags = orig_base_flags;
>
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH 02/14] block: Cancel jobs first in bdrv_close_all(), (continued)
- [Qemu-devel] [PATCH 02/14] block: Cancel jobs first in bdrv_close_all(), Kevin Wolf, 2016/05/04
- [Qemu-devel] [PATCH 11/14] block: Add blk_co_readv/writev(), Kevin Wolf, 2016/05/04
- [Qemu-devel] [PATCH 10/14] backup: Remove bs parameter from backup_do_cow(), Kevin Wolf, 2016/05/04
- [Qemu-devel] [PATCH 14/14] blockjob: Remove BlockJob.bs, Kevin Wolf, 2016/05/04
- [Qemu-devel] [PATCH 13/14] commit: Use BlockBackend for I/O, Kevin Wolf, 2016/05/04
- Re: [Qemu-devel] [PATCH 13/14] commit: Use BlockBackend for I/O,
Max Reitz <=
- [Qemu-devel] [PATCH 09/14] backup: Pack Notifier within BackupBlockJob, Kevin Wolf, 2016/05/04
- [Qemu-devel] [PATCH 07/14] mirror: Use BlockBackend for I/O, Kevin Wolf, 2016/05/04
- [Qemu-devel] [PATCH 12/14] backup: Use BlockBackend for I/O, Kevin Wolf, 2016/05/04