Re: [PATCH v4 02/18] migration/rdma: judge whether or not the RDMA is used for migration

[Zheng Chuan]

On 2021/2/4 1:49, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> Add enabled_rdma_migration into MigrationState to judge
>> whether or not the RDMA is used for migration.
>>
>> Signed-off-by: Zhimin Feng <fengzhimin1@huawei.com>
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> 
Hi, Dave. Sorry for late reply due to Spring Festival.

> I'd rather see a separate flag added to each of the MigrationState and
> MigrationIncomingState separately for outoging and incoming migration.
>
We use enabled_rdma_migration in migrate_use_rdma() to judge whether or not the 
RDMA is used for for both Src and Dst.
As far as i see, function like migrate_use_multifd() is used also for both 
sides.
If we use separate flag added to each of the MigrationState and 
MigrationIncomingState, we need to add two function to do that for each side.
I am not sure if that is really what you want.

> It's also probably better to call it 'is_rdma_migration' rather than
> enabled.
> 

Yes, I agree with you, it is better to use is_rdma_migration, will use it in 
next version.

> Dave
> 
>> ---
>>  migration/migration.c | 13 +++++++++++++
>>  migration/migration.h |  6 ++++++
>>  2 files changed, 19 insertions(+)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 447dfb9..129c81a 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -418,11 +418,13 @@ void migrate_add_address(SocketAddress *address)
>>  static void qemu_start_incoming_migration(const char *uri, Error **errp)
>>  {
>>      const char *p = NULL;
>> +    MigrationState *s = migrate_get_current();
>>  
>>      if (!yank_register_instance(MIGRATION_YANK_INSTANCE, errp)) {
>>          return;
>>      }
>>  
>> +    s->enabled_rdma_migration = false;
>>      qapi_event_send_migration(MIGRATION_STATUS_SETUP);
>>      if (strstart(uri, "tcp:", &p) ||
>>          strstart(uri, "unix:", NULL) ||
>> @@ -430,6 +432,7 @@ static void qemu_start_incoming_migration(const char 
>> *uri, Error **errp)
>>          socket_start_incoming_migration(p ? p : uri, errp);
>>  #ifdef CONFIG_RDMA
>>      } else if (strstart(uri, "rdma:", &p)) {
>> +        s->enabled_rdma_migration = true;
>>          rdma_start_incoming_migration(p, errp);
>>  #endif
>>      } else if (strstart(uri, "exec:", &p)) {
>> @@ -1921,6 +1924,7 @@ void migrate_init(MigrationState *s)
>>      s->start_postcopy = false;
>>      s->postcopy_after_devices = false;
>>      s->migration_thread_running = false;
>> +    s->enabled_rdma_migration = false;
>>      error_free(s->error);
>>      s->error = NULL;
>>      s->hostname = NULL;
>> @@ -2162,6 +2166,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool 
>> blk,
>>          socket_start_outgoing_migration(s, p ? p : uri, &local_err);
>>  #ifdef CONFIG_RDMA
>>      } else if (strstart(uri, "rdma:", &p)) {
>> +        s->enabled_rdma_migration = true;
>>          rdma_start_outgoing_migration(s, p, &local_err);
>>  #endif
>>      } else if (strstart(uri, "exec:", &p)) {
>> @@ -2391,6 +2396,14 @@ bool migrate_rdma_pin_all(void)
>>      return s->enabled_capabilities[MIGRATION_CAPABILITY_RDMA_PIN_ALL];
>>  }
>>  
>> +bool migrate_use_rdma(void)
>> +{
>> +    MigrationState *s;
>> +    s = migrate_get_current();
>> +
>> +    return s->enabled_rdma_migration;
>> +}
>> +
>>  bool migrate_use_multifd(void)
>>  {
>>      MigrationState *s;
>> diff --git a/migration/migration.h b/migration/migration.h
>> index 22b36f3..da5681b 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -280,6 +280,11 @@ struct MigrationState {
>>       * This save hostname when out-going migration starts
>>       */
>>      char *hostname;
>> +
>> +    /*
>> +     * Enable RDMA migration
>> +     */
>> +    bool enabled_rdma_migration;
>>  };
>>  
>>  void migrate_set_state(int *state, int old_state, int new_state);
>> @@ -317,6 +322,7 @@ bool migrate_validate_uuid(void);
>>  
>>  bool migrate_auto_converge(void);
>>  bool migrate_rdma_pin_all(void);
>> +bool migrate_use_rdma(void);
>>  bool migrate_use_multifd(void);
>>  bool migrate_pause_before_switchover(void);
>>  int migrate_multifd_channels(void);
>> -- 
>> 1.8.3.1
>>

-- 
Regards.
Chuan