| Message ID | 20191025121930.6855-9-jfreimann@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | add failover feature for assigned network devices | expand |
* Jens Freimann (jfreimann@redhat.com) wrote: > This patch adds a new migration state called wait-unplug. It is entered > after the SETUP state if failover devices are present. It 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. If one device won't unplug migration will stay in wait_unplug > state. > > Signed-off-by: Jens Freimann <jfreimann@redhat.com> > Acked-by: Cornelia Huck <cohuck@redhat.com> I think this is OK, so Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> but see question below > --- > include/migration/vmstate.h | 2 ++ > migration/migration.c | 21 +++++++++++++++++++++ > migration/migration.h | 3 +++ > migration/savevm.c | 36 ++++++++++++++++++++++++++++++++++++ > migration/savevm.h | 2 ++ > qapi/migration.json | 5 ++++- > 6 files changed, 68 insertions(+), 1 deletion(-) > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index b9ee563aa4..ac4f46a67d 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 3febd0f8f3..51764f2565 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -52,6 +52,7 @@ > #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 */ > > @@ -819,6 +820,7 @@ bool migration_is_setup_or_active(int state) > case MIGRATION_STATUS_SETUP: > case MIGRATION_STATUS_PRE_SWITCHOVER: > case MIGRATION_STATUS_DEVICE: > + case MIGRATION_STATUS_WAIT_UNPLUG: > return true; > > default: > @@ -954,6 +956,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; > } > @@ -1694,6 +1699,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(); > @@ -3264,6 +3270,19 @@ static void *migration_thread(void *opaque) > > qemu_savevm_state_setup(s->to_dst_file); > > + if (qemu_savevm_nr_failover_devices()) { > + migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > + MIGRATION_STATUS_WAIT_UNPLUG); > + > + while (s->state == MIGRATION_STATUS_WAIT_UNPLUG && > + !qemu_savevm_state_guest_unplug_pending()) { > + qemu_sem_timedwait(&s->wait_unplug_sem, 250); > + } > + > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, > + MIGRATION_STATUS_ACTIVE); > + } > + > s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; > migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > MIGRATION_STATUS_ACTIVE); > @@ -3511,6 +3530,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); > @@ -3556,6 +3576,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 8d95e261f6..0f18dea49e 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -1113,6 +1113,42 @@ void qemu_savevm_state_header(QEMUFile *f) > } > } > > +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; > +} > + > +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; I was expecting != I think? If all the devices say they've got one pending then doesn't n==nr_failover_devs and it returns true? But then what happens if only one has one pending? Dave > +} > + > void qemu_savevm_state_setup(QEMUFile *f) > { > SaveStateEntry *se; > diff --git a/migration/savevm.h b/migration/savevm.h > index 51a4b9caa8..c42b9c80ee 100644 > --- a/migration/savevm.h > +++ b/migration/savevm.h > @@ -31,6 +31,8 @@ > > bool qemu_savevm_state_blocked(Error **errp); > void qemu_savevm_state_setup(QEMUFile *f); > +int qemu_savevm_nr_failover_devices(void); > +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 e9e7a97c03..b7348d0c8b 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
Dr. David Alan Gilbert <dgilbert@redhat.com> schrieb am Di., 29. Okt. 2019, 03:57: > * Jens Freimann (jfreimann@redhat.com) wrote: > > This patch adds a new migration state called wait-unplug. It is entered > > after the SETUP state if failover devices are present. It 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. If one device won't unplug migration will stay in wait_unplug > > state. > > > > Signed-off-by: Jens Freimann <jfreimann@redhat.com> > > Acked-by: Cornelia Huck <cohuck@redhat.com> > > I think this is OK, so > > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > but see question below > > > --- > > include/migration/vmstate.h | 2 ++ > > migration/migration.c | 21 +++++++++++++++++++++ > > migration/migration.h | 3 +++ > > migration/savevm.c | 36 ++++++++++++++++++++++++++++++++++++ > > migration/savevm.h | 2 ++ > > qapi/migration.json | 5 ++++- > > 6 files changed, 68 insertions(+), 1 deletion(-) > > > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > index b9ee563aa4..ac4f46a67d 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 3febd0f8f3..51764f2565 100644 > > --- a/migration/migration.c > > +++ b/migration/migration.c > > @@ -52,6 +52,7 @@ > > #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 */ > > > > @@ -819,6 +820,7 @@ bool migration_is_setup_or_active(int state) > > case MIGRATION_STATUS_SETUP: > > case MIGRATION_STATUS_PRE_SWITCHOVER: > > case MIGRATION_STATUS_DEVICE: > > + case MIGRATION_STATUS_WAIT_UNPLUG: > > return true; > > > > default: > > @@ -954,6 +956,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; > > } > > @@ -1694,6 +1699,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(); > > @@ -3264,6 +3270,19 @@ static void *migration_thread(void *opaque) > > > > qemu_savevm_state_setup(s->to_dst_file); > > > > + if (qemu_savevm_nr_failover_devices()) { > > + migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > > + MIGRATION_STATUS_WAIT_UNPLUG); > > + > > + while (s->state == MIGRATION_STATUS_WAIT_UNPLUG && > > + !qemu_savevm_state_guest_unplug_pending()) { > > + qemu_sem_timedwait(&s->wait_unplug_sem, 250); > > + } > > + > > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, > > + MIGRATION_STATUS_ACTIVE); > > + } > > + > > s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; > > migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > > MIGRATION_STATUS_ACTIVE); > > @@ -3511,6 +3530,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); > > @@ -3556,6 +3576,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 8d95e261f6..0f18dea49e 100644 > > --- a/migration/savevm.c > > +++ b/migration/savevm.c > > @@ -1113,6 +1113,42 @@ void qemu_savevm_state_header(QEMUFile *f) > > } > > } > > > > +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; > > +} > > + > > +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; > > I was expecting != I think? If all the devices say > they've got one pending then doesn't n==nr_failover_devs and > it returns true? But then what happens if only one has one pending? > It's increased when unplug pending is false, which means the device is done. So it returns true when all devices are done with unplugging. It is correct but not obvious. I can reverse it in a follow up to make it more clear. regards, Jens > > Dave > > > +} > > + > > void qemu_savevm_state_setup(QEMUFile *f) > > { > > SaveStateEntry *se; > > diff --git a/migration/savevm.h b/migration/savevm.h > > index 51a4b9caa8..c42b9c80ee 100644 > > --- a/migration/savevm.h > > +++ b/migration/savevm.h > > @@ -31,6 +31,8 @@ > > > > bool qemu_savevm_state_blocked(Error **errp); > > void qemu_savevm_state_setup(QEMUFile *f); > > +int qemu_savevm_nr_failover_devices(void); > > +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 e9e7a97c03..b7348d0c8b 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 (jens@freimann.org) wrote: > Dr. David Alan Gilbert <dgilbert@redhat.com> schrieb am Di., 29. Okt. 2019, > 03:57: > > > * Jens Freimann (jfreimann@redhat.com) wrote: > > > This patch adds a new migration state called wait-unplug. It is entered > > > after the SETUP state if failover devices are present. It 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. If one device won't unplug migration will stay in wait_unplug > > > state. > > > > > > Signed-off-by: Jens Freimann <jfreimann@redhat.com> > > > Acked-by: Cornelia Huck <cohuck@redhat.com> > > > > I think this is OK, so > > > > > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > > > but see question below > > > > > --- > > > include/migration/vmstate.h | 2 ++ > > > migration/migration.c | 21 +++++++++++++++++++++ > > > migration/migration.h | 3 +++ > > > migration/savevm.c | 36 ++++++++++++++++++++++++++++++++++++ > > > migration/savevm.h | 2 ++ > > > qapi/migration.json | 5 ++++- > > > 6 files changed, 68 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > > index b9ee563aa4..ac4f46a67d 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 3febd0f8f3..51764f2565 100644 > > > --- a/migration/migration.c > > > +++ b/migration/migration.c > > > @@ -52,6 +52,7 @@ > > > #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 */ > > > > > > @@ -819,6 +820,7 @@ bool migration_is_setup_or_active(int state) > > > case MIGRATION_STATUS_SETUP: > > > case MIGRATION_STATUS_PRE_SWITCHOVER: > > > case MIGRATION_STATUS_DEVICE: > > > + case MIGRATION_STATUS_WAIT_UNPLUG: > > > return true; > > > > > > default: > > > @@ -954,6 +956,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; > > > } > > > @@ -1694,6 +1699,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(); > > > @@ -3264,6 +3270,19 @@ static void *migration_thread(void *opaque) > > > > > > qemu_savevm_state_setup(s->to_dst_file); > > > > > > + if (qemu_savevm_nr_failover_devices()) { > > > + migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > > > + MIGRATION_STATUS_WAIT_UNPLUG); > > > + > > > + while (s->state == MIGRATION_STATUS_WAIT_UNPLUG && > > > + !qemu_savevm_state_guest_unplug_pending()) { > > > + qemu_sem_timedwait(&s->wait_unplug_sem, 250); > > > + } > > > + > > > + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, > > > + MIGRATION_STATUS_ACTIVE); > > > + } > > > + > > > s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; > > > migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, > > > MIGRATION_STATUS_ACTIVE); > > > @@ -3511,6 +3530,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); > > > @@ -3556,6 +3576,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 8d95e261f6..0f18dea49e 100644 > > > --- a/migration/savevm.c > > > +++ b/migration/savevm.c > > > @@ -1113,6 +1113,42 @@ void qemu_savevm_state_header(QEMUFile *f) > > > } > > > } > > > > > > +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; > > > +} > > > + > > > +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; > > > > I was expecting != I think? If all the devices say > > they've got one pending then doesn't n==nr_failover_devs and > > it returns true? But then what happens if only one has one pending? > > > > It's increased when unplug pending is false, which means the device is > done. So it returns true when all devices are done with unplugging. It is > correct but not obvious. I can reverse it in a follow up to make it more > clear. Yes it doesn't quite match the name of the function; and/or add some comments! Dave > regards, > Jens > > > > > Dave > > > > > +} > > > + > > > void qemu_savevm_state_setup(QEMUFile *f) > > > { > > > SaveStateEntry *se; > > > diff --git a/migration/savevm.h b/migration/savevm.h > > > index 51a4b9caa8..c42b9c80ee 100644 > > > --- a/migration/savevm.h > > > +++ b/migration/savevm.h > > > @@ -31,6 +31,8 @@ > > > > > > bool qemu_savevm_state_blocked(Error **errp); > > > void qemu_savevm_state_setup(QEMUFile *f); > > > +int qemu_savevm_nr_failover_devices(void); > > > +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 e9e7a97c03..b7348d0c8b 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 > > > > > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index b9ee563aa4..ac4f46a67d 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 3febd0f8f3..51764f2565 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -52,6 +52,7 @@ #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 */ @@ -819,6 +820,7 @@ bool migration_is_setup_or_active(int state) case MIGRATION_STATUS_SETUP: case MIGRATION_STATUS_PRE_SWITCHOVER: case MIGRATION_STATUS_DEVICE: + case MIGRATION_STATUS_WAIT_UNPLUG: return true; default: @@ -954,6 +956,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; } @@ -1694,6 +1699,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(); @@ -3264,6 +3270,19 @@ static void *migration_thread(void *opaque) qemu_savevm_state_setup(s->to_dst_file); + if (qemu_savevm_nr_failover_devices()) { + migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, + MIGRATION_STATUS_WAIT_UNPLUG); + + while (s->state == MIGRATION_STATUS_WAIT_UNPLUG && + !qemu_savevm_state_guest_unplug_pending()) { + qemu_sem_timedwait(&s->wait_unplug_sem, 250); + } + + migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, + MIGRATION_STATUS_ACTIVE); + } + s->setup_time = qemu_clock_get_ms(QEMU_CLOCK_HOST) - setup_start; migrate_set_state(&s->state, MIGRATION_STATUS_SETUP, MIGRATION_STATUS_ACTIVE); @@ -3511,6 +3530,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); @@ -3556,6 +3576,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 8d95e261f6..0f18dea49e 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1113,6 +1113,42 @@ void qemu_savevm_state_header(QEMUFile *f) } } +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; +} + +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..c42b9c80ee 100644 --- a/migration/savevm.h +++ b/migration/savevm.h @@ -31,6 +31,8 @@ bool qemu_savevm_state_blocked(Error **errp); void qemu_savevm_state_setup(QEMUFile *f); +int qemu_savevm_nr_failover_devices(void); +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 e9e7a97c03..b7348d0c8b 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: