Re: [PATCH] migration: Optimization about wait-unplug migration state

[zhukeqian]

On 2020/2/5 22:40, Jens Freimann wrote:
> On Tue, Feb 04, 2020 at 01:08:41PM +0800, Keqian Zhu wrote:
>> qemu_savevm_nr_failover_devices() is originally designed to
>> get the number of failover devices, but it actually returns
>> the number of "unplug-pending" failover devices now. Moreover,
>> what drives migration state to wait-unplug should be the number
>> of "unplug-pending" failover devices, not all failover devices.
>>
>> We can also notice that qemu_savevm_state_guest_unplug_pending()
>> and qemu_savevm_nr_failover_devices() is equivalent almost (from
>> the code view). So the latter is incorrect semantically and
>> useless, just delete it.
>>
>> In the qemu_savevm_state_guest_unplug_pending(), once hit a
>> unplug-pending failover device, then it can return true right
>> now to save cpu time.
>>
>> Signed-off-by: Keqian Zhu <address@hidden>
>> ---
>> Cc: address@hidden
>> Cc: Juan Quintela <address@hidden>
>> Cc: "Dr. David Alan Gilbert" <address@hidden>
>> ---
>> migration/migration.c |  2 +-
>> migration/savevm.c    | 24 +++---------------------
>> migration/savevm.h    |  1 -
>> 3 files changed, 4 insertions(+), 23 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 3a21a4686c..deedc968cf 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -3333,7 +3333,7 @@ static void *migration_thread(void *opaque)
>>
>>     qemu_savevm_state_setup(s->to_dst_file);
>>
>> -    if (qemu_savevm_nr_failover_devices()) {
>> +    if (qemu_savevm_state_guest_unplug_pending()) {
>>         migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
>>                           MIGRATION_STATUS_WAIT_UNPLUG);
>>
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index f19cb9ec7a..1d4220ece8 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1140,36 +1140,18 @@ void qemu_savevm_state_header(QEMUFile *f)
>>     }
>> }
>>
>> -int qemu_savevm_nr_failover_devices(void)
>> +bool qemu_savevm_state_guest_unplug_pending(void)
>> {
>>     SaveStateEntry *se;
>> -    int n = 0;
>>
>>     QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>>         if (se->vmsd && se->vmsd->dev_unplug_pending &&
>>             se->vmsd->dev_unplug_pending(se->opaque)) {
>> -            n++;
>> -        }
>> -    }
>> -
>> -    return n;
>> -}
>> -
>> -bool qemu_savevm_state_guest_unplug_pending(void)
>> -{
>> -    SaveStateEntry *se;
>> -    int n = 0;
>> -
>> -    QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
>> -        if (!se->vmsd || !se->vmsd->dev_unplug_pending) {
>> -            continue;
>> -        }
>> -        if (se->vmsd->dev_unplug_pending(se->opaque)) {
>> -            n++;
>> +            return true;
>>         }
>>     }
>>
>> -    return n > 0;
>> +    return false;
>> }
>>
>> void qemu_savevm_state_setup(QEMUFile *f)
>> diff --git a/migration/savevm.h b/migration/savevm.h
>> index c42b9c80ee..ba64a7e271 100644
>> --- a/migration/savevm.h
>> +++ b/migration/savevm.h
>> @@ -31,7 +31,6 @@
>>
>> bool qemu_savevm_state_blocked(Error **errp);
>> void qemu_savevm_state_setup(QEMUFile *f);
>> -int qemu_savevm_nr_failover_devices(void);
>> bool qemu_savevm_state_guest_unplug_pending(void);
>> int qemu_savevm_state_resume_prepare(MigrationState *s);
>> void qemu_savevm_state_header(QEMUFile *f);
>> -- 
>> 2.19.1
> 
> Looks good to me. I tested it and it still works, so
> Tested-by: Jens Freimann <address@hidden>
> Reviewed-by: Jens Freimann <address@hidden>
> regards
> Jens
> 
> 
> .
> 
Thanks,
Keqian.