Message ID | 20200217150246.29180-11-vsementsov@virtuozzo.com |
---|---|
State | New |
Headers | show |
Series | Fix error handling during bitmap postcopy | expand |
On 17/02/2020 18:02, Vladimir Sementsov-Ogievskiy wrote: > If target is turned of prior to postcopy finished, target crashes > because busy bitmaps are found at shutdown. > Canceling incoming migration helps, as it removes all unfinished (and > therefore busy) bitmaps. > > Similarly on source we crash in bdrv_close_all which asserts that all > bdrv states are removed, because bdrv states involved into dirty bitmap > migration are referenced by it. So, we need to cancel outgoing > migration as well. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > migration/migration.h | 2 ++ > migration/block-dirty-bitmap.c | 16 ++++++++++++++++ > migration/migration.c | 13 +++++++++++++ > 3 files changed, 31 insertions(+) > > diff --git a/migration/migration.h b/migration/migration.h > index 2948f2387b..2de6b8bbe2 100644 > --- a/migration/migration.h > +++ b/migration/migration.h > @@ -332,6 +332,8 @@ void migrate_send_rp_recv_bitmap(MigrationIncomingState *mis, > void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value); > > void dirty_bitmap_mig_before_vm_start(void); > +void dirty_bitmap_mig_cancel_outgoing(void); > +void dirty_bitmap_mig_cancel_incoming(void); > void migrate_add_address(SocketAddress *address); > > int foreach_not_ignored_block(RAMBlockIterFunc func, void *opaque); > diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c > index aea5326804..3ca425d95e 100644 > --- a/migration/block-dirty-bitmap.c > +++ b/migration/block-dirty-bitmap.c > @@ -585,6 +585,22 @@ static void cancel_incoming_locked(DBMLoadState *s) > s->bitmaps = NULL; > } > > +void dirty_bitmap_mig_cancel_outgoing(void) > +{ > + dirty_bitmap_do_save_cleanup(&dbm_state.save); The comment above the dirty_bitmap_do_save_cleanup() says: "Called with iothread lock taken" > +} > + > +void dirty_bitmap_mig_cancel_incoming(void) > +{ > + DBMLoadState *s = &dbm_state.load; > + > + qemu_mutex_lock(&s->lock); > + > + cancel_incoming_locked(s); > + > + qemu_mutex_unlock(&s->lock); > +} > + > static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s) > { > GSList *item; > diff --git a/migration/migration.c b/migration/migration.c > index 515047932c..7c605ba218 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -181,6 +181,19 @@ void migration_shutdown(void) > */ > migrate_fd_cancel(current_migration); > object_unref(OBJECT(current_migration)); > + > + /* > + * Cancel outgoing migration of dirty bitmaps. It should > + * at least unref used block nodes. > + */ > + dirty_bitmap_mig_cancel_outgoing(); > + > + /* > + * Cancel incoming migration of dirty bitmaps. Dirty bitmaps > + * are non-critical data, and their loss never considered as > + * something serious. > + */ > + dirty_bitmap_mig_cancel_incoming(); > } > > /* For outgoing */ > Reviewed-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
On 2/17/20 9:02 AM, Vladimir Sementsov-Ogievskiy wrote: > If target is turned of prior to postcopy finished, target crashes s/of/off/ > because busy bitmaps are found at shutdown. > Canceling incoming migration helps, as it removes all unfinished (and > therefore busy) bitmaps. > > Similarly on source we crash in bdrv_close_all which asserts that all > bdrv states are removed, because bdrv states involved into dirty bitmap > migration are referenced by it. So, we need to cancel outgoing > migration as well. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- >
diff --git a/migration/migration.h b/migration/migration.h index 2948f2387b..2de6b8bbe2 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -332,6 +332,8 @@ void migrate_send_rp_recv_bitmap(MigrationIncomingState *mis, void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value); void dirty_bitmap_mig_before_vm_start(void); +void dirty_bitmap_mig_cancel_outgoing(void); +void dirty_bitmap_mig_cancel_incoming(void); void migrate_add_address(SocketAddress *address); int foreach_not_ignored_block(RAMBlockIterFunc func, void *opaque); diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c index aea5326804..3ca425d95e 100644 --- a/migration/block-dirty-bitmap.c +++ b/migration/block-dirty-bitmap.c @@ -585,6 +585,22 @@ static void cancel_incoming_locked(DBMLoadState *s) s->bitmaps = NULL; } +void dirty_bitmap_mig_cancel_outgoing(void) +{ + dirty_bitmap_do_save_cleanup(&dbm_state.save); +} + +void dirty_bitmap_mig_cancel_incoming(void) +{ + DBMLoadState *s = &dbm_state.load; + + qemu_mutex_lock(&s->lock); + + cancel_incoming_locked(s); + + qemu_mutex_unlock(&s->lock); +} + static void dirty_bitmap_load_complete(QEMUFile *f, DBMLoadState *s) { GSList *item; diff --git a/migration/migration.c b/migration/migration.c index 515047932c..7c605ba218 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -181,6 +181,19 @@ void migration_shutdown(void) */ migrate_fd_cancel(current_migration); object_unref(OBJECT(current_migration)); + + /* + * Cancel outgoing migration of dirty bitmaps. It should + * at least unref used block nodes. + */ + dirty_bitmap_mig_cancel_outgoing(); + + /* + * Cancel incoming migration of dirty bitmaps. Dirty bitmaps + * are non-critical data, and their loss never considered as + * something serious. + */ + dirty_bitmap_mig_cancel_incoming(); } /* For outgoing */
If target is turned of prior to postcopy finished, target crashes because busy bitmaps are found at shutdown. Canceling incoming migration helps, as it removes all unfinished (and therefore busy) bitmaps. Similarly on source we crash in bdrv_close_all which asserts that all bdrv states are removed, because bdrv states involved into dirty bitmap migration are referenced by it. So, we need to cancel outgoing migration as well. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- migration/migration.h | 2 ++ migration/block-dirty-bitmap.c | 16 ++++++++++++++++ migration/migration.c | 13 +++++++++++++ 3 files changed, 31 insertions(+)