Passing MigrationState to notifiers is unsound because they could access
unstable migration state internals or even modify the state. Instead, pass
the minimal info needed in a new MigrationEvent struct, which could be
extended in the future if needed.
Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
hw/net/virtio-net.c | 12 +++++++-----
hw/vfio/migration.c | 8 +++++---
include/migration/misc.h | 5 +++++
migration/migration.c | 5 ++++-
net/vhost-vdpa.c | 7 ++++---
ui/spice-core.c | 9 +++++----
6 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 9570353..71e1133 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3499,7 +3499,7 @@ out:
return !err;
}
-static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s)
+static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationEvent *e)
{
bool should_be_hidden;
Error *err = NULL;
@@ -3511,7 +3511,7 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s)
should_be_hidden = qatomic_read(&n->failover_primary_hidden);
- if (migration_in_setup(s) && !should_be_hidden) {
+ if (e->state == MIGRATION_STATUS_SETUP && !should_be_hidden) {
if (failover_unplug_primary(n, dev)) {
vmstate_unregister(VMSTATE_IF(dev), qdev_get_vmsd(dev), dev);
qapi_event_send_unplug_primary(dev->id);
@@ -3519,7 +3519,8 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s)
} else {
warn_report("couldn't unplug primary device");
}
- } else if (migration_has_failed(s)) {
+ } else if (e->state == MIGRATION_STATUS_FAILED ||
+ e->state == MIGRATION_STATUS_CANCELLED) {
/* We already unplugged the device let's plug it back */
if (!failover_replug_primary(n, dev, &err)) {
if (err) {
@@ -3532,9 +3533,10 @@ static void virtio_net_handle_migration_primary(VirtIONet *n, MigrationState *s)
static int virtio_net_migration_state_notifier(NotifierWithReturn *notifier,
void *data, Error **errp)
{
- MigrationState *s = data;
+ MigrationEvent *e = data;
+
VirtIONet *n = container_of(notifier, VirtIONet, migration_state);
- virtio_net_handle_migration_primary(n, s);
+ virtio_net_handle_migration_primary(n, e);
return 0;
}
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 6b6acc4..746ec08 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -757,19 +757,21 @@ static void vfio_vmstate_change(void *opaque, bool running, RunState state)
static int vfio_migration_state_notifier(NotifierWithReturn *notifier,
void *data, Error **errp)
{
- MigrationState *s = data;
+ MigrationEvent *e = data;
VFIOMigration *migration = container_of(notifier, VFIOMigration,
migration_state);
VFIODevice *vbasedev = migration->vbasedev;
trace_vfio_migration_state_notifier(vbasedev->name,
- MigrationStatus_str(s->state));
+ MigrationStatus_str(e->state));
- switch (s->state) {
+ switch (e->state) {
case MIGRATION_STATUS_CANCELLING:
case MIGRATION_STATUS_CANCELLED:
case MIGRATION_STATUS_FAILED:
vfio_migration_set_state_or_reset(vbasedev, VFIO_DEVICE_STATE_RUNNING);
+ default:
+ break;
}
return 0;
}
diff --git a/include/migration/misc.h b/include/migration/misc.h
index dcc98bb..0b4ce0f 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -60,6 +60,11 @@ void migration_object_init(void);
void migration_shutdown(void);
bool migration_is_idle(void);
bool migration_is_active(MigrationState *);
+
+typedef struct MigrationEvent {
+ MigrationStatus state;
+} MigrationEvent;
+
void migration_add_notifier(NotifierWithReturn *notify,
NotifierWithReturnFunc func);
void migration_remove_notifier(NotifierWithReturn *notify);
diff --git a/migration/migration.c b/migration/migration.c
index ad3acd1..4c5180c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1441,7 +1441,10 @@ void migration_remove_notifier(NotifierWithReturn *notify)
void migration_call_notifiers(MigrationState *s)
{
- notifier_with_return_list_notify(&migration_state_notifiers, s, 0);
+ MigrationEvent e;
+
+ e.state = s->state;
+ notifier_with_return_list_notify(&migration_state_notifiers, &e, 0);
}
bool migration_in_setup(MigrationState *s)
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 1c00519..f96ac75 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -325,13 +325,14 @@ static void vhost_vdpa_net_log_global_enable(VhostVDPAState *s, bool enable)
static int vdpa_net_migration_state_notifier(NotifierWithReturn *notifier,
void *data, Error **errp)
{
- MigrationState *migration = data;
+ MigrationEvent *e = data;
VhostVDPAState *s = container_of(notifier, VhostVDPAState,
migration_state);
- if (migration_in_setup(migration)) {
+ if (e->state == MIGRATION_STATUS_SETUP) {
vhost_vdpa_net_log_global_enable(s, true);
- } else if (migration_has_failed(migration)) {
+ } else if (e->state == MIGRATION_STATUS_FAILED ||
+ e->state == MIGRATION_STATUS_CANCELLED) {
vhost_vdpa_net_log_global_enable(s, false);
}
return 0;
diff --git a/ui/spice-core.c b/ui/spice-core.c
index e43a93f..74aa3bc 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -571,19 +571,20 @@ static SpiceInfo *qmp_query_spice_real(Error **errp)
static int migration_state_notifier(NotifierWithReturn *notifier,
void *data, Error **errp)
{
- MigrationState *s = data;
+ MigrationEvent *e = data;
if (!spice_have_target_host) {
return 0;
}
- if (migration_in_setup(s)) {
+ if (e->state == MIGRATION_STATUS_SETUP) {
spice_server_migrate_start(spice_server);
- } else if (migration_has_finished(s) ||
+ } else if (e->state == MIGRATION_STATUS_COMPLETED ||
migration_in_postcopy_after_devices()) {
spice_server_migrate_end(spice_server, true);
spice_have_target_host = false;
- } else if (migration_has_failed(s)) {
+ } else if (e->state == MIGRATION_STATUS_FAILED ||
+ e->state == MIGRATION_STATUS_CANCELLED) {
spice_server_migrate_end(spice_server, false);
spice_have_target_host = false;
}
--
1.8.3.1