Re: [Qemu-devel] [PATCH 3/3] migration/savevm: move non SaveStateEntry condition check out of iteration

[Wei Yang]

On Fri, Jul 19, 2019 at 05:59:50PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (address@hidden) wrote:
>> in_postcopy and iterable_only are not SaveStateEntry specific, it would
>> be more proper to check them out of iteration.
>> 
>> Signed-off-by: Wei Yang <address@hidden>
>
>Worth it just to make that big if statement simpler!
>

Yep, the original one looks a little hard to understand.

Thanks for your comment.

>
>Reviewed-by: Dr. David Alan Gilbert <address@hidden>
>
>> ---
>>  migration/savevm.c | 13 ++++++-------
>>  1 file changed, 6 insertions(+), 7 deletions(-)
>> 
>> diff --git a/migration/savevm.c b/migration/savevm.c
>> index c41e13e322..8a2ada529e 100644
>> --- a/migration/savevm.c
>> +++ b/migration/savevm.c
>> @@ -1247,8 +1247,7 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f)
>>  }
>>  
>>  static
>> -int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool 
>> in_postcopy,
>> -                                                bool iterable_only)
>> +int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool 
>> in_postcopy)
>>  {
>>      SaveStateEntry *se;
>>      int ret;
>> @@ -1257,7 +1256,6 @@ int 
>> qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy,
>>          if (!se->ops ||
>>              (in_postcopy && se->ops->has_postcopy &&
>>               se->ops->has_postcopy(se->opaque)) ||
>> -            (in_postcopy && !iterable_only) ||
>>              !se->ops->save_live_complete_precopy) {
>>              continue;
>>          }
>> @@ -1369,10 +1367,11 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, 
>> bool iterable_only,
>>  
>>      cpu_synchronize_all_states();
>>  
>> -    ret = qemu_savevm_state_complete_precopy_iterable(f, in_postcopy,
>> -                                                      iterable_only);
>> -    if (ret) {
>> -        return ret;
>> +    if (!in_postcopy || iterable_only) {
>> +        ret = qemu_savevm_state_complete_precopy_iterable(f, in_postcopy);
>> +        if (ret) {
>> +            return ret;
>> +        }
>>      }
>>  
>>      if (iterable_only) {
>> -- 
>> 2.17.1
>> 
>--
>Dr. David Alan Gilbert / address@hidden / Manchester, UK

-- 
Wei Yang
Help you, Help me