On 29.04.20 08:10, Vladimir Sementsov-Ogievskiy wrote: > Instead of just relying on the comment "Called only on full-dirty > region" in block_copy_task_create() let's move initial dirty area > search directly to block_copy_task_create(). Let's also use effective > bdrv_dirty_bitmap_next_dirty_area instead of looping through all > non-dirty clusters. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> > --- > block/block-copy.c | 78 ++++++++++++++++++++++++++-------------------- > 1 file changed, 44 insertions(+), 34 deletions(-) > > diff --git a/block/block-copy.c b/block/block-copy.c > index 35ff9cc3ef..5cf032c4d8 100644 > --- a/block/block-copy.c > +++ b/block/block-copy.c
[...]
> @@ -106,17 +111,27 @@ static bool coroutine_fn
> block_copy_wait_one(BlockCopyState *s, int64_t offset,
> return true;
> }
>
> -/* Called only on full-dirty region */
> +/*
> + * Search for the first dirty area in offset/bytes range and create task at
> + * the beginning of it.
Oh, that’s even better.
> + */
> static BlockCopyTask *block_copy_task_create(BlockCopyState *s,
> int64_t offset, int64_t bytes)
> {
> - BlockCopyTask *task = g_new(BlockCopyTask, 1);
> + if (!bdrv_dirty_bitmap_next_dirty_area(s->copy_bitmap,
> + offset, offset + bytes,
> + s->copy_size, &offset, &bytes))
> + {
> + return NULL;
> + }
>
> + /* region is dirty, so no existent tasks possible in it */
> assert(!find_conflicting_task(s, offset, bytes));
>
> bdrv_reset_dirty_bitmap(s->copy_bitmap, offset, bytes);
> s->in_flight_bytes += bytes;
>
> + BlockCopyTask *task = g_new(BlockCopyTask, 1);
This should be declared at the top of the function.
With that fixed:
Reviewed-by: Max Reitz <[email protected]>
> *task = (BlockCopyTask) {
> .s = s,
> .offset = offset,
signature.asc
Description: OpenPGP digital signature
