From: Prasad Pandit <pjp@fedoraproject.org>

Remove multifd capability check with Postcopy mode.
This helps to enable both multifd and postcopy together.

Update migrate_multifd() to return false when migration
reaches Postcopy phase. In Postcopy phase, source guest
is paused, so the migration threads on the source stop
sending/pushing data on the channels. The destination
guest starts running and Postcopy threads there begin
to request/pull data from the source side.

Signed-off-by: Prasad Pandit <pjp@fedoraproject.org>
---
 migration/options.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/migration/options.c b/migration/options.c
index ad8d6989a8..47c5137d5f 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -266,7 +266,8 @@ bool migrate_multifd(void)
 {
     MigrationState *s = migrate_get_current();
 
-    return s->capabilities[MIGRATION_CAPABILITY_MULTIFD];
+    return s->capabilities[MIGRATION_CAPABILITY_MULTIFD]
+            && !migration_in_postcopy();
 }
 
 bool migrate_pause_before_switchover(void)
@@ -479,11 +480,6 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
             error_setg(errp, "Postcopy is not compatible with ignore-shared");
             return false;
         }
-
-        if (new_caps[MIGRATION_CAPABILITY_MULTIFD]) {
-            error_setg(errp, "Postcopy is not yet compatible with multifd");
-            return false;
-        }
     }
 
     if (new_caps[MIGRATION_CAPABILITY_BACKGROUND_SNAPSHOT]) {
-- 
2.47.0

On Tue, Oct 29, 2024 at 08:39:06PM +0530, Prasad Pandit wrote:
> From: Prasad Pandit <pjp@fedoraproject.org>
> 
> Remove multifd capability check with Postcopy mode.
> This helps to enable both multifd and postcopy together.
> 
> Update migrate_multifd() to return false when migration
> reaches Postcopy phase. In Postcopy phase, source guest
> is paused, so the migration threads on the source stop
> sending/pushing data on the channels. The destination
> guest starts running and Postcopy threads there begin
> to request/pull data from the source side.
> 
> Signed-off-by: Prasad Pandit <pjp@fedoraproject.org>
> ---
>  migration/options.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/migration/options.c b/migration/options.c
> index ad8d6989a8..47c5137d5f 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -266,7 +266,8 @@ bool migrate_multifd(void)
>  {
>      MigrationState *s = migrate_get_current();
>  
> -    return s->capabilities[MIGRATION_CAPABILITY_MULTIFD];
> +    return s->capabilities[MIGRATION_CAPABILITY_MULTIFD]
> +            && !migration_in_postcopy();
>  }

We need to keep this as-is.. I'm afraid.

You can always do proper check with multifd & !postcopy in your use cases.

>  
>  bool migrate_pause_before_switchover(void)
> @@ -479,11 +480,6 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
>              error_setg(errp, "Postcopy is not compatible with ignore-shared");
>              return false;
>          }
> -
> -        if (new_caps[MIGRATION_CAPABILITY_MULTIFD]) {
> -            error_setg(errp, "Postcopy is not yet compatible with multifd");
> -            return false;
> -        }
>      }
>  
>      if (new_caps[MIGRATION_CAPABILITY_BACKGROUND_SNAPSHOT]) {
> -- 
> 2.47.0
> 

-- 
Peter Xu

On Fri, 1 Nov 2024 at 20:06, Peter Xu <peterx@redhat.com> wrote:
> > -    return s->capabilities[MIGRATION_CAPABILITY_MULTIFD];
> > +    return s->capabilities[MIGRATION_CAPABILITY_MULTIFD]
> > +            && !migration_in_postcopy();
> >  }
>
> We need to keep this as-is.. I'm afraid.
> You can always do proper check with multifd & !postcopy in your use cases.

* Above change simplifies it a lot. Separate checks as
migrate_multifd() && !migration_in_postcopy() make it more complicated
to follow, because migrate_multifd() is often combined with other
checks like migrate_multifd_flush, or migrate_mapped_ram etc. I was
hoping to avoid adding one more check to those conditionals. Also,
with the above change we don't have to explicitly check where to add
!migration_in_postcopy() check.

* Will try to separate them.

Thank you.
---
  - Prasad

On Mon, Nov 04, 2024 at 05:53:22PM +0530, Prasad Pandit wrote:
> On Fri, 1 Nov 2024 at 20:06, Peter Xu <peterx@redhat.com> wrote:
> > > -    return s->capabilities[MIGRATION_CAPABILITY_MULTIFD];
> > > +    return s->capabilities[MIGRATION_CAPABILITY_MULTIFD]
> > > +            && !migration_in_postcopy();
> > >  }
> >
> > We need to keep this as-is.. I'm afraid.
> > You can always do proper check with multifd & !postcopy in your use cases.
> 
> * Above change simplifies it a lot. Separate checks as
> migrate_multifd() && !migration_in_postcopy() make it more complicated
> to follow, because migrate_multifd() is often combined with other
> checks like migrate_multifd_flush, or migrate_mapped_ram etc. I was
> hoping to avoid adding one more check to those conditionals. Also,
> with the above change we don't have to explicitly check where to add
> !migration_in_postcopy() check.

We definitely need a helper like this to simply detect what the user chose
on the feature.

You can still introduce a new helper, e.g. migrate_multifd_precopy(), if
that simplifies the code.

Thanks,

> 
> * Will try to separate them.
> 
> Thank you.
> ---
>   - Prasad
> 

-- 
Peter Xu

On Mon, 4 Nov 2024 at 22:23, Peter Xu <peterx@redhat.com> wrote:
> We definitely need a helper like this to simply detect what the user chose
> on the feature.
>
> You can still introduce a new helper, e.g. migrate_multifd_precopy(), if
> that simplifies the code.

Okay. Thank you.
---
  - Prasad