Re: [PATCH v3 03/17] migration: Block migration comment or code is wrong

[Vladimir Sementsov-Ogievskiy]

On 11/3/22 19:16, Avihai Horon wrote:
> From: Juan Quintela <quintela@redhat.com>
>
> And it appears that what is wrong is the code. During bulk stage we
> need to make sure that some block is dirty, but no games with
> max_size at all.

:) That made me interested in, why we need this one block, so I decided to 
search through the history.

And what I see? Haha, that was my commit 04636dc410b163c "migration/block: fix 
pending() return value" [1], which you actually revert with this patch.

So, at least we should note, that it's a revert of [1].

Still that this will reintroduce the bug fixed by [1].

As I understand the problem is (was) that in block_save_complete() we finalize 
only dirty blocks, but don't finalize the bulk phase if it's not finalized yet. 
So, we can fix block_save_complete() to finalize the bulk phase, instead of 
hacking with pending in [1].

Interesting, why we need this one block, described in the comment you refer to? 
Was it an incomplete workaround for the same problem, described in [1]? If so, 
we can fix block_save_complete() and remove this if() together with the comment 
from block_save_pending().

> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   migration/block.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/migration/block.c b/migration/block.c
> index b3d680af75..39ce4003c6 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -879,8 +879,8 @@ static void block_save_pending(void *opaque, uint64_t 
> max_size,
>       blk_mig_unlock();
>   
>       /* Report at least one block pending during bulk phase */
> -    if (pending <= max_size && !block_mig_state.bulk_completed) {
> -        pending = max_size + BLK_MIG_BLOCK_SIZE;
> +    if (!pending && !block_mig_state.bulk_completed) {
> +        pending = BLK_MIG_BLOCK_SIZE;
>       }
>   
>       trace_migration_block_save_pending(pending);

--
Best regards,
Vladimir