When we start multifd, we will want to delay the main channel until
the others are created.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index d9d5415..e122684 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -358,14 +358,11 @@ static void process_incoming_migration_co(void *opaque)
 
 static void migration_incoming_setup(QEMUFile *f)
 {
-    MigrationIncomingState *mis = migration_incoming_get_current();
-
     if (multifd_load_setup() != 0) {
         /* We haven't been able to create multifd threads
            nothing better to do */
         exit(EXIT_FAILURE);
     }
-    mis->from_src_file = f;
     qemu_file_set_blocking(f, false);
 }
 
@@ -384,18 +381,26 @@ void migration_fd_process_incoming(QEMUFile *f)
 gboolean migration_ioc_process_incoming(QIOChannel *ioc)
 {
     MigrationIncomingState *mis = migration_incoming_get_current();
+    gboolean result = FALSE;
 
     if (!mis->from_src_file) {
         QEMUFile *f = qemu_fopen_channel_input(ioc);
         mis->from_src_file = f;
-        migration_fd_process_incoming(f);
-        if (!migrate_use_multifd()) {
-            return FALSE;
-        } else {
-            return TRUE;
+        migration_incoming_setup(f);
+        if (migrate_use_multifd()) {
+            result = TRUE;
         }
+    } else {
+        /* we can only arrive here if multifd is on
+           and this is a new channel */
+        result = multifd_new_channel(ioc);
     }
-    return multifd_new_channel(ioc);
+    if (result == FALSE) {
+        /* called when !multifd and for last multifd channel */
+        migration_incoming_process();
+    }
+
+    return result;
 }
 
 /*
-- 
2.9.4

* Juan Quintela (quintela@redhat.com) wrote:
> When we start multifd, we will want to delay the main channel until
> the others are created.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration/migration.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index d9d5415..e122684 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -358,14 +358,11 @@ static void process_incoming_migration_co(void *opaque)
>  
>  static void migration_incoming_setup(QEMUFile *f)
>  {
> -    MigrationIncomingState *mis = migration_incoming_get_current();
> -
>      if (multifd_load_setup() != 0) {
>          /* We haven't been able to create multifd threads
>             nothing better to do */
>          exit(EXIT_FAILURE);
>      }
> -    mis->from_src_file = f;
>      qemu_file_set_blocking(f, false);
>  }
>  
> @@ -384,18 +381,26 @@ void migration_fd_process_incoming(QEMUFile *f)
>  gboolean migration_ioc_process_incoming(QIOChannel *ioc)
>  {
>      MigrationIncomingState *mis = migration_incoming_get_current();
> +    gboolean result = FALSE;

I wonder if we need some state somewhere so that we can see that the
incoming migration is partially connected - since the main incoming
coroutine hasn't started yet, we've not got much of mis setup.

Dave

>      if (!mis->from_src_file) {
>          QEMUFile *f = qemu_fopen_channel_input(ioc);
>          mis->from_src_file = f;
> -        migration_fd_process_incoming(f);
> -        if (!migrate_use_multifd()) {
> -            return FALSE;
> -        } else {
> -            return TRUE;
> +        migration_incoming_setup(f);
> +        if (migrate_use_multifd()) {
> +            result = TRUE;
>          }
> +    } else {
> +        /* we can only arrive here if multifd is on
> +           and this is a new channel */
> +        result = multifd_new_channel(ioc);
>      }
> -    return multifd_new_channel(ioc);
> +    if (result == FALSE) {
> +        /* called when !multifd and for last multifd channel */
> +        migration_incoming_process();
> +    }
> +
> +    return result;
>  }
>  
>  /*
> -- 
> 2.9.4
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> When we start multifd, we will want to delay the main channel until
>> the others are created.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  migration/migration.c | 23 ++++++++++++++---------
>>  1 file changed, 14 insertions(+), 9 deletions(-)
>> 
>> diff --git a/migration/migration.c b/migration/migration.c
>> index d9d5415..e122684 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -358,14 +358,11 @@ static void process_incoming_migration_co(void *opaque)
>>  
>>  static void migration_incoming_setup(QEMUFile *f)
>>  {
>> -    MigrationIncomingState *mis = migration_incoming_get_current();
>> -
>>      if (multifd_load_setup() != 0) {
>>          /* We haven't been able to create multifd threads
>>             nothing better to do */
>>          exit(EXIT_FAILURE);
>>      }
>> -    mis->from_src_file = f;
>>      qemu_file_set_blocking(f, false);
>>  }
>>  
>> @@ -384,18 +381,26 @@ void migration_fd_process_incoming(QEMUFile *f)
>>  gboolean migration_ioc_process_incoming(QIOChannel *ioc)
>>  {
>>      MigrationIncomingState *mis = migration_incoming_get_current();
>> +    gboolean result = FALSE;
>
> I wonder if we need some state somewhere so that we can see that the
> incoming migration is partially connected - since the main incoming
> coroutine hasn't started yet, we've not got much of mis setup.

For other reasons this code has changed, and now this variable don't
exist.

Later, Juan.

On Mon, Jul 17, 2017 at 03:42:35PM +0200, Juan Quintela wrote:
> When we start multifd, we will want to delay the main channel until
> the others are created.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration/migration.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index d9d5415..e122684 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -358,14 +358,11 @@ static void process_incoming_migration_co(void *opaque)
>  
>  static void migration_incoming_setup(QEMUFile *f)
>  {
> -    MigrationIncomingState *mis = migration_incoming_get_current();
> -
>      if (multifd_load_setup() != 0) {
>          /* We haven't been able to create multifd threads
>             nothing better to do */
>          exit(EXIT_FAILURE);
>      }
> -    mis->from_src_file = f;

Shall we keep this, and ...

>      qemu_file_set_blocking(f, false);
>  }
>  
> @@ -384,18 +381,26 @@ void migration_fd_process_incoming(QEMUFile *f)
>  gboolean migration_ioc_process_incoming(QIOChannel *ioc)
>  {
>      MigrationIncomingState *mis = migration_incoming_get_current();
> +    gboolean result = FALSE;
>  
>      if (!mis->from_src_file) {
>          QEMUFile *f = qemu_fopen_channel_input(ioc);
>          mis->from_src_file = f;

... remove this instead?  I am not sure, but looks like RDMA is still
using migration_fd_process_incoming():

rdma_accept_incoming_migration
  migration_fd_process_incoming
    migration_incoming_setup
    migration_incoming_process
      process_incoming_migration_co <-- here we'll use from_src_file
                                        while it's not inited?

> -        migration_fd_process_incoming(f);
> -        if (!migrate_use_multifd()) {
> -            return FALSE;
> -        } else {
> -            return TRUE;
> +        migration_incoming_setup(f);
> +        if (migrate_use_multifd()) {
> +            result = TRUE;
>          }
> +    } else {
> +        /* we can only arrive here if multifd is on
> +           and this is a new channel */
> +        result = multifd_new_channel(ioc);
>      }
> -    return multifd_new_channel(ioc);
> +    if (result == FALSE) {
> +        /* called when !multifd and for last multifd channel */
> +        migration_incoming_process();
> +    }
> +
> +    return result;

-- 
Peter Xu

Peter Xu <peterx@redhat.com> wrote:
> On Mon, Jul 17, 2017 at 03:42:35PM +0200, Juan Quintela wrote:
>> When we start multifd, we will want to delay the main channel until
>> the others are created.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  migration/migration.c | 23 ++++++++++++++---------
>>  1 file changed, 14 insertions(+), 9 deletions(-)
>> 
>> diff --git a/migration/migration.c b/migration/migration.c
>> index d9d5415..e122684 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -358,14 +358,11 @@ static void process_incoming_migration_co(void *opaque)
>>  
>>  static void migration_incoming_setup(QEMUFile *f)
>>  {
>> -    MigrationIncomingState *mis = migration_incoming_get_current();
>> -
>>      if (multifd_load_setup() != 0) {
>>          /* We haven't been able to create multifd threads
>>             nothing better to do */
>>          exit(EXIT_FAILURE);
>>      }
>> -    mis->from_src_file = f;
>
> Shall we keep this, and ...
>
>>      qemu_file_set_blocking(f, false);
>>  }
>>  
>> @@ -384,18 +381,26 @@ void migration_fd_process_incoming(QEMUFile *f)
>>  gboolean migration_ioc_process_incoming(QIOChannel *ioc)
>>  {
>>      MigrationIncomingState *mis = migration_incoming_get_current();
>> +    gboolean result = FALSE;
>>  
>>      if (!mis->from_src_file) {
>>          QEMUFile *f = qemu_fopen_channel_input(ioc);
>>          mis->from_src_file = f;
>
> ... remove this instead?  I am not sure, but looks like RDMA is still
> using migration_fd_process_incoming():
>
> rdma_accept_incoming_migration
>   migration_fd_process_incoming
>     migration_incoming_setup
>     migration_incoming_process
>       process_incoming_migration_co <-- here we'll use from_src_file
>                                         while it's not inited?

Reworked all the "incoming" logic for other reasons, I *think* that now
it is correct.

Later, Juan.