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 <richardw.yang@linux.intel.com>
---
 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

* Wei Yang (richardw.yang@linux.intel.com) 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 <richardw.yang@linux.intel.com>

Worth it just to make that big if statement simpler!


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  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 / dgilbert@redhat.com / Manchester, UK

On Fri, Jul 19, 2019 at 05:59:50PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.yang@linux.intel.com) 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 <richardw.yang@linux.intel.com>
>
>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 <dgilbert@redhat.com>
>
>> ---
>>  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 / dgilbert@redhat.com / Manchester, UK

-- 
Wei Yang
Help you, Help me