| Message ID | 20191011112015.11785-8-jfreimann@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | add failover feature for assigned network devices | expand |
On Fri, Oct 11, 2019 at 01:20:12PM +0200, Jens Freimann wrote: > This patch adds a new migration state called wait-unplug. It is entered > after the SETUP state and will transition into ACTIVE once all devices > were succesfully unplugged from the guest. > > So if a guest doesn't respond or takes long to honor the unplug request > the user will see the migration state 'wait-unplug'. > > In the migration thread we query failover devices if they're are still > pending the guest unplug. When all are unplugged the migration > continues. We give it a defined number of iterations including small > waiting periods before we proceed. > > Signed-off-by: Jens Freimann <jfreimann@redhat.com> > --- > include/migration/vmstate.h | 2 ++ > migration/migration.c | 34 ++++++++++++++++++++++++++++++++++ > migration/migration.h | 3 +++ > migration/savevm.c | 36 ++++++++++++++++++++++++++++++++++++ > migration/savevm.h | 1 + > qapi/migration.json | 5 ++++- > 6 files changed, 80 insertions(+), 1 deletion(-) > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index 1fbfd099dd..39ef125225 100644 > --- a/include/migration/vmstate.h > +++ b/include/migration/vmstate.h > @@ -186,6 +186,8 @@ struct VMStateDescription { > int (*pre_save)(void *opaque); > int (*post_save)(void *opaque); > bool (*needed)(void *opaque); > + bool (*dev_unplug_pending)(void *opaque); > + > const VMStateField *fields; > const VMStateDescription **subsections; > }; > diff --git a/migration/migration.c b/migration/migration.c > index 5f7e4d15e9..a17d9fb990 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -52,9 +52,14 @@ > #include "hw/qdev-properties.h" > #include "monitor/monitor.h" > #include "net/announce.h" > +#include "qemu/queue.h" > > #define MAX_THROTTLE (32 << 20) /* Migration transfer speed throttling */ > > +/* Time in milliseconds to wait for guest OS to unplug PCI device */ > +#define FAILOVER_GUEST_UNPLUG_WAIT 10000 This value should be controllable from QMP. And I think the default should be infinite wait. > +#define FAILOVER_UNPLUG_RETRIES 5 This is a bit of a hack. We really should just have unplug signal wakeup, or time expire. E.g. a new kind of notifier? > + > /* Amount of time to allocate to each "chunk" of bandwidth-throttled > * data. */ > #define BUFFER_DELAY 100 > @@ -954,6 +959,9 @@ static void fill_source_migration_info(MigrationInfo *info) > case MIGRATION_STATUS_CANCELLED: > info->has_status = true; > break; > + case MIGRATION_STATUS_WAIT_UNPLUG: > + info->has_status = true; > + break; > } > info->status = s->state; > } > @@ -1695,6 +1703,7 @@ bool migration_is_idle(void) > case MIGRATION_STATUS_COLO: > case MIGRATION_STATUS_PRE_SWITCHOVER: > case MIGRATION_STATUS_DEVICE: > + case MIGRATION_STATUS_WAIT_UNPLUG: > return false; > case MIGRATION_STATUS__MAX: > g_assert_not_reached(); > @@ -3224,6 +3233,8 @@ static void *migration_thread(void *opaque) > int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST); > MigThrError thr_error; > bool urgent = false; > + bool all_unplugged = true; > + int i = 0; > > rcu_register_thread(); > > @@ -3260,6 +3271,27 @@ static void *migration_thread(void *opaque) > > qemu_savevm_state_setup(s->to_dst_file); > > + migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > + MIGRATION_STATUS_WAIT_UNPLUG); > + > + while (i < FAILOVER_UNPLUG_RETRIES && > + s->state == MIGRATION_STATUS_WAIT_UNPLUG) { > + i++; > + qemu_sem_timedwait(&s->wait_unplug_sem, FAILOVER_GUEST_UNPLUG_WAIT); Should be FAILOVER_GUEST_UNPLUG_WAIT / FAILOVER_UNPLUG_RETRIES such that time set is the total. > + all_unplugged = qemu_savevm_state_guest_unplug_pending(); > + if (all_unplugged) { > + break; > + } > + } > + > + if (all_unplugged) { > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, > + MIGRATION_STATUS_ACTIVE); > + } else { > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, > + MIGRATION_STATUS_CANCELLING); > + } > + > s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; > migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > MIGRATION_STATUS_ACTIVE); > @@ -3508,6 +3540,7 @@ static void migration_instance_finalize(Object *obj) > qemu_mutex_destroy(&ms->qemu_file_lock); > g_free(params->tls_hostname); > g_free(params->tls_creds); > + qemu_sem_destroy(&ms->wait_unplug_sem); > qemu_sem_destroy(&ms->rate_limit_sem); > qemu_sem_destroy(&ms->pause_sem); > qemu_sem_destroy(&ms->postcopy_pause_sem); > @@ -3553,6 +3586,7 @@ static void migration_instance_init(Object *obj) > qemu_sem_init(&ms->postcopy_pause_rp_sem, 0); > qemu_sem_init(&ms->rp_state.rp_sem, 0); > qemu_sem_init(&ms->rate_limit_sem, 0); > + qemu_sem_init(&ms->wait_unplug_sem, 0); > qemu_mutex_init(&ms->qemu_file_lock); > } > > diff --git a/migration/migration.h b/migration/migration.h > index 4f2fe193dc..79b3dda146 100644 > --- a/migration/migration.h > +++ b/migration/migration.h > @@ -206,6 +206,9 @@ struct MigrationState > /* Flag set once the migration thread called bdrv_inactivate_all */ > bool block_inactive; > > + /* Migration is waiting for guest to unplug device */ > + QemuSemaphore wait_unplug_sem; > + > /* Migration is paused due to pause-before-switchover */ > QemuSemaphore pause_sem; > > diff --git a/migration/savevm.c b/migration/savevm.c > index bb9462a54d..26e5bde687 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -942,6 +942,20 @@ static void qemu_savevm_command_send(QEMUFile *f, > qemu_fflush(f); > } > > +static int qemu_savevm_nr_failover_devices(void) > +{ > + SaveStateEntry *se; > + int n = 0; > + > + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { > + if (se->vmsd && se->vmsd->dev_unplug_pending) { > + n++; > + } > + } > + > + return n; > +} > + > void qemu_savevm_send_colo_enable(QEMUFile *f) > { > trace_savevm_send_colo_enable(); > @@ -1113,6 +1127,28 @@ void qemu_savevm_state_header(QEMUFile *f) > } > } > > +bool qemu_savevm_state_guest_unplug_pending(void) > +{ > + int nr_failover_devs; > + SaveStateEntry *se; > + bool ret = false; > + int n = 0; > + > + nr_failover_devs = qemu_savevm_nr_failover_devices(); > + > + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { > + if (!se->vmsd || !se->vmsd->dev_unplug_pending) { > + continue; > + } > + ret = se->vmsd->dev_unplug_pending(se->opaque); > + if (!ret) { > + n++; > + } > + } > + > + return n == nr_failover_devs; > +} > + > void qemu_savevm_state_setup(QEMUFile *f) > { > SaveStateEntry *se; > diff --git a/migration/savevm.h b/migration/savevm.h > index 51a4b9caa8..ba64a7e271 100644 > --- a/migration/savevm.h > +++ b/migration/savevm.h > @@ -31,6 +31,7 @@ > > bool qemu_savevm_state_blocked(Error **errp); > void qemu_savevm_state_setup(QEMUFile *f); > +bool qemu_savevm_state_guest_unplug_pending(void); > int qemu_savevm_state_resume_prepare(MigrationState *s); > void qemu_savevm_state_header(QEMUFile *f); > int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy); > diff --git a/qapi/migration.json b/qapi/migration.json > index 52e69e2868..5a06cd489f 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -133,6 +133,9 @@ > # @device: During device serialisation when pause-before-switchover is enabled > # (since 2.11) > # > +# @wait-unplug: wait for device unplug request by guest OS to be completed. > +# (since 4.2) > +# > # Since: 2.3 > # > ## > @@ -140,7 +143,7 @@ > 'data': [ 'none', 'setup', 'cancelling', 'cancelled', > 'active', 'postcopy-active', 'postcopy-paused', > 'postcopy-recover', 'completed', 'failed', 'colo', > - 'pre-switchover', 'device' ] } > + 'pre-switchover', 'device', 'wait-unplug' ] } > > ## > # @MigrationInfo: > -- > 2.21.0
On Fri, Oct 11, 2019 at 08:57:48AM -0400, Michael S. Tsirkin wrote: >On Fri, Oct 11, 2019 at 01:20:12PM +0200, Jens Freimann wrote: >> This patch adds a new migration state called wait-unplug. It is entered >> after the SETUP state and will transition into ACTIVE once all devices >> were succesfully unplugged from the guest. >> >> So if a guest doesn't respond or takes long to honor the unplug request >> the user will see the migration state 'wait-unplug'. >> >> In the migration thread we query failover devices if they're are still >> pending the guest unplug. When all are unplugged the migration >> continues. We give it a defined number of iterations including small >> waiting periods before we proceed. >> >> Signed-off-by: Jens Freimann <jfreimann@redhat.com> >> --- >> include/migration/vmstate.h | 2 ++ >> migration/migration.c | 34 ++++++++++++++++++++++++++++++++++ >> migration/migration.h | 3 +++ >> migration/savevm.c | 36 ++++++++++++++++++++++++++++++++++++ >> migration/savevm.h | 1 + >> qapi/migration.json | 5 ++++- >> 6 files changed, 80 insertions(+), 1 deletion(-) >> >> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h >> index 1fbfd099dd..39ef125225 100644 >> --- a/include/migration/vmstate.h >> +++ b/include/migration/vmstate.h >> @@ -186,6 +186,8 @@ struct VMStateDescription { >> int (*pre_save)(void *opaque); >> int (*post_save)(void *opaque); >> bool (*needed)(void *opaque); >> + bool (*dev_unplug_pending)(void *opaque); >> + >> const VMStateField *fields; >> const VMStateDescription **subsections; >> }; >> diff --git a/migration/migration.c b/migration/migration.c >> index 5f7e4d15e9..a17d9fb990 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -52,9 +52,14 @@ >> #include "hw/qdev-properties.h" >> #include "monitor/monitor.h" >> #include "net/announce.h" >> +#include "qemu/queue.h" >> >> #define MAX_THROTTLE (32 << 20) /* Migration transfer speed throttling */ >> >> +/* Time in milliseconds to wait for guest OS to unplug PCI device */ >> +#define FAILOVER_GUEST_UNPLUG_WAIT 10000 > >This value should be controllable from QMP. >And I think the default should be infinite wait. ok, I can do that. > >> +#define FAILOVER_UNPLUG_RETRIES 5 > > >This is a bit of a hack. We really should just >have unplug signal wakeup, or time expire. >E.g. a new kind of notifier? I tried to keep it simple. But you're probably right. Seems like a new notifier similar to the migration state notifier is a good option. Maybe I could also reuse the migration state notifier I have added and check for the new migration state in it. Thanks for the review! regards, Jens > >> + >> /* Amount of time to allocate to each "chunk" of bandwidth-throttled >> * data. */ >> #define BUFFER_DELAY 100 >> @@ -954,6 +959,9 @@ static void fill_source_migration_info(MigrationInfo *info) >> case MIGRATION_STATUS_CANCELLED: >> info->has_status = true; >> break; >> + case MIGRATION_STATUS_WAIT_UNPLUG: >> + info->has_status = true; >> + break; >> } >> info->status = s->state; >> } >> @@ -1695,6 +1703,7 @@ bool migration_is_idle(void) >> case MIGRATION_STATUS_COLO: >> case MIGRATION_STATUS_PRE_SWITCHOVER: >> case MIGRATION_STATUS_DEVICE: >> + case MIGRATION_STATUS_WAIT_UNPLUG: >> return false; >> case MIGRATION_STATUS__MAX: >> g_assert_not_reached(); >> @@ -3224,6 +3233,8 @@ static void *migration_thread(void *opaque) >> int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST); >> MigThrError thr_error; >> bool urgent = false; >> + bool all_unplugged = true; >> + int i = 0; >> >> rcu_register_thread(); >> >> @@ -3260,6 +3271,27 @@ static void *migration_thread(void *opaque) >> >> qemu_savevm_state_setup(s->to_dst_file); >> >> + migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, >> + MIGRATION_STATUS_WAIT_UNPLUG); >> + >> + while (i < FAILOVER_UNPLUG_RETRIES && >> + s->state == MIGRATION_STATUS_WAIT_UNPLUG) { >> + i++; >> + qemu_sem_timedwait(&s->wait_unplug_sem, FAILOVER_GUEST_UNPLUG_WAIT); > >Should be FAILOVER_GUEST_UNPLUG_WAIT / FAILOVER_UNPLUG_RETRIES > >such that time set is the total. > >> + all_unplugged = qemu_savevm_state_guest_unplug_pending(); >> + if (all_unplugged) { >> + break; >> + } >> + } >> + >> + if (all_unplugged) { >> + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, >> + MIGRATION_STATUS_ACTIVE); >> + } else { >> + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, >> + MIGRATION_STATUS_CANCELLING); >> + } >> + >> s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; >> migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, >> MIGRATION_STATUS_ACTIVE); >> @@ -3508,6 +3540,7 @@ static void migration_instance_finalize(Object *obj) >> qemu_mutex_destroy(&ms->qemu_file_lock); >> g_free(params->tls_hostname); >> g_free(params->tls_creds); >> + qemu_sem_destroy(&ms->wait_unplug_sem); >> qemu_sem_destroy(&ms->rate_limit_sem); >> qemu_sem_destroy(&ms->pause_sem); >> qemu_sem_destroy(&ms->postcopy_pause_sem); >> @@ -3553,6 +3586,7 @@ static void migration_instance_init(Object *obj) >> qemu_sem_init(&ms->postcopy_pause_rp_sem, 0); >> qemu_sem_init(&ms->rp_state.rp_sem, 0); >> qemu_sem_init(&ms->rate_limit_sem, 0); >> + qemu_sem_init(&ms->wait_unplug_sem, 0); >> qemu_mutex_init(&ms->qemu_file_lock); >> } >> >> diff --git a/migration/migration.h b/migration/migration.h >> index 4f2fe193dc..79b3dda146 100644 >> --- a/migration/migration.h >> +++ b/migration/migration.h >> @@ -206,6 +206,9 @@ struct MigrationState >> /* Flag set once the migration thread called bdrv_inactivate_all */ >> bool block_inactive; >> >> + /* Migration is waiting for guest to unplug device */ >> + QemuSemaphore wait_unplug_sem; >> + >> /* Migration is paused due to pause-before-switchover */ >> QemuSemaphore pause_sem; >> >> diff --git a/migration/savevm.c b/migration/savevm.c >> index bb9462a54d..26e5bde687 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -942,6 +942,20 @@ static void qemu_savevm_command_send(QEMUFile *f, >> qemu_fflush(f); >> } >> >> +static int qemu_savevm_nr_failover_devices(void) >> +{ >> + SaveStateEntry *se; >> + int n = 0; >> + >> + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { >> + if (se->vmsd && se->vmsd->dev_unplug_pending) { >> + n++; >> + } >> + } >> + >> + return n; >> +} >> + >> void qemu_savevm_send_colo_enable(QEMUFile *f) >> { >> trace_savevm_send_colo_enable(); >> @@ -1113,6 +1127,28 @@ void qemu_savevm_state_header(QEMUFile *f) >> } >> } >> >> +bool qemu_savevm_state_guest_unplug_pending(void) >> +{ >> + int nr_failover_devs; >> + SaveStateEntry *se; >> + bool ret = false; >> + int n = 0; >> + >> + nr_failover_devs = qemu_savevm_nr_failover_devices(); >> + >> + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { >> + if (!se->vmsd || !se->vmsd->dev_unplug_pending) { >> + continue; >> + } >> + ret = se->vmsd->dev_unplug_pending(se->opaque); >> + if (!ret) { >> + n++; >> + } >> + } >> + >> + return n == nr_failover_devs; >> +} >> + >> void qemu_savevm_state_setup(QEMUFile *f) >> { >> SaveStateEntry *se; >> diff --git a/migration/savevm.h b/migration/savevm.h >> index 51a4b9caa8..ba64a7e271 100644 >> --- a/migration/savevm.h >> +++ b/migration/savevm.h >> @@ -31,6 +31,7 @@ >> >> bool qemu_savevm_state_blocked(Error **errp); >> void qemu_savevm_state_setup(QEMUFile *f); >> +bool qemu_savevm_state_guest_unplug_pending(void); >> int qemu_savevm_state_resume_prepare(MigrationState *s); >> void qemu_savevm_state_header(QEMUFile *f); >> int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy); >> diff --git a/qapi/migration.json b/qapi/migration.json >> index 52e69e2868..5a06cd489f 100644 >> --- a/qapi/migration.json >> +++ b/qapi/migration.json >> @@ -133,6 +133,9 @@ >> # @device: During device serialisation when pause-before-switchover is enabled >> # (since 2.11) >> # >> +# @wait-unplug: wait for device unplug request by guest OS to be completed. >> +# (since 4.2) >> +# >> # Since: 2.3 >> # >> ## >> @@ -140,7 +143,7 @@ >> 'data': [ 'none', 'setup', 'cancelling', 'cancelled', >> 'active', 'postcopy-active', 'postcopy-paused', >> 'postcopy-recover', 'completed', 'failed', 'colo', >> - 'pre-switchover', 'device' ] } >> + 'pre-switchover', 'device', 'wait-unplug' ] } >> >> ## >> # @MigrationInfo: >> -- >> 2.21.0
* Jens Freimann (jfreimann@redhat.com) wrote: > This patch adds a new migration state called wait-unplug. It is entered > after the SETUP state and will transition into ACTIVE once all devices > were succesfully unplugged from the guest. > > So if a guest doesn't respond or takes long to honor the unplug request > the user will see the migration state 'wait-unplug'. > > In the migration thread we query failover devices if they're are still > pending the guest unplug. When all are unplugged the migration > continues. We give it a defined number of iterations including small > waiting periods before we proceed. > > Signed-off-by: Jens Freimann <jfreimann@redhat.com> > --- > include/migration/vmstate.h | 2 ++ > migration/migration.c | 34 ++++++++++++++++++++++++++++++++++ > migration/migration.h | 3 +++ > migration/savevm.c | 36 ++++++++++++++++++++++++++++++++++++ > migration/savevm.h | 1 + > qapi/migration.json | 5 ++++- > 6 files changed, 80 insertions(+), 1 deletion(-) > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index 1fbfd099dd..39ef125225 100644 > --- a/include/migration/vmstate.h > +++ b/include/migration/vmstate.h > @@ -186,6 +186,8 @@ struct VMStateDescription { > int (*pre_save)(void *opaque); > int (*post_save)(void *opaque); > bool (*needed)(void *opaque); > + bool (*dev_unplug_pending)(void *opaque); > + > const VMStateField *fields; > const VMStateDescription **subsections; > }; > diff --git a/migration/migration.c b/migration/migration.c > index 5f7e4d15e9..a17d9fb990 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -52,9 +52,14 @@ > #include "hw/qdev-properties.h" > #include "monitor/monitor.h" > #include "net/announce.h" > +#include "qemu/queue.h" > > #define MAX_THROTTLE (32 << 20) /* Migration transfer speed throttling */ > > +/* Time in milliseconds to wait for guest OS to unplug PCI device */ > +#define FAILOVER_GUEST_UNPLUG_WAIT 10000 > +#define FAILOVER_UNPLUG_RETRIES 5 > + > /* Amount of time to allocate to each "chunk" of bandwidth-throttled > * data. */ > #define BUFFER_DELAY 100 > @@ -954,6 +959,9 @@ static void fill_source_migration_info(MigrationInfo *info) > case MIGRATION_STATUS_CANCELLED: > info->has_status = true; > break; > + case MIGRATION_STATUS_WAIT_UNPLUG: > + info->has_status = true; > + break; > } > info->status = s->state; > } > @@ -1695,6 +1703,7 @@ bool migration_is_idle(void) > case MIGRATION_STATUS_COLO: > case MIGRATION_STATUS_PRE_SWITCHOVER: > case MIGRATION_STATUS_DEVICE: > + case MIGRATION_STATUS_WAIT_UNPLUG: I think the patchew failure that it shows in the migration test is because that new state is visible to the test and it's not expecting it. Dave > return false; > case MIGRATION_STATUS__MAX: > g_assert_not_reached(); > @@ -3224,6 +3233,8 @@ static void *migration_thread(void *opaque) > int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST); > MigThrError thr_error; > bool urgent = false; > + bool all_unplugged = true; > + int i = 0; > > rcu_register_thread(); > > @@ -3260,6 +3271,27 @@ static void *migration_thread(void *opaque) > > qemu_savevm_state_setup(s->to_dst_file); > > + migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > + MIGRATION_STATUS_WAIT_UNPLUG); > + > + while (i < FAILOVER_UNPLUG_RETRIES && > + s->state == MIGRATION_STATUS_WAIT_UNPLUG) { > + i++; > + qemu_sem_timedwait(&s->wait_unplug_sem, FAILOVER_GUEST_UNPLUG_WAIT); > + all_unplugged = qemu_savevm_state_guest_unplug_pending(); > + if (all_unplugged) { > + break; > + } > + } > + > + if (all_unplugged) { > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, > + MIGRATION_STATUS_ACTIVE); > + } else { > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, > + MIGRATION_STATUS_CANCELLING); > + } > + > s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; > migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > MIGRATION_STATUS_ACTIVE); > @@ -3508,6 +3540,7 @@ static void migration_instance_finalize(Object *obj) > qemu_mutex_destroy(&ms->qemu_file_lock); > g_free(params->tls_hostname); > g_free(params->tls_creds); > + qemu_sem_destroy(&ms->wait_unplug_sem); > qemu_sem_destroy(&ms->rate_limit_sem); > qemu_sem_destroy(&ms->pause_sem); > qemu_sem_destroy(&ms->postcopy_pause_sem); > @@ -3553,6 +3586,7 @@ static void migration_instance_init(Object *obj) > qemu_sem_init(&ms->postcopy_pause_rp_sem, 0); > qemu_sem_init(&ms->rp_state.rp_sem, 0); > qemu_sem_init(&ms->rate_limit_sem, 0); > + qemu_sem_init(&ms->wait_unplug_sem, 0); > qemu_mutex_init(&ms->qemu_file_lock); > } > > diff --git a/migration/migration.h b/migration/migration.h > index 4f2fe193dc..79b3dda146 100644 > --- a/migration/migration.h > +++ b/migration/migration.h > @@ -206,6 +206,9 @@ struct MigrationState > /* Flag set once the migration thread called bdrv_inactivate_all */ > bool block_inactive; > > + /* Migration is waiting for guest to unplug device */ > + QemuSemaphore wait_unplug_sem; > + > /* Migration is paused due to pause-before-switchover */ > QemuSemaphore pause_sem; > > diff --git a/migration/savevm.c b/migration/savevm.c > index bb9462a54d..26e5bde687 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -942,6 +942,20 @@ static void qemu_savevm_command_send(QEMUFile *f, > qemu_fflush(f); > } > > +static int qemu_savevm_nr_failover_devices(void) > +{ > + SaveStateEntry *se; > + int n = 0; > + > + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { > + if (se->vmsd && se->vmsd->dev_unplug_pending) { > + n++; > + } > + } > + > + return n; > +} > + > void qemu_savevm_send_colo_enable(QEMUFile *f) > { > trace_savevm_send_colo_enable(); > @@ -1113,6 +1127,28 @@ void qemu_savevm_state_header(QEMUFile *f) > } > } > > +bool qemu_savevm_state_guest_unplug_pending(void) > +{ > + int nr_failover_devs; > + SaveStateEntry *se; > + bool ret = false; > + int n = 0; > + > + nr_failover_devs = qemu_savevm_nr_failover_devices(); > + > + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { > + if (!se->vmsd || !se->vmsd->dev_unplug_pending) { > + continue; > + } > + ret = se->vmsd->dev_unplug_pending(se->opaque); > + if (!ret) { > + n++; > + } > + } > + > + return n == nr_failover_devs; > +} > + > void qemu_savevm_state_setup(QEMUFile *f) > { > SaveStateEntry *se; > diff --git a/migration/savevm.h b/migration/savevm.h > index 51a4b9caa8..ba64a7e271 100644 > --- a/migration/savevm.h > +++ b/migration/savevm.h > @@ -31,6 +31,7 @@ > > bool qemu_savevm_state_blocked(Error **errp); > void qemu_savevm_state_setup(QEMUFile *f); > +bool qemu_savevm_state_guest_unplug_pending(void); > int qemu_savevm_state_resume_prepare(MigrationState *s); > void qemu_savevm_state_header(QEMUFile *f); > int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy); > diff --git a/qapi/migration.json b/qapi/migration.json > index 52e69e2868..5a06cd489f 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -133,6 +133,9 @@ > # @device: During device serialisation when pause-before-switchover is enabled > # (since 2.11) > # > +# @wait-unplug: wait for device unplug request by guest OS to be completed. > +# (since 4.2) > +# > # Since: 2.3 > # > ## > @@ -140,7 +143,7 @@ > 'data': [ 'none', 'setup', 'cancelling', 'cancelled', > 'active', 'postcopy-active', 'postcopy-paused', > 'postcopy-recover', 'completed', 'failed', 'colo', > - 'pre-switchover', 'device' ] } > + 'pre-switchover', 'device', 'wait-unplug' ] } > > ## > # @MigrationInfo: > -- > 2.21.0 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
* Jens Freimann (jfreimann@redhat.com) wrote: > This patch adds a new migration state called wait-unplug. It is entered > after the SETUP state and will transition into ACTIVE once all devices > were succesfully unplugged from the guest. > > So if a guest doesn't respond or takes long to honor the unplug request > the user will see the migration state 'wait-unplug'. > > In the migration thread we query failover devices if they're are still > pending the guest unplug. When all are unplugged the migration > continues. We give it a defined number of iterations including small > waiting periods before we proceed. > > Signed-off-by: Jens Freimann <jfreimann@redhat.com> > --- > include/migration/vmstate.h | 2 ++ > migration/migration.c | 34 ++++++++++++++++++++++++++++++++++ > migration/migration.h | 3 +++ > migration/savevm.c | 36 ++++++++++++++++++++++++++++++++++++ > migration/savevm.h | 1 + > qapi/migration.json | 5 ++++- > 6 files changed, 80 insertions(+), 1 deletion(-) > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index 1fbfd099dd..39ef125225 100644 > --- a/include/migration/vmstate.h > +++ b/include/migration/vmstate.h > @@ -186,6 +186,8 @@ struct VMStateDescription { > int (*pre_save)(void *opaque); > int (*post_save)(void *opaque); > bool (*needed)(void *opaque); > + bool (*dev_unplug_pending)(void *opaque); > + > const VMStateField *fields; > const VMStateDescription **subsections; > }; > diff --git a/migration/migration.c b/migration/migration.c > index 5f7e4d15e9..a17d9fb990 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -52,9 +52,14 @@ > #include "hw/qdev-properties.h" > #include "monitor/monitor.h" > #include "net/announce.h" > +#include "qemu/queue.h" > > #define MAX_THROTTLE (32 << 20) /* Migration transfer speed throttling */ > > +/* Time in milliseconds to wait for guest OS to unplug PCI device */ > +#define FAILOVER_GUEST_UNPLUG_WAIT 10000 > +#define FAILOVER_UNPLUG_RETRIES 5 > + > /* Amount of time to allocate to each "chunk" of bandwidth-throttled > * data. */ > #define BUFFER_DELAY 100 > @@ -954,6 +959,9 @@ static void fill_source_migration_info(MigrationInfo *info) > case MIGRATION_STATUS_CANCELLED: > info->has_status = true; > break; > + case MIGRATION_STATUS_WAIT_UNPLUG: > + info->has_status = true; > + break; > } > info->status = s->state; > } > @@ -1695,6 +1703,7 @@ bool migration_is_idle(void) > case MIGRATION_STATUS_COLO: > case MIGRATION_STATUS_PRE_SWITCHOVER: > case MIGRATION_STATUS_DEVICE: > + case MIGRATION_STATUS_WAIT_UNPLUG: > return false; > case MIGRATION_STATUS__MAX: > g_assert_not_reached(); > @@ -3224,6 +3233,8 @@ static void *migration_thread(void *opaque) > int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST); > MigThrError thr_error; > bool urgent = false; > + bool all_unplugged = true; > + int i = 0; > > rcu_register_thread(); > > @@ -3260,6 +3271,27 @@ static void *migration_thread(void *opaque) > > qemu_savevm_state_setup(s->to_dst_file); > > + migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > + MIGRATION_STATUS_WAIT_UNPLUG); I think I'd prefer if you only went into this state if you had any devices that were going to need unplugging. > + while (i < FAILOVER_UNPLUG_RETRIES && > + s->state == MIGRATION_STATUS_WAIT_UNPLUG) { > + i++; > + qemu_sem_timedwait(&s->wait_unplug_sem, FAILOVER_GUEST_UNPLUG_WAIT); > + all_unplugged = qemu_savevm_state_guest_unplug_pending(); > + if (all_unplugged) { > + break; > + } > + } > + > + if (all_unplugged) { > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, > + MIGRATION_STATUS_ACTIVE); > + } else { > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, > + MIGRATION_STATUS_CANCELLING); > + } I think you can get rid of both the timeout and the count and just make sure that migrate_cancel works at this point. This pushes the problem up a layer, which I think is fine. > s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; > migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > MIGRATION_STATUS_ACTIVE); > @@ -3508,6 +3540,7 @@ static void migration_instance_finalize(Object *obj) > qemu_mutex_destroy(&ms->qemu_file_lock); > g_free(params->tls_hostname); > g_free(params->tls_creds); > + qemu_sem_destroy(&ms->wait_unplug_sem); > qemu_sem_destroy(&ms->rate_limit_sem); > qemu_sem_destroy(&ms->pause_sem); > qemu_sem_destroy(&ms->postcopy_pause_sem); > @@ -3553,6 +3586,7 @@ static void migration_instance_init(Object *obj) > qemu_sem_init(&ms->postcopy_pause_rp_sem, 0); > qemu_sem_init(&ms->rp_state.rp_sem, 0); > qemu_sem_init(&ms->rate_limit_sem, 0); > + qemu_sem_init(&ms->wait_unplug_sem, 0); > qemu_mutex_init(&ms->qemu_file_lock); > } > > diff --git a/migration/migration.h b/migration/migration.h > index 4f2fe193dc..79b3dda146 100644 > --- a/migration/migration.h > +++ b/migration/migration.h > @@ -206,6 +206,9 @@ struct MigrationState > /* Flag set once the migration thread called bdrv_inactivate_all */ > bool block_inactive; > > + /* Migration is waiting for guest to unplug device */ > + QemuSemaphore wait_unplug_sem; > + > /* Migration is paused due to pause-before-switchover */ > QemuSemaphore pause_sem; > > diff --git a/migration/savevm.c b/migration/savevm.c > index bb9462a54d..26e5bde687 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -942,6 +942,20 @@ static void qemu_savevm_command_send(QEMUFile *f, > qemu_fflush(f); > } > > +static int qemu_savevm_nr_failover_devices(void) > +{ > + SaveStateEntry *se; > + int n = 0; > + > + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { > + if (se->vmsd && se->vmsd->dev_unplug_pending) { > + n++; > + } > + } > + > + return n; > +} > + > void qemu_savevm_send_colo_enable(QEMUFile *f) > { > trace_savevm_send_colo_enable(); > @@ -1113,6 +1127,28 @@ void qemu_savevm_state_header(QEMUFile *f) > } > } > > +bool qemu_savevm_state_guest_unplug_pending(void) > +{ > + int nr_failover_devs; > + SaveStateEntry *se; > + bool ret = false; > + int n = 0; > + > + nr_failover_devs = qemu_savevm_nr_failover_devices(); > + > + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { > + if (!se->vmsd || !se->vmsd->dev_unplug_pending) { > + continue; > + } > + ret = se->vmsd->dev_unplug_pending(se->opaque); > + if (!ret) { > + n++; > + } > + } > + > + return n == nr_failover_devs; > +} > + > void qemu_savevm_state_setup(QEMUFile *f) > { > SaveStateEntry *se; > diff --git a/migration/savevm.h b/migration/savevm.h > index 51a4b9caa8..ba64a7e271 100644 > --- a/migration/savevm.h > +++ b/migration/savevm.h > @@ -31,6 +31,7 @@ > > bool qemu_savevm_state_blocked(Error **errp); > void qemu_savevm_state_setup(QEMUFile *f); > +bool qemu_savevm_state_guest_unplug_pending(void); > int qemu_savevm_state_resume_prepare(MigrationState *s); > void qemu_savevm_state_header(QEMUFile *f); > int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy); > diff --git a/qapi/migration.json b/qapi/migration.json > index 52e69e2868..5a06cd489f 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -133,6 +133,9 @@ > # @device: During device serialisation when pause-before-switchover is enabled > # (since 2.11) > # > +# @wait-unplug: wait for device unplug request by guest OS to be completed. > +# (since 4.2) > +# > # Since: 2.3 > # > ## > @@ -140,7 +143,7 @@ > 'data': [ 'none', 'setup', 'cancelling', 'cancelled', > 'active', 'postcopy-active', 'postcopy-paused', > 'postcopy-recover', 'completed', 'failed', 'colo', > - 'pre-switchover', 'device' ] } > + 'pre-switchover', 'device', 'wait-unplug' ] } > > ## > # @MigrationInfo: > -- > 2.21.0 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Fri, Oct 11, 2019 at 06:11:33PM +0100, Dr. David Alan Gilbert wrote: >* Jens Freimann (jfreimann@redhat.com) wrote: >> This patch adds a new migration state called wait-unplug. It is entered >> after the SETUP state and will transition into ACTIVE once all devices >> were succesfully unplugged from the guest. >> >> So if a guest doesn't respond or takes long to honor the unplug request >> the user will see the migration state 'wait-unplug'. >> >> In the migration thread we query failover devices if they're are still >> pending the guest unplug. When all are unplugged the migration >> continues. We give it a defined number of iterations including small >> waiting periods before we proceed. >> >> Signed-off-by: Jens Freimann <jfreimann@redhat.com> [..] >> @@ -3260,6 +3271,27 @@ static void *migration_thread(void *opaque) >> >> qemu_savevm_state_setup(s->to_dst_file); >> >> + migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, >> + MIGRATION_STATUS_WAIT_UNPLUG); > >I think I'd prefer if you only went into this state if you had any >devices that were going to need unplugging. Sure, that makes sense. I'll change it. >> + while (i < FAILOVER_UNPLUG_RETRIES && >> + s->state == MIGRATION_STATUS_WAIT_UNPLUG) { >> + i++; >> + qemu_sem_timedwait(&s->wait_unplug_sem, FAILOVER_GUEST_UNPLUG_WAIT); >> + all_unplugged = qemu_savevm_state_guest_unplug_pending(); >> + if (all_unplugged) { >> + break; >> + } >> + } >> + >> + if (all_unplugged) { >> + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, >> + MIGRATION_STATUS_ACTIVE); >> + } else { >> + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, >> + MIGRATION_STATUS_CANCELLING); >> + } > >I think you can get rid of both the timeout and the count and just make >sure that migrate_cancel works at this point. I see, I need to add the new state to migration_is_setup_or_active() or a cancel won't work. >This pushes the problem up a layer, which I think is fine. Seems good to me. To be clear, you're saying I should just poll on the device unplugged state? Like while (s->state == MIGRATION_STATUS_WAIT_UNPLUG && !qemu_savevm_state_guest_unplug_pending()) { _ /* This block intentionally left blank */ } regards, Jens
* Jens Freimann (jfreimann@redhat.com) wrote: > On Fri, Oct 11, 2019 at 06:11:33PM +0100, Dr. David Alan Gilbert wrote: > > * Jens Freimann (jfreimann@redhat.com) wrote: > > > This patch adds a new migration state called wait-unplug. It is entered > > > after the SETUP state and will transition into ACTIVE once all devices > > > were succesfully unplugged from the guest. > > > > > > So if a guest doesn't respond or takes long to honor the unplug request > > > the user will see the migration state 'wait-unplug'. > > > > > > In the migration thread we query failover devices if they're are still > > > pending the guest unplug. When all are unplugged the migration > > > continues. We give it a defined number of iterations including small > > > waiting periods before we proceed. > > > > > > Signed-off-by: Jens Freimann <jfreimann@redhat.com> > [..] > > > @@ -3260,6 +3271,27 @@ static void *migration_thread(void *opaque) > > > > > > qemu_savevm_state_setup(s->to_dst_file); > > > > > > + migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > > > + MIGRATION_STATUS_WAIT_UNPLUG); > > > > I think I'd prefer if you only went into this state if you had any > > devices that were going to need unplugging. > > Sure, that makes sense. I'll change it. > > > > + while (i < FAILOVER_UNPLUG_RETRIES && > > > + s->state == MIGRATION_STATUS_WAIT_UNPLUG) { > > > + i++; > > > + qemu_sem_timedwait(&s->wait_unplug_sem, FAILOVER_GUEST_UNPLUG_WAIT); > > > + all_unplugged = qemu_savevm_state_guest_unplug_pending(); > > > + if (all_unplugged) { > > > + break; > > > + } > > > + } > > > + > > > + if (all_unplugged) { > > > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, > > > + MIGRATION_STATUS_ACTIVE); > > > + } else { > > > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, > > > + MIGRATION_STATUS_CANCELLING); > > > + } > > > > I think you can get rid of both the timeout and the count and just make > > sure that migrate_cancel works at this point. > > I see, I need to add the new state to migration_is_setup_or_active() or > a cancel won't work. You probably need to do that anyway given all the other places is_setup_or_active is called. > > This pushes the problem up a layer, which I think is fine. > > Seems good to me. To be clear, you're saying I should just poll on > the device unplugged state? Like > > while (s->state == MIGRATION_STATUS_WAIT_UNPLUG && > !qemu_savevm_state_guest_unplug_pending()) { > _ /* This block intentionally left blank */ > } I'd keep the qemu_sem_timedwait in there, but with a short time out (e.g. 250ms say); that way it doesn't eat cpu, but also the cancel still happens quickly. Dave > > regards, > Jens -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Tue, Oct 15, 2019 at 11:50:08AM +0100, Dr. David Alan Gilbert wrote: >* Jens Freimann (jfreimann@redhat.com) wrote: >> On Fri, Oct 11, 2019 at 06:11:33PM +0100, Dr. David Alan Gilbert wrote: >> > * Jens Freimann (jfreimann@redhat.com) wrote: >> > > This patch adds a new migration state called wait-unplug. It is entered >> > > after the SETUP state and will transition into ACTIVE once all devices >> > > were succesfully unplugged from the guest. >> > > >> > > So if a guest doesn't respond or takes long to honor the unplug request >> > > the user will see the migration state 'wait-unplug'. >> > > >> > > In the migration thread we query failover devices if they're are still >> > > pending the guest unplug. When all are unplugged the migration >> > > continues. We give it a defined number of iterations including small >> > > waiting periods before we proceed. >> > > >> > > Signed-off-by: Jens Freimann <jfreimann@redhat.com> >> [..] >> > > + while (i < FAILOVER_UNPLUG_RETRIES && >> > > + s->state == MIGRATION_STATUS_WAIT_UNPLUG) { >> > > + i++; >> > > + qemu_sem_timedwait(&s->wait_unplug_sem, FAILOVER_GUEST_UNPLUG_WAIT); >> > > + all_unplugged = qemu_savevm_state_guest_unplug_pending(); >> > > + if (all_unplugged) { >> > > + break; >> > > + } >> > > + } >> > > + >> > > + if (all_unplugged) { >> > > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, >> > > + MIGRATION_STATUS_ACTIVE); >> > > + } else { >> > > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, >> > > + MIGRATION_STATUS_CANCELLING); >> > > + } >> > >> > I think you can get rid of both the timeout and the count and just make >> > sure that migrate_cancel works at this point. >> >> I see, I need to add the new state to migration_is_setup_or_active() or >> a cancel won't work. > >You probably need to do that anyway given all the other places >is_setup_or_active is called. Yes, done. >> > This pushes the problem up a layer, which I think is fine. >> >> Seems good to me. To be clear, you're saying I should just poll on >> the device unplugged state? Like >> >> while (s->state == MIGRATION_STATUS_WAIT_UNPLUG && >> !qemu_savevm_state_guest_unplug_pending()) { >> _ /* This block intentionally left blank */ >> } > >I'd keep the qemu_sem_timedwait in there, but with a short time out >(e.g. 250ms say); that way it doesn't eat cpu, but also the cancel still >happens quickly. Yes, that's what I do now and it works fine. Thanks! regards, Jens
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 1fbfd099dd..39ef125225 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -186,6 +186,8 @@ struct VMStateDescription { int (*pre_save)(void *opaque); int (*post_save)(void *opaque); bool (*needed)(void *opaque); + bool (*dev_unplug_pending)(void *opaque); + const VMStateField *fields; const VMStateDescription **subsections; }; diff --git a/migration/migration.c b/migration/migration.c index 5f7e4d15e9..a17d9fb990 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -52,9 +52,14 @@ #include "hw/qdev-properties.h" #include "monitor/monitor.h" #include "net/announce.h" +#include "qemu/queue.h" #define MAX_THROTTLE (32 << 20) /* Migration transfer speed throttling */ +/* Time in milliseconds to wait for guest OS to unplug PCI device */ +#define FAILOVER_GUEST_UNPLUG_WAIT 10000 +#define FAILOVER_UNPLUG_RETRIES 5 + /* Amount of time to allocate to each "chunk" of bandwidth-throttled * data. */ #define BUFFER_DELAY 100 @@ -954,6 +959,9 @@ static void fill_source_migration_info(MigrationInfo *info) case MIGRATION_STATUS_CANCELLED: info->has_status = true; break; + case MIGRATION_STATUS_WAIT_UNPLUG: + info->has_status = true; + break; } info->status = s->state; } @@ -1695,6 +1703,7 @@ bool migration_is_idle(void) case MIGRATION_STATUS_COLO: case MIGRATION_STATUS_PRE_SWITCHOVER: case MIGRATION_STATUS_DEVICE: + case MIGRATION_STATUS_WAIT_UNPLUG: return false; case MIGRATION_STATUS__MAX: g_assert_not_reached(); @@ -3224,6 +3233,8 @@ static void *migration_thread(void *opaque) int64_t setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST); MigThrError thr_error; bool urgent = false; + bool all_unplugged = true; + int i = 0; rcu_register_thread(); @@ -3260,6 +3271,27 @@ static void *migration_thread(void *opaque) qemu_savevm_state_setup(s->to_dst_file); + migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, + MIGRATION_STATUS_WAIT_UNPLUG); + + while (i < FAILOVER_UNPLUG_RETRIES && + s->state == MIGRATION_STATUS_WAIT_UNPLUG) { + i++; + qemu_sem_timedwait(&s->wait_unplug_sem, FAILOVER_GUEST_UNPLUG_WAIT); + all_unplugged = qemu_savevm_state_guest_unplug_pending(); + if (all_unplugged) { + break; + } + } + + if (all_unplugged) { + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, + MIGRATION_STATUS_ACTIVE); + } else { + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, + MIGRATION_STATUS_CANCELLING); + } + s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, MIGRATION_STATUS_ACTIVE); @@ -3508,6 +3540,7 @@ static void migration_instance_finalize(Object *obj) qemu_mutex_destroy(&ms->qemu_file_lock); g_free(params->tls_hostname); g_free(params->tls_creds); + qemu_sem_destroy(&ms->wait_unplug_sem); qemu_sem_destroy(&ms->rate_limit_sem); qemu_sem_destroy(&ms->pause_sem); qemu_sem_destroy(&ms->postcopy_pause_sem); @@ -3553,6 +3586,7 @@ static void migration_instance_init(Object *obj) qemu_sem_init(&ms->postcopy_pause_rp_sem, 0); qemu_sem_init(&ms->rp_state.rp_sem, 0); qemu_sem_init(&ms->rate_limit_sem, 0); + qemu_sem_init(&ms->wait_unplug_sem, 0); qemu_mutex_init(&ms->qemu_file_lock); } diff --git a/migration/migration.h b/migration/migration.h index 4f2fe193dc..79b3dda146 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -206,6 +206,9 @@ struct MigrationState /* Flag set once the migration thread called bdrv_inactivate_all */ bool block_inactive; + /* Migration is waiting for guest to unplug device */ + QemuSemaphore wait_unplug_sem; + /* Migration is paused due to pause-before-switchover */ QemuSemaphore pause_sem; diff --git a/migration/savevm.c b/migration/savevm.c index bb9462a54d..26e5bde687 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -942,6 +942,20 @@ static void qemu_savevm_command_send(QEMUFile *f, qemu_fflush(f); } +static int qemu_savevm_nr_failover_devices(void) +{ + SaveStateEntry *se; + int n = 0; + + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { + if (se->vmsd && se->vmsd->dev_unplug_pending) { + n++; + } + } + + return n; +} + void qemu_savevm_send_colo_enable(QEMUFile *f) { trace_savevm_send_colo_enable(); @@ -1113,6 +1127,28 @@ void qemu_savevm_state_header(QEMUFile *f) } } +bool qemu_savevm_state_guest_unplug_pending(void) +{ + int nr_failover_devs; + SaveStateEntry *se; + bool ret = false; + int n = 0; + + nr_failover_devs = qemu_savevm_nr_failover_devices(); + + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { + if (!se->vmsd || !se->vmsd->dev_unplug_pending) { + continue; + } + ret = se->vmsd->dev_unplug_pending(se->opaque); + if (!ret) { + n++; + } + } + + return n == nr_failover_devs; +} + void qemu_savevm_state_setup(QEMUFile *f) { SaveStateEntry *se; diff --git a/migration/savevm.h b/migration/savevm.h index 51a4b9caa8..ba64a7e271 100644 --- a/migration/savevm.h +++ b/migration/savevm.h @@ -31,6 +31,7 @@ bool qemu_savevm_state_blocked(Error **errp); void qemu_savevm_state_setup(QEMUFile *f); +bool qemu_savevm_state_guest_unplug_pending(void); int qemu_savevm_state_resume_prepare(MigrationState *s); void qemu_savevm_state_header(QEMUFile *f); int qemu_savevm_state_iterate(QEMUFile *f, bool postcopy); diff --git a/qapi/migration.json b/qapi/migration.json index 52e69e2868..5a06cd489f 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -133,6 +133,9 @@ # @device: During device serialisation when pause-before-switchover is enabled # (since 2.11) # +# @wait-unplug: wait for device unplug request by guest OS to be completed. +# (since 4.2) +# # Since: 2.3 # ## @@ -140,7 +143,7 @@ 'data': [ 'none', 'setup', 'cancelling', 'cancelled', 'active', 'postcopy-active', 'postcopy-paused', 'postcopy-recover', 'completed', 'failed', 'colo', - 'pre-switchover', 'device' ] } + 'pre-switchover', 'device', 'wait-unplug' ] } ## # @MigrationInfo:
This patch adds a new migration state called wait-unplug. It is entered after the SETUP state and will transition into ACTIVE once all devices were succesfully unplugged from the guest. So if a guest doesn't respond or takes long to honor the unplug request the user will see the migration state 'wait-unplug'. In the migration thread we query failover devices if they're are still pending the guest unplug. When all are unplugged the migration continues. We give it a defined number of iterations including small waiting periods before we proceed. Signed-off-by: Jens Freimann <jfreimann@redhat.com> --- include/migration/vmstate.h | 2 ++ migration/migration.c | 34 ++++++++++++++++++++++++++++++++++ migration/migration.h | 3 +++ migration/savevm.c | 36 ++++++++++++++++++++++++++++++++++++ migration/savevm.h | 1 + qapi/migration.json | 5 ++++- 6 files changed, 80 insertions(+), 1 deletion(-)