diff mbox

[10/19] migration: Remove use of old MigrationParams

Message ID 20170417200041.2451-11-quintela@redhat.com
State New
Headers show

Commit Message

Juan Quintela April 17, 2017, 8 p.m. UTC
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(-)

Comments

Dr. David Alan Gilbert April 18, 2017, 7:07 p.m. UTC | #1
* 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
Juan Quintela April 19, 2017, 7:17 a.m. UTC | #2
"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * 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.

Added an unused bool.  I think it is clearer in two patches.

Thanks, Juan.
diff mbox

Patch

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;
 };
 
 /* 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;