| Message ID | 20170517153812.21993-4-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/17/2017 10:38 AM, Juan Quintela 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. > > For savevm, one can't do a: > > savevm -b/-i foo > > but now one can do: > > migrate_set_capability block on > savevm foo > > And we can't use block migration. We could disable block capability > unconditionally, but it would not be much better. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > > --- > - Maintain shared/enabled dependency (Xu suggestion) > - Now we maintain the dependency on the setter functions > - improve error messages > > Signed-off-by: Juan Quintela <quintela@redhat.com> This second S-o-b gets stripped, but the one that counts is in place (I've made the same mistake before, as well - doing 'git commit --amend -s' on a commit where I already had a --- separator) Reviewed-by: Eric Blake <eblake@redhat.com>
Eric Blake <eblake@redhat.com> wrote: > On 05/17/2017 10:38 AM, Juan Quintela 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. >> >> For savevm, one can't do a: >> >> savevm -b/-i foo >> >> but now one can do: >> >> migrate_set_capability block on >> savevm foo >> >> And we can't use block migration. We could disable block capability >> unconditionally, but it would not be much better. >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> >> >> --- >> - Maintain shared/enabled dependency (Xu suggestion) >> - Now we maintain the dependency on the setter functions >> - improve error messages >> >> Signed-off-by: Juan Quintela <quintela@redhat.com> > > This second S-o-b gets stripped, but the one that counts is in place > (I've made the same mistake before, as well - doing 'git commit --amend > -s' on a commit where I already had a --- separator) And this time I *really* remove the -s from my git-format-patch alias. I dropped that last week on the terminal, rebooted the machine and ..... magic ... got the old alias O:-) > Reviewed-by: Eric Blake <eblake@redhat.com> Thanks
Juan Quintela <quintela@redhat.com> writes: > 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 for the time being > now it is empty. > > For savevm, one can't do a: > > savevm -b/-i foo > > but now one can do: > > migrate_set_capability block on > savevm foo > > And we can't use block migration. We could disable block capability > unconditionally, but it would not be much better. I think I get what you're trying to say, but only because I have plenty of context right now. Let me try to rephrase: migration: Use new configuration instead of old MigrationParams The previous commit introduced a MigrationCapability and a MigrationParameter for block migration. Use them instead of the old MigrationParams. Take care to reject attempts to combine block migration with snapshots, e.g. like this: migrate_set_capability block on savevm foo > Signed-off-by: Juan Quintela <quintela@redhat.com> Preferably with a commit message I can still understand three weeks from now: Reviewed-by: Markus Armbruster <armbru@redhat.com>
I got confused and replied to an old version. Please ignore. Markus Armbruster <armbru@redhat.com> writes: > Juan Quintela <quintela@redhat.com> writes: > >> 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 > > for the time being > >> now it is empty. >> >> For savevm, one can't do a: >> >> savevm -b/-i foo >> >> but now one can do: >> >> migrate_set_capability block on >> savevm foo >> >> And we can't use block migration. We could disable block capability >> unconditionally, but it would not be much better. > > I think I get what you're trying to say, but only because I have plenty > of context right now. Let me try to rephrase: > > migration: Use new configuration instead of old MigrationParams > > The previous commit introduced a MigrationCapability and a > MigrationParameter for block migration. Use them instead of the old > MigrationParams. > > Take care to reject attempts to combine block migration with > snapshots, e.g. like this: > > migrate_set_capability block on > savevm foo > >> Signed-off-by: Juan Quintela <quintela@redhat.com> > > Preferably with a commit message I can still understand three weeks from > now: > Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff --git a/include/migration/migration.h b/include/migration/migration.h index 024a048..4dedc66 100644 --- a/include/migration/migration.h +++ b/include/migration/migration.h @@ -39,8 +39,7 @@ #define QEMU_VM_SECTION_FOOTER 0x7e struct MigrationParams { - bool blk; - bool shared; + bool unused; /* C doesn't allow empty structs */ }; /* Messages sent on the return path from destination to source */ diff --git a/migration/block.c b/migration/block.c index 060087f..5d22926 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_incremental(); assert(i < num_bs); bmds_bs[i].bmds = bmds; @@ -994,22 +991,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(); } 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 963c802..8c86892 100644 --- a/migration/colo.c +++ b/migration/colo.c @@ -14,6 +14,7 @@ #include "qemu/timer.h" #include "sysemu/sysemu.h" #include "migration/colo.h" +#include "migration/block.h" #include "io/channel-buffer.h" #include "trace.h" #include "qemu/error-report.h" @@ -345,8 +346,7 @@ static int colo_do_checkpoint_transaction(MigrationState *s, } /* Disable block migration */ - s->params.blk = 0; - s->params.shared = 0; + migrate_set_block_enabled(false, &local_err); 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 f2bb448..f4cbfe6 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1224,9 +1224,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) { diff --git a/migration/savevm.c b/migration/savevm.c index f5e8194..2f1f4eb 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1233,8 +1233,6 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp) { int ret; MigrationParams params = { - .blk = 0, - .shared = 0 }; MigrationState *ms = migrate_init(¶ms); MigrationStatus status; @@ -1245,6 +1243,12 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp) goto done; } + if (migrate_use_block()) { + error_setg(errp, "Block migration and snapshots are incompatible"); + ret = -EINVAL; + goto done; + } + qemu_mutex_unlock_iothread(); qemu_savevm_state_header(f); qemu_savevm_state_begin(f, ¶ms);