block: Inactivate BDS when migration completes

So far, live migration with shared storage meant that the image is in a
not-really-ready don't-touch-me state on the destination while the
source is still actively using it, but after completing the migration,
the image was fully opened on both sides. This is bad.

This patch adds a block driver callback to inactivate images on the
source before completing the migration. Inactivation means that it goes
to a state as if it was just live migrated to the qemu instance on the
source (i.e. BDRV_O_INACTIVE is set). You're then supposed to continue
either on the source or on the destination, which takes ownership of the
image.

A typical migration looks like this now with respect to disk images:

1. Destination qemu is started, the image is opened with
   BDRV_O_INACTIVE. The image is fully opened on the source.

2. Migration is about to complete. The source flushes the image and
   inactivates it. Now both sides have the image opened with
   BDRV_O_INACTIVE and are expecting the other side to still modify it.

3. One side (the destination on success) continues and calls
   bdrv_invalidate_all() in order to take ownership of the image again.
   This removes BDRV_O_INACTIVE on the resuming side; the flag remains
   set on the other side.

This ensures that the same image isn't written to by both instances
(unless both are resumed, but then you get what you deserve). This is
important because .bdrv_close for non-BDRV_O_INACTIVE images could write
to the image file, which is definitely forbidden while another host is
using the image.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>

diff --git a/block.c b/block.c
index 95b2967192026d5543257633b11dcbf03d12ee05..5709d3ddb48d355f4057bcb9b69130f2777bb87c 100644 (file)
--- a/block.c
+++ b/block.c
@@ -3303,6 +3303,40 @@ void bdrv_invalidate_cache_all(Error **errp)
     }
 }
 
+static int bdrv_inactivate(BlockDriverState *bs)
+{
+    int ret;
+
+    if (bs->drv->bdrv_inactivate) {
+        ret = bs->drv->bdrv_inactivate(bs);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
+    bs->open_flags |= BDRV_O_INACTIVE;
+    return 0;
+}
+
+int bdrv_inactivate_all(void)
+{
+    BlockDriverState *bs;
+    int ret;
+
+    QTAILQ_FOREACH(bs, &bdrv_states, device_list) {
+        AioContext *aio_context = bdrv_get_aio_context(bs);
+
+        aio_context_acquire(aio_context);
+        ret = bdrv_inactivate(bs);
+        aio_context_release(aio_context);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
+    return 0;
+}
+
 /**************************************************************/
 /* removable device support */
 

diff --git a/include/block/block.h b/include/block/block.h
index 2b7d33c78eb4ba83aa8996e5323c682e1aeb98c7..25f36dcc74d83f579f20eacbfeae444c46efe26c 100644 (file)
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -369,6 +369,7 @@ BlockAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
 /* Invalidate any cached metadata used by image formats */
 void bdrv_invalidate_cache(BlockDriverState *bs, Error **errp);
 void bdrv_invalidate_cache_all(Error **errp);
+int bdrv_inactivate_all(void);
 
 /* Ensure contents are flushed to disk.  */
 int bdrv_flush(BlockDriverState *bs);

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 256609dd3d2d0edf38ddcc681e3a38d1ad94e461..428fa3397e4ebfd07b43f749cd653d0dd97a5ff2 100644 (file)
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -172,6 +172,7 @@ struct BlockDriver {
      * Invalidate any cached meta-data.
      */
     void (*bdrv_invalidate_cache)(BlockDriverState *bs, Error **errp);
+    int (*bdrv_inactivate)(BlockDriverState *bs);
 
     /*
      * Flushes all data that was already written to the OS all the way down to

diff --git a/migration/migration.c b/migration/migration.c
index bc611e453bd54405989a9f438f0568ab3e07b7ff..aaca451cf812681d2abe80c64de447fc08889c99 100644 (file)
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1422,7 +1422,11 @@ static int postcopy_start(MigrationState *ms, bool *old_vm_running)
     *old_vm_running = runstate_is_running();
     global_state_store();
     ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+    if (ret < 0) {
+        goto fail;
+    }
 
+    ret = bdrv_inactivate_all();
     if (ret < 0) {
         goto fail;
     }
@@ -1541,6 +1545,9 @@ static void migration_completion(MigrationState *s, int current_active_state,
 
         if (!ret) {
             ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE);
+            if (ret >= 0) {
+                ret = bdrv_inactivate_all();
+            }
             if (ret >= 0) {
                 qemu_file_set_rate_limit(s->file, INT64_MAX);
                 qemu_savevm_state_complete_precopy(s->file, false);

diff --git a/qmp.c b/qmp.c
index 3ff6db79b9b78911b0c39930c50293caa3f1cf92..53affe2cfd468ea93c1e3b5e6df6744d44db71d7 100644 (file)
--- a/qmp.c
+++ b/qmp.c
@@ -192,6 +192,18 @@ void qmp_cont(Error **errp)
         }
     }
 
+    /* Continuing after completed migration. Images have been inactivated to
+     * allow the destination to take control. Need to get control back now. */
+    if (runstate_check(RUN_STATE_FINISH_MIGRATE) ||
+        runstate_check(RUN_STATE_POSTMIGRATE))
+    {
+        bdrv_invalidate_cache_all(&local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;
+        }
+    }
+
     if (runstate_check(RUN_STATE_INMIGRATE)) {
         autostart = 1;
     } else {