[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v10 1/2] mirror: Rewrite mirror_iteration
From: |
Max Reitz |
Subject: |
Re: [Qemu-block] [PATCH v10 1/2] mirror: Rewrite mirror_iteration |
Date: |
Tue, 2 Feb 2016 18:20:29 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 |
On 13.01.2016 03:50, Fam Zheng wrote:
> The "pnum < nb_sectors" condition in deciding whether to actually copy
> data is unnecessarily strict, and the qiov initialization is
> unnecessarily for bdrv_aio_write_zeroes and bdrv_aio_discard.
>
> Rewrite mirror_iteration to fix both flaws.
>
> The output of iotests 109 is updated because we now report the offset
> and len slightly differently in mirroring progress.
>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
> block/mirror.c | 333
> +++++++++++++++++++++++++++------------------
> tests/qemu-iotests/109.out | 80 +++++------
> trace-events | 1 -
> 3 files changed, 242 insertions(+), 172 deletions(-)
Unfortunately, this patch now conflicts with your series that added the
@file parameter to bdrv_get_block_status():
>
> diff --git a/block/mirror.c b/block/mirror.c
> index f201f2b..7606aea 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
[...]
> @@ -158,115 +159,79 @@ static void mirror_read_complete(void *opaque, int ret)
> mirror_write_complete, op);
[...]
> +static int mirror_do_read(MirrorBlockJob *s, int64_t sector_num,
> + int nb_sectors)
> {
> BlockDriverState *source = s->common.bs;
> - int nb_sectors, sectors_per_chunk, nb_chunks, max_iov;
> - int64_t end, sector_num, next_chunk, next_sector, hbitmap_next_sector;
> - uint64_t delay_ns = 0;
> + int sectors_per_chunk, nb_chunks;
> + int ret = nb_sectors;
> MirrorOp *op;
> - int pnum;
> - int64_t ret;
I guess a
- BlockDriverState *file;
is needed here.
>
> - max_iov = MIN(source->bl.max_iov, s->target->bl.max_iov);
[...]
> /* Copy the dirty cluster. */
> s->in_flight++;
> s->sectors_in_flight += nb_sectors;
> trace_mirror_one_iteration(s, sector_num, nb_sectors);
>
> - ret = bdrv_get_block_status_above(source, NULL, sector_num,
> - nb_sectors, &pnum);
This should now be:
- nb_sectors, &pnum, &file);
> - if (ret < 0 || pnum < nb_sectors ||
> - (ret & BDRV_BLOCK_DATA && !(ret & BDRV_BLOCK_ZERO))) {
> - bdrv_aio_readv(source, sector_num, &op->qiov, nb_sectors,
> - mirror_read_complete, op);
> - } else if (ret & BDRV_BLOCK_ZERO) {
> + bdrv_aio_readv(source, sector_num, &op->qiov, nb_sectors,
> + mirror_read_complete, op);
> + return ret;
> +}
[...]
> + /* Clear dirty bits before querying the block status, because
> + * calling bdrv_get_block_status_above could yield - if some blocks are
> + * marked dirty in this window, we need to know.
> + */
> + bdrv_reset_dirty_bitmap(s->dirty_bitmap, sector_num,
> + nb_chunks * sectors_per_chunk);
> + bitmap_set(s->in_flight_bitmap, sector_num / sectors_per_chunk,
> nb_chunks);
> + while (nb_chunks > 0 && sector_num < end) {
> + int ret;
> + int io_sectors;
+ BlockDriverState *file;
> + enum MirrorMethod {
> + MIRROR_METHOD_COPY,
> + MIRROR_METHOD_ZERO,
> + MIRROR_METHOD_DISCARD
> + } mirror_method = MIRROR_METHOD_COPY;
> +
> + assert(!(sector_num % sectors_per_chunk));
> + ret = bdrv_get_block_status_above(source, NULL, sector_num,
> + nb_chunks * sectors_per_chunk,
> + &io_sectors);
+ &io_sectors, &file);
So it's not too difficult but it's not trivial either, so I was afraid
to do these changes myself while applying.
Max
> + if (ret < 0) {
> + io_sectors = nb_chunks * sectors_per_chunk;
> + }
[...]
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-block] [PATCH v10 1/2] mirror: Rewrite mirror_iteration,
Max Reitz <=