Re: [PATCH v5 03/14] migration: Simplify migration_iteration_run()

2023-01-09 Thread Cédric Le Goater 

On 12/29/22 12:03, Avihai Horon wrote:

From: Juan Quintela 

Signed-off-by: Juan Quintela 
Signed-off-by: Avihai Horon 




Reviewed-by: Cédric Le Goater 

Thanks,

C.




---
  migration/migration.c | 25 +
  1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 9795d0ec5c..61b9ce0fe8 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3758,23 +3758,24 @@ static MigIterateState 
migration_iteration_run(MigrationState *s)
  trace_migrate_pending(pending_size, s->threshold_size,
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(>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 || pending_size < s->threshold_size) {
  trace_migration_thread_low_pending(pending_size);
  migration_completion(s);
  return MIG_ITERATE_BREAK;
  }
  
+/* Still a significant amount to transfer */

+if (!in_postcopy && pend_pre <= s->threshold_size &&
+qatomic_read(>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);
  return MIG_ITERATE_RESUME;
  }

Re: [PATCH v5 03/14] migration: Simplify migration_iteration_run()

2023-01-08 Thread Avihai Horon 



On 06/01/2023 19:56, Alex Williamson wrote:

External email: Use caution opening links or attachments


On Thu, 29 Dec 2022 13:03:34 +0200
Avihai Horon  wrote:


From: Juan Quintela 

IMHO, there should always be a commit log description.  Why is this a
simplification?


Yes. It just rephrases the condition so we can drop the else {}.


   What does it allow us to do?  Nothing later obviously
depends on this, why is it part of this series?


Right.
This is part of Juan's RFC [1].
Eventually we didn't need the RFC because of the new MIG_DATA_SIZE ioctl.
Since Juan already did this work, I thought I might as well take some of 
these patches.


I will simply drop it in next version.
If a real need to simplify this if else condition arises in the future, 
it can be done then.


Thanks.

[1] 
https://lore.kernel.org/qemu-devel/20221003031600.20084-1-quint...@redhat.com/



   Thanks,

Alex


Signed-off-by: Juan Quintela 
Signed-off-by: Avihai Horon 
---
  migration/migration.c | 25 +
  1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 9795d0ec5c..61b9ce0fe8 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3758,23 +3758,24 @@ static MigIterateState 
migration_iteration_run(MigrationState *s)
  trace_migrate_pending(pending_size, s->threshold_size,
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(>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 || pending_size < s->threshold_size) {
  trace_migration_thread_low_pending(pending_size);
  migration_completion(s);
  return MIG_ITERATE_BREAK;
  }

+/* Still a significant amount to transfer */
+if (!in_postcopy && pend_pre <= s->threshold_size &&
+qatomic_read(>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);
  return MIG_ITERATE_RESUME;
  }

Re: [PATCH v5 03/14] migration: Simplify migration_iteration_run()

2023-01-06 Thread Alex Williamson 
On Thu, 29 Dec 2022 13:03:34 +0200
Avihai Horon  wrote:

> From: Juan Quintela 

IMHO, there should always be a commit log description.  Why is this a
simplification?  What does it allow us to do?  Nothing later obviously
depends on this, why is it part of this series?  Thanks,

Alex

> Signed-off-by: Juan Quintela 
> Signed-off-by: Avihai Horon 
> ---
>  migration/migration.c | 25 +
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 9795d0ec5c..61b9ce0fe8 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3758,23 +3758,24 @@ static MigIterateState 
> migration_iteration_run(MigrationState *s)
>  trace_migrate_pending(pending_size, s->threshold_size,
>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(>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 || pending_size < s->threshold_size) {
>  trace_migration_thread_low_pending(pending_size);
>  migration_completion(s);
>  return MIG_ITERATE_BREAK;
>  }
>  
> +/* Still a significant amount to transfer */
> +if (!in_postcopy && pend_pre <= s->threshold_size &&
> +qatomic_read(>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);
>  return MIG_ITERATE_RESUME;
>  }
>

[PATCH v5 03/14] migration: Simplify migration_iteration_run()

2022-12-29 Thread Avihai Horon 
From: Juan Quintela 

Signed-off-by: Juan Quintela 
Signed-off-by: Avihai Horon 
---
 migration/migration.c | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 9795d0ec5c..61b9ce0fe8 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3758,23 +3758,24 @@ static MigIterateState 
migration_iteration_run(MigrationState *s)
 trace_migrate_pending(pending_size, s->threshold_size,
   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(>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 || pending_size < s->threshold_size) {
 trace_migration_thread_low_pending(pending_size);
 migration_completion(s);
 return MIG_ITERATE_BREAK;
 }
 
+/* Still a significant amount to transfer */
+if (!in_postcopy && pend_pre <= s->threshold_size &&
+qatomic_read(>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);
 return MIG_ITERATE_RESUME;
 }
 
-- 
2.26.3