* John Snow (js...@redhat.com) wrote: > > > On 03/12/2018 11:30 AM, Dr. David Alan Gilbert wrote: > > * Vladimir Sementsov-Ogievskiy (vsement...@virtuozzo.com) wrote: > >> There would be savevm states (dirty-bitmap) which can migrate only in > >> postcopy stage. The corresponding pending is introduced here. > >> > >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > >> --- > >> include/migration/register.h | 17 +++++++++++++++-- > >> migration/savevm.h | 5 +++-- > >> hw/s390x/s390-stattrib.c | 7 ++++--- > >> migration/block.c | 7 ++++--- > >> migration/migration.c | 16 +++++++++------- > >> migration/ram.c | 9 +++++---- > >> migration/savevm.c | 13 ++++++++----- > >> migration/trace-events | 2 +- > >> 8 files changed, 49 insertions(+), 27 deletions(-) > >> > >> diff --git a/include/migration/register.h b/include/migration/register.h > >> index f4f7bdc177..9436a87678 100644 > >> --- a/include/migration/register.h > >> +++ b/include/migration/register.h > >> @@ -37,8 +37,21 @@ typedef struct SaveVMHandlers { > >> int (*save_setup)(QEMUFile *f, void *opaque); > >> void (*save_live_pending)(QEMUFile *f, void *opaque, > >> uint64_t threshold_size, > >> - uint64_t *non_postcopiable_pending, > >> - uint64_t *postcopiable_pending); > >> + uint64_t *res_precopy_only, > >> + uint64_t *res_compatible, > >> + uint64_t *res_postcopy_only); > >> + /* Note for save_live_pending: > >> + * - res_precopy_only is for data which must be migrated in precopy > >> phase > >> + * or in stopped state, in other words - before target vm start > >> + * - res_compatible is for data which may be migrated in any phase > >> + * - res_postcopy_only is for data which must be migrated in postcopy > >> phase > >> + * or in stopped state, in other words - after source vm stop > >> + * > >> + * Sum of res_postcopy_only, res_compatible and res_postcopy_only is > >> the > >> + * whole amount of pending data. > >> + */ > >> + > >> + > >> LoadStateHandler *load_state; > >> int (*load_setup)(QEMUFile *f, void *opaque); > >> int (*load_cleanup)(void *opaque); > >> diff --git a/migration/savevm.h b/migration/savevm.h > >> index 295c4a1f2c..cf4f0d37ca 100644 > >> --- a/migration/savevm.h > >> +++ b/migration/savevm.h > >> @@ -38,8 +38,9 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f); > >> int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only, > >> bool inactivate_disks); > >> void qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size, > >> - uint64_t *res_non_postcopiable, > >> - uint64_t *res_postcopiable); > >> + uint64_t *res_precopy_only, > >> + uint64_t *res_compatible, > >> + uint64_t *res_postcopy_only); > >> void qemu_savevm_send_ping(QEMUFile *f, uint32_t value); > >> void qemu_savevm_send_open_return_path(QEMUFile *f); > >> int qemu_savevm_send_packaged(QEMUFile *f, const uint8_t *buf, size_t > >> len); > >> diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c > >> index 2902f54f11..dd3fbfd1eb 100644 > >> --- a/hw/s390x/s390-stattrib.c > >> +++ b/hw/s390x/s390-stattrib.c > >> @@ -183,15 +183,16 @@ static int cmma_save_setup(QEMUFile *f, void *opaque) > >> } > >> > >> static void cmma_save_pending(QEMUFile *f, void *opaque, uint64_t > >> max_size, > >> - uint64_t *non_postcopiable_pending, > >> - uint64_t *postcopiable_pending) > >> + uint64_t *res_precopy_only, > >> + uint64_t *res_compatible, > >> + uint64_t *res_postcopy_only) > >> { > >> S390StAttribState *sas = S390_STATTRIB(opaque); > >> S390StAttribClass *sac = S390_STATTRIB_GET_CLASS(sas); > >> long long res = sac->get_dirtycount(sas); > >> > >> if (res >= 0) { > >> - *non_postcopiable_pending += res; > >> + *res_precopy_only += res; > >> } > >> } > >> > >> diff --git a/migration/block.c b/migration/block.c > >> index 1f03946797..5652ca3337 100644 > >> --- a/migration/block.c > >> +++ b/migration/block.c > >> @@ -866,8 +866,9 @@ static int block_save_complete(QEMUFile *f, void > >> *opaque) > >> } > >> > >> static void block_save_pending(QEMUFile *f, void *opaque, uint64_t > >> max_size, > >> - uint64_t *non_postcopiable_pending, > >> - uint64_t *postcopiable_pending) > >> + uint64_t *res_precopy_only, > >> + uint64_t *res_compatible, > >> + uint64_t *res_postcopy_only) > >> { > >> /* Estimate pending number of bytes to send */ > >> uint64_t pending; > >> @@ -888,7 +889,7 @@ static void block_save_pending(QEMUFile *f, void > >> *opaque, uint64_t max_size, > >> > >> DPRINTF("Enter save live pending %" PRIu64 "\n", pending); > >> /* We don't do postcopy */ > >> - *non_postcopiable_pending += pending; > >> + *res_precopy_only += pending; > >> } > >> > >> static int block_load(QEMUFile *f, void *opaque, int version_id) > >> diff --git a/migration/migration.c b/migration/migration.c > >> index c99a4e62d7..3beedd676e 100644 > >> --- a/migration/migration.c > >> +++ b/migration/migration.c > >> @@ -2215,21 +2215,23 @@ typedef enum { > >> */ > >> static MigIterateState migration_iteration_run(MigrationState *s) > >> { > >> - uint64_t pending_size, pend_post, pend_nonpost; > >> + uint64_t pending_size, pend_pre, pend_compat, pend_post; > >> bool in_postcopy = s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE; > >> > >> - qemu_savevm_state_pending(s->to_dst_file, s->threshold_size, > >> - &pend_nonpost, &pend_post); > >> - pending_size = pend_nonpost + pend_post; > >> + qemu_savevm_state_pending(s->to_dst_file, s->threshold_size, > >> &pend_pre, > >> + &pend_compat, &pend_post); > >> + pending_size = pend_pre + pend_compat + pend_post; > >> > >> trace_migrate_pending(pending_size, s->threshold_size, > >> - pend_post, pend_nonpost); > >> + pend_pre, pend_compat, pend_post); > >> > >> if (pending_size && pending_size >= s->threshold_size) { > >> /* Still a significant amount to transfer */ > >> if (migrate_postcopy() && !in_postcopy && > >> - pend_nonpost <= s->threshold_size && > >> - atomic_read(&s->start_postcopy)) { > >> + pend_pre <= s->threshold_size && > >> + (atomic_read(&s->start_postcopy) || > >> + (pend_pre + pend_compat <= s->threshold_size))) > > > > This change does something different from the description; > > it causes a postcopy_start even if the user never ran the postcopy-start > > command; so sorry, we can't do that; because postcopy for RAM is > > something that users can enable but only switch into when they've given > > up on it completing normally. > > > > However, I guess that leaves you with a problem; which is what happens > > to the system when you've run out of pend_pre+pend_compat but can't > > complete because pend_post is non-0; so I don't know the answer to that. > > > > Dave > > > > I'm willing to pull the bitmap related changes into my tree for 2.12, > but it would be _really_ nice to have this patchset in 2.12 -- is there > anything we can do to get this into the migration queue? > > I'm afraid I don't know what the right answer here is either, so I can't > fix this up myself and I imagine Vladimir will need some feedback from > you to change this design.
Well I'm good with everything except the change in semantics for existing migration; we can't do that because it can break existing users. > Are we -hosed- ? Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK