On 29.04.20 13:54, Vladimir Sementsov-Ogievskiy wrote:
> 29.04.2020 14:38, Max Reitz wrote:
>> 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 <vsement...@virtuozzo.com>
>>> ---
>>> 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.
>>
>
> I just thought, why not to try another style? Are you against?
> Requirement to declare variables at start of block is obsolete, isn't it?
Oh, it absolutely is and personally I’m absolutely not against it, but
CODING_STYLE says:
> Mixed declarations (interleaving statements and declarations within
> blocks) are generally not allowed; declarations should be at the beginning
>
>
>
> of blocks.
Max
>> Reviewed-by: Max Reitz <mre...@redhat.com>
>>
>>> *task = (BlockCopyTask) {
>>> .s = s,
>>> .offset = offset,
>>
>
>
signature.asc
Description: OpenPGP digital signature
