Re: [Qemu-devel] [PATCH 10/19] migration: Remove use of old MigrationParams

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Juan Quintela (quintela@redhat.com) wrote:
> We have change in the previous patch to use migration capabilities for
> it.  Notice that we continue using the old command line flags from
> migrate command from the time being.  Remove the set_params method as
> now it is empty.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  include/migration/migration.h |  2 --
>  migration/block.c             | 17 ++---------------
>  migration/colo.c              |  3 ---
>  migration/migration.c         |  8 +++++---
>  migration/savevm.c            |  2 --
>  5 files changed, 7 insertions(+), 25 deletions(-)
> 
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 5c78ae1..09d3188 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -41,8 +41,6 @@
>  extern int only_migratable;
>  
>  struct MigrationParams {
> -    bool blk;
> -    bool shared;
>  };

I'm not sure that's portable C - some suggests that empty struct's
are a GNU addition.
I suggest either leaving them here, or merging this with the next
patch.

Dave

>  /* Messages sent on the return path from destination to source */
> diff --git a/migration/block.c b/migration/block.c
> index 7734ff7..9490343 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -94,9 +94,6 @@ typedef struct BlkMigBlock {
>  } BlkMigBlock;
>  
>  typedef struct BlkMigState {
> -    /* Written during setup phase.  Can be read without a lock.  */
> -    int blk_enable;
> -    int shared_base;
>      QSIMPLEQ_HEAD(bmds_list, BlkMigDevState) bmds_list;
>      int64_t total_sector_sum;
>      bool zero_blocks;
> @@ -425,7 +422,7 @@ static int init_blk_migration(QEMUFile *f)
>          bmds->bulk_completed = 0;
>          bmds->total_sectors = sectors;
>          bmds->completed_sectors = 0;
> -        bmds->shared_base = block_mig_state.shared_base;
> +        bmds->shared_base = migrate_use_block_shared();
>  
>          assert(i < num_bs);
>          bmds_bs[i].bmds = bmds;
> @@ -963,22 +960,12 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
>      return 0;
>  }
>  
> -static void block_set_params(const MigrationParams *params, void *opaque)
> -{
> -    block_mig_state.blk_enable = params->blk;
> -    block_mig_state.shared_base = params->shared;
> -
> -    /* shared base means that blk_enable = 1 */
> -    block_mig_state.blk_enable |= params->shared;
> -}
> -
>  static bool block_is_active(void *opaque)
>  {
> -    return block_mig_state.blk_enable == 1;
> +    return migrate_use_block_enabled();
>  }
>  
>  static SaveVMHandlers savevm_block_handlers = {
> -    .set_params = block_set_params,
>      .save_live_setup = block_save_setup,
>      .save_live_iterate = block_save_iterate,
>      .save_live_complete_precopy = block_save_complete,
> diff --git a/migration/colo.c b/migration/colo.c
> index eec1959..4c2365e 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -333,9 +333,6 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
>          goto out;
>      }
>  
> -    /* Disable block migration */
> -    s->params.blk = 0;
> -    s->params.shared = 0;
>      qemu_savevm_state_header(fb);
>      qemu_savevm_state_begin(fb, &s->params);
>      qemu_mutex_lock_iothread();
> diff --git a/migration/migration.c b/migration/migration.c
> index 2f10657..39b4a41 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -743,6 +743,10 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
>          s->enabled_capabilities[cap->value->capability] = cap->value->state;
>      }
>  
> +    if (s->enabled_capabilities[MIGRATION_CAPABILITY_BLOCK_SHARED]) {
> +        s->enabled_capabilities[MIGRATION_CAPABILITY_BLOCK_ENABLED] = true;
> +    }
> +
>      if (migrate_postcopy_ram()) {
>          if (migrate_use_compression()) {
>              /* The decompression threads asynchronously write into RAM
> @@ -1170,9 +1174,6 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>      MigrationParams params;
>      const char *p;
>  
> -    params.blk = has_blk && blk;
> -    params.shared = has_inc && inc;
> -
>      if (migration_is_setup_or_active(s->state) ||
>          s->state == MIGRATION_STATUS_CANCELLING ||
>          s->state == MIGRATION_STATUS_COLO) {
> @@ -1195,6 +1196,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>      }
>  
>      if (has_inc && inc) {
> +        migrate_set_block_enabled(s);
>          migrate_set_block_shared(s);
>      }
>  
> diff --git a/migration/savevm.c b/migration/savevm.c
> index c47b209..c4435e1 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1237,8 +1237,6 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
>  {
>      int ret;
>      MigrationParams params = {
> -        .blk = 0,
> -        .shared = 0
>      };
>      MigrationState *ms = migrate_init(&params);
>      MigrationStatus status;
> -- 
> 2.9.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK