| Message ID | 20230526115908.196171-1-f.ebner@proxmox.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration: stop tracking ram writes when cancelling background migration | expand |
On Fri, May 26, 2023 at 01:59:08PM +0200, Fiona Ebner wrote: > Currently, it is only done when the iteration finishes successfully. > Not cleaning up the userfaultfd write protection can lead to > symptoms/issues such as the process hanging in memmove or GDB not > being able to attach. > > Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Peter Xu <peterx@redhat.com> > For the success case, the stuff in between the old call and new call > site should not depend on tracking to already be stopped, right? Yes I think so. There's only device states to be flushed and no guest memory should be touched. Even if we'll touch them, since we've finished migrating all of them so they're already unprotected anyway (actually, even during ram save it's read-only here from bg thread), so not any problem I can see. Thanks,
Fiona Ebner <f.ebner@proxmox.com> wrote: > Currently, it is only done when the iteration finishes successfully. > Not cleaning up the userfaultfd write protection can lead to > symptoms/issues such as the process hanging in memmove or GDB not > being able to attach. > > Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> > --- > > For the success case, the stuff in between the old call and new call > site should not depend on tracking to already be stopped, right? > AFAICS, it's just some QEMU file operations and manipulation of > migration state and counters, but I have only limited knowledge, so I > thought I'd better ask :) Reviewed-by: Juan Quintela <quintela@redhat.com> queued for next PULL
diff --git a/migration/migration.c b/migration/migration.c index 5de7f734b9..60583699f7 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2400,13 +2400,6 @@ static void bg_migration_completion(MigrationState *s) { int current_active_state = s->state; - /* - * Stop tracking RAM writes - un-protect memory, un-register UFFD - * memory ranges, flush kernel wait queues and wake up threads - * waiting for write fault to be resolved. - */ - ram_write_tracking_stop(); - if (s->state == MIGRATION_STATUS_ACTIVE) { /* * By this moment we have RAM content saved into the migration stream. @@ -2788,6 +2781,13 @@ static void migration_iteration_finish(MigrationState *s) static void bg_migration_iteration_finish(MigrationState *s) { + /* + * Stop tracking RAM writes - un-protect memory, un-register UFFD + * memory ranges, flush kernel wait queues and wake up threads + * waiting for write fault to be resolved. + */ + ram_write_tracking_stop(); + qemu_mutex_lock_iothread(); switch (s->state) { case MIGRATION_STATUS_COMPLETED:
Currently, it is only done when the iteration finishes successfully. Not cleaning up the userfaultfd write protection can lead to symptoms/issues such as the process hanging in memmove or GDB not being able to attach. Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> --- For the success case, the stuff in between the old call and new call site should not depend on tracking to already be stopped, right? AFAICS, it's just some QEMU file operations and manipulation of migration state and counters, but I have only limited knowledge, so I thought I'd better ask :) migration/migration.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)