[Qemu-devel] [PATCH v3 0/4] dataplane: virtio-blk live migration with x-data-plane=on

[Qemu-devel] [PATCH v3 0/4] dataplane: virtio-blk live migration with x-data-plane=on

[Stefan Hajnoczi]

v3:
 * Fix spice migration, also needed for dataplane migration [kwolf]

v2:
 * Rebase onto qemu.git/master
 * Split bdrv_in_use() into separate patch, it's a bug fix [bonzini]

These patches add live migration support to -device virtio-blk-pci,x-data-plane=on.

Patches 1 & 2 make migration state notifiers reliable and fix spice migration.
Dataplane migration faces the same issues so I'm including the patches here.

Patch 3 implements a switch from dataplane mode back to regular virtio-blk mode
when migration starts.  This way live migration works.

Patch 4 is a bug fix: we need to check the device is not in use when creating
the dataplane thread.  It applies to live migration but it turns out the same
problem can be hit without migration too, see the commit description.

If migration is cancelled or the guest accesses the virtio-blk device after
completion, dataplane starts again.

Since this approach is so small, it's more palatable for QEMU 1.6 than trying
to make vring.c log dirty memory.  It makes dataplane usable in situations
where live migration is a requirement.

Stefan Hajnoczi (4):
  migration: notify migration state before starting thread
  migration: fix spice migration
  dataplane: enable virtio-blk x-data-plane=on live migration
  dataplane: refuse to start if device is already in use

 hw/block/dataplane/virtio-blk.c | 17 ++++++++---------
 hw/block/virtio-blk.c           | 32 ++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-blk.h  |  1 +
 include/migration/migration.h   |  2 +-
 migration.c                     |  8 +++++---
 ui/spice-core.c                 |  2 +-
 6 files changed, 48 insertions(+), 14 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH v3 1/4] migration: notify migration state before starting thread

[Stefan Hajnoczi]

The migration thread runs outside the QEMU global mutex when possible.
Therefore we must notify migration state change *before* starting the
migration thread.

This allows registered listeners to act before live migration iterations
begin.  Therefore they can get into a state that allows for live
migration.  When the migration thread starts everything will be ready.

Without this patch there is a race condition during migration setup,
depending on whether the migration thread has already transitioned from
SETUP to ACTIVE state.

Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 migration.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/migration.c b/migration.c
index e12e784..a5ed26b 100644
--- a/migration.c
+++ b/migration.c
@@ -658,7 +658,9 @@ void migrate_fd_connect(MigrationState *s)
     qemu_file_set_rate_limit(s->file,
                              s->bandwidth_limit / XFER_LIMIT_RATIO);
 
+    /* Notify before starting migration thread */
+    notifier_list_notify(&migration_state_notifiers, s);
+
     qemu_thread_create(&s->thread, migration_thread, s,
                        QEMU_THREAD_JOINABLE);
-    notifier_list_notify(&migration_state_notifiers, s);
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 2/4] migration: fix spice migration

[Stefan Hajnoczi]

Commit 29ae8a4133082e16970c9d4be09f4b6a15034617 ("rdma: introduce
MIG_STATE_NONE and change MIG_STATE_SETUP state transition") changed the
state transitions during migration setup.

Spice used to be notified with MIG_STATE_ACTIVE and it detected this
using migration_is_active().  Spice is now notified with
MIG_STATE_SETUP and migration_is_active() no longer works.

Replace migration_is_active() with migration_in_setup() to fix spice
migration.

Cc: Michael R. Hines <mrhines@us.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 include/migration/migration.h | 2 +-
 migration.c                   | 4 ++--
 ui/spice-core.c               | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index 08c772d..140e6b4 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -90,7 +90,7 @@ int migrate_fd_close(MigrationState *s);
 
 void add_migration_state_change_notifier(Notifier *notify);
 void remove_migration_state_change_notifier(Notifier *notify);
-bool migration_is_active(MigrationState *);
+bool migration_in_setup(MigrationState *);
 bool migration_has_finished(MigrationState *);
 bool migration_has_failed(MigrationState *);
 MigrationState *migrate_get_current(void);
diff --git a/migration.c b/migration.c
index a5ed26b..9fc7294 100644
--- a/migration.c
+++ b/migration.c
@@ -338,9 +338,9 @@ void remove_migration_state_change_notifier(Notifier *notify)
     notifier_remove(notify);
 }
 
-bool migration_is_active(MigrationState *s)
+bool migration_in_setup(MigrationState *s)
 {
-    return s->state == MIG_STATE_ACTIVE;
+    return s->state == MIG_STATE_SETUP;
 }
 
 bool migration_has_finished(MigrationState *s)
diff --git a/ui/spice-core.c b/ui/spice-core.c
index f308fd9..033fd89 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -563,7 +563,7 @@ static void migration_state_notifier(Notifier *notifier, void *data)
 {
     MigrationState *s = data;
 
-    if (migration_is_active(s)) {
+    if (migration_in_setup(s)) {
         spice_server_migrate_start(spice_server);
     } else if (migration_has_finished(s)) {
         spice_server_migrate_end(spice_server, true);
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 3/4] dataplane: enable virtio-blk x-data-plane=on live migration

[Stefan Hajnoczi]

Although the dataplane thread does not cooperate with dirty memory
logging yet it's fairly easy to temporarily disable dataplane during
live migration.  This way virtio-blk can live migrate when
x-data-plane=on.

The dataplane thread will restart after migration is cancelled or if the
guest resuming virtio-blk operation after migration completes.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/dataplane/virtio-blk.c |  9 ---------
 hw/block/virtio-blk.c           | 32 ++++++++++++++++++++++++++++++++
 include/hw/virtio/virtio-blk.h  |  1 +
 3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 2faed43..63c3ffa 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -18,7 +18,6 @@
 #include "qemu/error-report.h"
 #include "hw/virtio/dataplane/vring.h"
 #include "ioq.h"
-#include "migration/migration.h"
 #include "block/block.h"
 #include "hw/virtio/virtio-blk.h"
 #include "virtio-blk.h"
@@ -69,8 +68,6 @@ struct VirtIOBlockDataPlane {
                                              queue */
 
     unsigned int num_reqs;
-
-    Error *migration_blocker;
 };
 
 /* Raise an interrupt to signal guest, if necessary */
@@ -433,10 +430,6 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
     /* Prevent block operations that conflict with data plane thread */
     bdrv_set_in_use(blk->conf.bs, 1);
 
-    error_setg(&s->migration_blocker,
-            "x-data-plane does not support migration");
-    migrate_add_blocker(s->migration_blocker);
-
     *dataplane = s;
     return true;
 }
@@ -448,8 +441,6 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s)
     }
 
     virtio_blk_data_plane_stop(s);
-    migrate_del_blocker(s->migration_blocker);
-    error_free(s->migration_blocker);
     bdrv_set_in_use(s->blk->conf.bs, 0);
     g_free(s);
 }
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index cf12469..1237b6a 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -19,6 +19,7 @@
 #include "hw/virtio/virtio-blk.h"
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
 # include "dataplane/virtio-blk.h"
+# include "migration/migration.h"
 #endif
 #include "block/scsi.h"
 #ifdef __linux__
@@ -628,6 +629,34 @@ void virtio_blk_set_conf(DeviceState *dev, VirtIOBlkConf *blk)
     memcpy(&(s->blk), blk, sizeof(struct VirtIOBlkConf));
 }
 
+#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+/* Disable dataplane thread during live migration since it does not
+ * update the dirty memory bitmap yet.
+ */
+static void virtio_blk_migration_state_changed(Notifier *notifier, void *data)
+{
+    VirtIOBlock *s = container_of(notifier, VirtIOBlock,
+                                  migration_state_notifier);
+    MigrationState *mig = data;
+
+    if (migration_in_setup(mig)) {
+        if (!s->dataplane) {
+            return;
+        }
+        virtio_blk_data_plane_destroy(s->dataplane);
+        s->dataplane = NULL;
+    } else if (migration_has_finished(mig) ||
+               migration_has_failed(mig)) {
+        if (s->dataplane) {
+            return;
+        }
+        bdrv_drain_all(); /* complete in-flight non-dataplane requests */
+        virtio_blk_data_plane_create(VIRTIO_DEVICE(s), &s->blk,
+                                     &s->dataplane);
+    }
+}
+#endif /* CONFIG_VIRTIO_BLK_DATA_PLANE */
+
 static int virtio_blk_device_init(VirtIODevice *vdev)
 {
     DeviceState *qdev = DEVICE(vdev);
@@ -664,6 +693,8 @@ static int virtio_blk_device_init(VirtIODevice *vdev)
         virtio_cleanup(vdev);
         return -1;
     }
+    s->migration_state_notifier.notify = virtio_blk_migration_state_changed;
+    add_migration_state_change_notifier(&s->migration_state_notifier);
 #endif
 
     s->change = qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
@@ -683,6 +714,7 @@ static int virtio_blk_device_exit(DeviceState *dev)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VirtIOBlock *s = VIRTIO_BLK(dev);
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+    remove_migration_state_change_notifier(&s->migration_state_notifier);
     virtio_blk_data_plane_destroy(s->dataplane);
     s->dataplane = NULL;
 #endif
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index fc71853..b87cf49 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -125,6 +125,7 @@ typedef struct VirtIOBlock {
     unsigned short sector_mask;
     VMChangeStateEntry *change;
 #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
+    Notifier migration_state_notifier;
     struct VirtIOBlockDataPlane *dataplane;
 #endif
 } VirtIOBlock;
-- 
1.8.1.4

[Qemu-devel] [PATCH v3 4/4] dataplane: refuse to start if device is already in use

[Stefan Hajnoczi]

Dataplane must check whether a block device is in use before launching
the dataplane thread.  This is necessary since the thread does not
synchronize with the main loop and I/O requests could cause corruption.

One example is when a drive is added and a block job is started before
hotplugging the virtio-blk-pci adapter.  In this case we must not use
dataplane mode.

Cc: qemu-stable@nongnu.org
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/dataplane/virtio-blk.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 63c3ffa..411becc 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -415,6 +415,14 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *blk,
         return false;
     }
 
+    /* If dataplane is (re-)enabled while the guest is running there could be
+     * block jobs that can conflict.
+     */
+    if (bdrv_in_use(blk->conf.bs)) {
+        error_report("cannot start dataplane thread while device is in use");
+        return false;
+    }
+
     fd = raw_get_aio_fd(blk->conf.bs);
     if (fd < 0) {
         error_report("drive is incompatible with x-data-plane, "
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v3 0/4] dataplane: virtio-blk live migration with x-data-plane=on

[Kevin Wolf]

Am 29.07.2013 um 15:01 hat Stefan Hajnoczi geschrieben:
> v3:
>  * Fix spice migration, also needed for dataplane migration [kwolf]
> 
> v2:
>  * Rebase onto qemu.git/master
>  * Split bdrv_in_use() into separate patch, it's a bug fix [bonzini]
> 
> These patches add live migration support to -device virtio-blk-pci,x-data-plane=on.
> 
> Patches 1 & 2 make migration state notifiers reliable and fix spice migration.
> Dataplane migration faces the same issues so I'm including the patches here.
> 
> Patch 3 implements a switch from dataplane mode back to regular virtio-blk mode
> when migration starts.  This way live migration works.
> 
> Patch 4 is a bug fix: we need to check the device is not in use when creating
> the dataplane thread.  It applies to live migration but it turns out the same
> problem can be hit without migration too, see the commit description.
> 
> If migration is cancelled or the guest accesses the virtio-blk device after
> completion, dataplane starts again.
> 
> Since this approach is so small, it's more palatable for QEMU 1.6 than trying
> to make vring.c log dirty memory.  It makes dataplane usable in situations
> where live migration is a requirement.
> 
> Stefan Hajnoczi (4):
>   migration: notify migration state before starting thread
>   migration: fix spice migration
>   dataplane: enable virtio-blk x-data-plane=on live migration
>   dataplane: refuse to start if device is already in use
> 
>  hw/block/dataplane/virtio-blk.c | 17 ++++++++---------
>  hw/block/virtio-blk.c           | 32 ++++++++++++++++++++++++++++++++
>  include/hw/virtio/virtio-blk.h  |  1 +
>  include/migration/migration.h   |  2 +-
>  migration.c                     |  8 +++++---
>  ui/spice-core.c                 |  2 +-
>  6 files changed, 48 insertions(+), 14 deletions(-)

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH v3 2/4] migration: fix spice migration

[Michael R. Hines]

On 07/29/2013 09:01 AM, Stefan Hajnoczi wrote:
> Commit 29ae8a4133082e16970c9d4be09f4b6a15034617 ("rdma: introduce
> MIG_STATE_NONE and change MIG_STATE_SETUP state transition") changed the
> state transitions during migration setup.
>
> Spice used to be notified with MIG_STATE_ACTIVE and it detected this
> using migration_is_active().  Spice is now notified with
> MIG_STATE_SETUP and migration_is_active() no longer works.
>
> Replace migration_is_active() with migration_in_setup() to fix spice
> migration.
>
> Cc: Michael R. Hines <mrhines@us.ibm.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>   include/migration/migration.h | 2 +-
>   migration.c                   | 4 ++--
>   ui/spice-core.c               | 2 +-
>   3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 08c772d..140e6b4 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -90,7 +90,7 @@ int migrate_fd_close(MigrationState *s);
>
>   void add_migration_state_change_notifier(Notifier *notify);
>   void remove_migration_state_change_notifier(Notifier *notify);
> -bool migration_is_active(MigrationState *);
> +bool migration_in_setup(MigrationState *);
>   bool migration_has_finished(MigrationState *);
>   bool migration_has_failed(MigrationState *);
>   MigrationState *migrate_get_current(void);
> diff --git a/migration.c b/migration.c
> index a5ed26b..9fc7294 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -338,9 +338,9 @@ void remove_migration_state_change_notifier(Notifier *notify)
>       notifier_remove(notify);
>   }
>
> -bool migration_is_active(MigrationState *s)
> +bool migration_in_setup(MigrationState *s)
>   {
> -    return s->state == MIG_STATE_ACTIVE;
> +    return s->state == MIG_STATE_SETUP;
>   }
>
>   bool migration_has_finished(MigrationState *s)
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index f308fd9..033fd89 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -563,7 +563,7 @@ static void migration_state_notifier(Notifier *notifier, void *data)
>   {
>       MigrationState *s = data;
>
> -    if (migration_is_active(s)) {
> +    if (migration_in_setup(s)) {
>           spice_server_migrate_start(spice_server);
>       } else if (migration_has_finished(s)) {
>           spice_server_migrate_end(spice_server, true);

Do we really have to replace this function? Can we just add a new function?

- Michael

Re: [Qemu-devel] [PATCH v3 2/4] migration: fix spice migration

[Stefan Hajnoczi]

On Mon, Jul 29, 2013 at 10:46:50AM -0400, Michael R. Hines wrote:
> On 07/29/2013 09:01 AM, Stefan Hajnoczi wrote:
> >Commit 29ae8a4133082e16970c9d4be09f4b6a15034617 ("rdma: introduce
> >MIG_STATE_NONE and change MIG_STATE_SETUP state transition") changed the
> >state transitions during migration setup.
> >
> >Spice used to be notified with MIG_STATE_ACTIVE and it detected this
> >using migration_is_active().  Spice is now notified with
> >MIG_STATE_SETUP and migration_is_active() no longer works.
> >
> >Replace migration_is_active() with migration_in_setup() to fix spice
> >migration.
> >
> >Cc: Michael R. Hines <mrhines@us.ibm.com>
> >Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >---
> >  include/migration/migration.h | 2 +-
> >  migration.c                   | 4 ++--
> >  ui/spice-core.c               | 2 +-
> >  3 files changed, 4 insertions(+), 4 deletions(-)
> >
> >diff --git a/include/migration/migration.h b/include/migration/migration.h
> >index 08c772d..140e6b4 100644
> >--- a/include/migration/migration.h
> >+++ b/include/migration/migration.h
> >@@ -90,7 +90,7 @@ int migrate_fd_close(MigrationState *s);
> >
> >  void add_migration_state_change_notifier(Notifier *notify);
> >  void remove_migration_state_change_notifier(Notifier *notify);
> >-bool migration_is_active(MigrationState *);
> >+bool migration_in_setup(MigrationState *);
> >  bool migration_has_finished(MigrationState *);
> >  bool migration_has_failed(MigrationState *);
> >  MigrationState *migrate_get_current(void);
> >diff --git a/migration.c b/migration.c
> >index a5ed26b..9fc7294 100644
> >--- a/migration.c
> >+++ b/migration.c
> >@@ -338,9 +338,9 @@ void remove_migration_state_change_notifier(Notifier *notify)
> >      notifier_remove(notify);
> >  }
> >
> >-bool migration_is_active(MigrationState *s)
> >+bool migration_in_setup(MigrationState *s)
> >  {
> >-    return s->state == MIG_STATE_ACTIVE;
> >+    return s->state == MIG_STATE_SETUP;
> >  }
> >
> >  bool migration_has_finished(MigrationState *s)
> >diff --git a/ui/spice-core.c b/ui/spice-core.c
> >index f308fd9..033fd89 100644
> >--- a/ui/spice-core.c
> >+++ b/ui/spice-core.c
> >@@ -563,7 +563,7 @@ static void migration_state_notifier(Notifier *notifier, void *data)
> >  {
> >      MigrationState *s = data;
> >
> >-    if (migration_is_active(s)) {
> >+    if (migration_in_setup(s)) {
> >          spice_server_migrate_start(spice_server);
> >      } else if (migration_has_finished(s)) {
> >          spice_server_migrate_end(spice_server, true);
> 
> Do we really have to replace this function? Can we just add a new function?

Yes, migration_is_active() is deadcode since there are no callers.

There is also no way to receive a migration state change callback with
ACTIVE.  The notifiers are not called on the SETUP -> ACTIVE transition
because the migration thread does cmpxchg only (it doesn't invoke
notifiers).

If someone would like it again in the future they will first need to
decide how to notify when the migration thread transitions states.
Leaving this function around would be misleading since it cannot be used
correctly at the moment.

Stefan

Re: [Qemu-devel] [PATCH v3 0/4] dataplane: virtio-blk live migration with x-data-plane=on

[Stefan Hajnoczi]

On Mon, Jul 29, 2013 at 03:01:56PM +0200, Stefan Hajnoczi wrote:
> v3:
>  * Fix spice migration, also needed for dataplane migration [kwolf]
> 
> v2:
>  * Rebase onto qemu.git/master
>  * Split bdrv_in_use() into separate patch, it's a bug fix [bonzini]
> 
> These patches add live migration support to -device virtio-blk-pci,x-data-plane=on.
> 
> Patches 1 & 2 make migration state notifiers reliable and fix spice migration.
> Dataplane migration faces the same issues so I'm including the patches here.
> 
> Patch 3 implements a switch from dataplane mode back to regular virtio-blk mode
> when migration starts.  This way live migration works.
> 
> Patch 4 is a bug fix: we need to check the device is not in use when creating
> the dataplane thread.  It applies to live migration but it turns out the same
> problem can be hit without migration too, see the commit description.
> 
> If migration is cancelled or the guest accesses the virtio-blk device after
> completion, dataplane starts again.
> 
> Since this approach is so small, it's more palatable for QEMU 1.6 than trying
> to make vring.c log dirty memory.  It makes dataplane usable in situations
> where live migration is a requirement.
> 
> Stefan Hajnoczi (4):
>   migration: notify migration state before starting thread
>   migration: fix spice migration
>   dataplane: enable virtio-blk x-data-plane=on live migration
>   dataplane: refuse to start if device is already in use
> 
>  hw/block/dataplane/virtio-blk.c | 17 ++++++++---------
>  hw/block/virtio-blk.c           | 32 ++++++++++++++++++++++++++++++++
>  include/hw/virtio/virtio-blk.h  |  1 +
>  include/migration/migration.h   |  2 +-
>  migration.c                     |  8 +++++---
>  ui/spice-core.c                 |  2 +-
>  6 files changed, 48 insertions(+), 14 deletions(-)

Applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

Re: [Qemu-devel] [PATCH v3 2/4] migration: fix spice migration

[Michael R. Hines]

On 07/29/2013 11:01 AM, Stefan Hajnoczi wrote:
> On Mon, Jul 29, 2013 at 10:46:50AM -0400, Michael R. Hines wrote:
>> On 07/29/2013 09:01 AM, Stefan Hajnoczi wrote:
>>> Commit 29ae8a4133082e16970c9d4be09f4b6a15034617 ("rdma: introduce
>>> MIG_STATE_NONE and change MIG_STATE_SETUP state transition") changed the
>>> state transitions during migration setup.
>>>
>>> Spice used to be notified with MIG_STATE_ACTIVE and it detected this
>>> using migration_is_active().  Spice is now notified with
>>> MIG_STATE_SETUP and migration_is_active() no longer works.
>>>
>>> Replace migration_is_active() with migration_in_setup() to fix spice
>>> migration.
>>>
>>> Cc: Michael R. Hines <mrhines@us.ibm.com>
>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>>   include/migration/migration.h | 2 +-
>>>   migration.c                   | 4 ++--
>>>   ui/spice-core.c               | 2 +-
>>>   3 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/migration/migration.h b/include/migration/migration.h
>>> index 08c772d..140e6b4 100644
>>> --- a/include/migration/migration.h
>>> +++ b/include/migration/migration.h
>>> @@ -90,7 +90,7 @@ int migrate_fd_close(MigrationState *s);
>>>
>>>   void add_migration_state_change_notifier(Notifier *notify);
>>>   void remove_migration_state_change_notifier(Notifier *notify);
>>> -bool migration_is_active(MigrationState *);
>>> +bool migration_in_setup(MigrationState *);
>>>   bool migration_has_finished(MigrationState *);
>>>   bool migration_has_failed(MigrationState *);
>>>   MigrationState *migrate_get_current(void);
>>> diff --git a/migration.c b/migration.c
>>> index a5ed26b..9fc7294 100644
>>> --- a/migration.c
>>> +++ b/migration.c
>>> @@ -338,9 +338,9 @@ void remove_migration_state_change_notifier(Notifier *notify)
>>>       notifier_remove(notify);
>>>   }
>>>
>>> -bool migration_is_active(MigrationState *s)
>>> +bool migration_in_setup(MigrationState *s)
>>>   {
>>> -    return s->state == MIG_STATE_ACTIVE;
>>> +    return s->state == MIG_STATE_SETUP;
>>>   }
>>>
>>>   bool migration_has_finished(MigrationState *s)
>>> diff --git a/ui/spice-core.c b/ui/spice-core.c
>>> index f308fd9..033fd89 100644
>>> --- a/ui/spice-core.c
>>> +++ b/ui/spice-core.c
>>> @@ -563,7 +563,7 @@ static void migration_state_notifier(Notifier *notifier, void *data)
>>>   {
>>>       MigrationState *s = data;
>>>
>>> -    if (migration_is_active(s)) {
>>> +    if (migration_in_setup(s)) {
>>>           spice_server_migrate_start(spice_server);
>>>       } else if (migration_has_finished(s)) {
>>>           spice_server_migrate_end(spice_server, true);
>> Do we really have to replace this function? Can we just add a new function?
> Yes, migration_is_active() is deadcode since there are no callers.
>
> There is also no way to receive a migration state change callback with
> ACTIVE.  The notifiers are not called on the SETUP -> ACTIVE transition
> because the migration thread does cmpxchg only (it doesn't invoke
> notifiers).
>
> If someone would like it again in the future they will first need to
> decide how to notify when the migration thread transitions states.
> Leaving this function around would be misleading since it cannot be used
> correctly at the moment.
>
> Stefan
>
Acknowledged.

- Michael