Re: [PULL 5/5] migration: simplify migration_iteration_run()

[Juan Quintela <quintela@redhat.com>]

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
> On 30.01.23 11:03, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>>   migration/migration.c | 24 ++++++++++++------------
>>   1 file changed, 12 insertions(+), 12 deletions(-)
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 594a42f085..644c61e91d 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -3764,23 +3764,23 @@ static MigIterateState migration_iteration_run(MigrationState *s)
>>                                       pend_pre, pend_compat, pend_post);
>>       }
>>   -    if (pending_size && pending_size >= s->threshold_size) {
>> -        /* Still a significant amount to transfer */
>> -        if (!in_postcopy && pend_pre <= s->threshold_size &&
>> -            qatomic_read(&s->start_postcopy)) {
>> -            if (postcopy_start(s)) {
>> -                error_report("%s: postcopy failed to start", __func__);
>> -            }
>> -            return MIG_ITERATE_SKIP;
>> -        }
>> -        /* Just another iteration step */
>> -        qemu_savevm_state_iterate(s->to_dst_file, in_postcopy);
>> -    } else {
>> +    if (pending_size < s->threshold_size) {
>
> to keep the logic, formally it should be "if (!pending_size || pending_size < s->threshold_size)"...

And here I am, back.

To stand corrected O:-)

I have to do the change that you suggested.

Why?  Because it never ever happens with "real" migrations.
But qemu-iotest 181, well, sometimes it fails.

Never when you compile x86_64.
You need to compile all architectures.
And it fails only sometimes.

Ok, changed to
if (!pending_size || pending_size < s->threshold_size)

Thanks, Juan.

> Actually, could s->threshold_size be 0 here? Or, worth an assertion
> assert(s->threshold_size) ?

This test in particular sets the "bandwidth_limit" to 4k, that is one
page, so it could be that gets zero.

Later, Juan.