[Qemu-devel] [PATCH V3 0/7] Make QED with live migration safe

[Qemu-devel] [PATCH V3 0/7] Make QED with live migration safe

[Benoît Canet]

This is the third version of a patchset aiming at making the combined
usage of QED and live migration safe.

v3:

-qed: Drop the flags qed structure member and use bs->open_flags to reopen (stefana)
-qed: When opening honor flags parameter instead of bs->open_flags

v2:

-The block layer is not aware anymore of the migration state. (stefanha)
-No bdrv_invalidate_cache renaming since the semantic do not change. (stefanha)
-The qed bdrv_invalidate_cache function does a reopening of the image to flush
 metadata and to do the image integrity check. (stefanha)

Benoît Canet (7):
  block: Add new BDRV_O_INCOMING flag to notice incoming live migration
  block: add a function to clear incoming live migration flags
  blockdev: open images with BDRV_O_INCOMING on incoming live migration
  qed: add bdrv_invalidate_cache to be called after incoming live
    migration
  migration: clear BDRV_O_INCOMING flags on end of incoming live
    migration
  qed: honor BDRV_O_INCOMING for incoming live migration
  qed: remove incoming live migration blocker

 block.c     |    9 +++++++++
 block.h     |    3 +++
 block/qed.c |   24 +++++++++++++-----------
 block/qed.h |    2 --
 blockdev.c  |    4 ++++
 migration.c |    1 +
 6 files changed, 30 insertions(+), 13 deletions(-)

-- 
1.7.7.6

[Qemu-devel] [PATCH V3 1/7] block: Add new BDRV_O_INCOMING flag to notice incoming live migration

[Benoît Canet]

>From original patch with Patchwork-id: 31110 by
Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

"Add a flag to indicate that incoming migration is pending and care needs
to be taken for data consistency.  Block drivers should not modify the
image file before incoming migration is complete since the migration
source host is still using the image file."

The rationale for not using bdrv->read_only is the following.

"Unfortunately this is not possible because too many other places in QEMU
test bdrv_is_read_only() and use it for their own evil purposes.  For
example, ide_init_drive() will error out because read-only harddisks are
not supported.  We're mixing guest and host side read-only concepts so
this simpler alternative does not work."

Signed-off-by: Benoit Canet <benoit.canet@gmail.com>
---
 block.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/block.h b/block.h
index 415bb17..b3b18d6 100644
--- a/block.h
+++ b/block.h
@@ -71,6 +71,7 @@ typedef struct BlockDevOps {
 #define BDRV_O_NO_BACKING  0x0100 /* don't open the backing file */
 #define BDRV_O_NO_FLUSH    0x0200 /* disable flushing on this disk */
 #define BDRV_O_COPY_ON_READ 0x0400 /* copy read backing sectors into image */
+#define BDRV_O_INCOMING    0x0800  /* consistency hint for incoming migration */
 
 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
 
-- 
1.7.7.6

[Qemu-devel] [PATCH V3 2/7] block: add a function to clear incoming live migration flags

[Benoît Canet]

This function will clear all BDRV_O_INCOMING flags.

Signed-off-by: Benoit Canet <benoit.canet@gmail.com>
---
 block.c |    9 +++++++++
 block.h |    2 ++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index b88ee90..45085e7 100644
--- a/block.c
+++ b/block.c
@@ -3584,6 +3584,15 @@ void bdrv_invalidate_cache_all(void)
     }
 }
 
+void bdrv_clear_incoming_migration_all(void)
+{
+    BlockDriverState *bs;
+
+    QTAILQ_FOREACH(bs, &bdrv_states, list) {
+        bs->open_flags = bs->open_flags & ~(BDRV_O_INCOMING);
+    }
+}
+
 int bdrv_flush(BlockDriverState *bs)
 {
     Coroutine *co;
diff --git a/block.h b/block.h
index b3b18d6..951b476 100644
--- a/block.h
+++ b/block.h
@@ -223,6 +223,8 @@ BlockDriverAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
 void bdrv_invalidate_cache(BlockDriverState *bs);
 void bdrv_invalidate_cache_all(void);
 
+void bdrv_clear_incoming_migration_all(void);
+
 /* Ensure contents are flushed to disk.  */
 int bdrv_flush(BlockDriverState *bs);
 int coroutine_fn bdrv_co_flush(BlockDriverState *bs);
-- 
1.7.7.6

[Qemu-devel] [PATCH V3 3/7] blockdev: open images with BDRV_O_INCOMING on incoming live migration

[Benoît Canet]

Open images with BDRV_O_INCOMING in order to inform block drivers
that an incoming live migration is coming.

Signed-off-by: Benoit Canet <benoit.canet@gmail.com>
---
 blockdev.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 1a500b8..9b57133 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -591,6 +591,10 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi)
         bdrv_flags |= BDRV_O_COPY_ON_READ;
     }
 
+    if (runstate_check(RUN_STATE_INMIGRATE)) {
+        bdrv_flags |= BDRV_O_INCOMING;
+    }
+
     if (media == MEDIA_CDROM) {
         /* CDROM is fine for any interface, don't check.  */
         ro = 1;
-- 
1.7.7.6

[Qemu-devel] [PATCH V3 4/7] qed: add bdrv_invalidate_cache to be called after incoming live migration

[Benoît Canet]

The QED image is reopened to flush metadata and check consistency.

Signed-off-by: Benoit Canet <benoit.canet@gmail.com>
---
 block/qed.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index a041d31..e32f598 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1516,6 +1516,15 @@ static int bdrv_qed_change_backing_file(BlockDriverState *bs,
     return ret;
 }
 
+static void bdrv_qed_invalidate_cache(BlockDriverState *bs)
+{
+    BDRVQEDState *s = bs->opaque;
+
+    bdrv_qed_close(bs);
+    memset(s, 0, sizeof(BDRVQEDState));
+    bdrv_qed_open(bs, bs->open_flags);
+}
+
 static int bdrv_qed_check(BlockDriverState *bs, BdrvCheckResult *result)
 {
     BDRVQEDState *s = bs->opaque;
@@ -1568,6 +1577,7 @@ static BlockDriver bdrv_qed = {
     .bdrv_getlength           = bdrv_qed_getlength,
     .bdrv_get_info            = bdrv_qed_get_info,
     .bdrv_change_backing_file = bdrv_qed_change_backing_file,
+    .bdrv_invalidate_cache    = bdrv_qed_invalidate_cache,
     .bdrv_check               = bdrv_qed_check,
 };
 
-- 
1.7.7.6

[Qemu-devel] [PATCH V3 5/7] migration: clear BDRV_O_INCOMING flags on end of incoming live migration

[Benoît Canet]

Signed-off-by: Benoît Canet <benoit.canet@gmail.com>
---
 migration.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/migration.c b/migration.c
index 8c119ba..94f7839 100644
--- a/migration.c
+++ b/migration.c
@@ -91,6 +91,7 @@ void process_incoming_migration(QEMUFile *f)
     qemu_announce_self();
     DPRINTF("successfully loaded vm state\n");
 
+    bdrv_clear_incoming_migration_all();
     /* Make sure all file formats flush their mutable metadata */
     bdrv_invalidate_cache_all();
 
-- 
1.7.7.6

[Qemu-devel] [PATCH V3 6/7] qed: honor BDRV_O_INCOMING for incoming live migration

[Benoît Canet]

>From original commit with Patchwork-id: 31108 by
Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

"The QED image format includes a file header bit to mark images dirty.
QED normally checks dirty images on open and fixes inconsistent
metadata.  This is undesirable during live migration since the dirty bit
may be set if the source host is modifying the image file.  The check
should be postponed until migration completes.

Skip operations that modify the image file if the BDRV_O_INCOMING flag
is set."

Signed-off-by: Benoit Canet <benoit.canet@gmail.com>
---
 block/qed.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index e32f598..4a53db6 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -450,7 +450,7 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags)
      * feature is no longer valid.
      */
     if ((s->header.autoclear_features & ~QED_AUTOCLEAR_FEATURE_MASK) != 0 &&
-        !bdrv_is_read_only(bs->file)) {
+        !bdrv_is_read_only(bs->file) && !(flags & BDRV_O_INCOMING)) {
         s->header.autoclear_features &= QED_AUTOCLEAR_FEATURE_MASK;
 
         ret = qed_write_header_sync(s);
@@ -477,7 +477,8 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags)
          * potentially inconsistent images to be opened read-only.  This can
          * aid data recovery from an otherwise inconsistent image.
          */
-        if (!bdrv_is_read_only(bs->file)) {
+        if (!bdrv_is_read_only(bs->file) &&
+            !(flags & BDRV_O_INCOMING)) {
             BdrvCheckResult result = {0};
 
             ret = qed_check(s, &result, true);
-- 
1.7.7.6

[Qemu-devel] [PATCH V3 7/7] qed: remove incoming live migration blocker

[Benoît Canet]

Signed-off-by: Benoit Canet <benoit.canet@gmail.com>
---
 block/qed.c |    9 ---------
 block/qed.h |    2 --
 2 files changed, 0 insertions(+), 11 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index 4a53db6..74ea278 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -498,12 +498,6 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags)
     s->need_check_timer = qemu_new_timer_ns(vm_clock,
                                             qed_need_check_timer_cb, s);
 
-    error_set(&s->migration_blocker,
-              QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-              "qed", bs->device_name, "live migration");
-    migrate_add_blocker(s->migration_blocker);
-
-
 out:
     if (ret) {
         qed_free_l2_cache(&s->l2_cache);
@@ -516,9 +510,6 @@ static void bdrv_qed_close(BlockDriverState *bs)
 {
     BDRVQEDState *s = bs->opaque;
 
-    migrate_del_blocker(s->migration_blocker);
-    error_free(s->migration_blocker);
-
     qed_cancel_need_check_timer(s);
     qemu_free_timer(s->need_check_timer);
 
diff --git a/block/qed.h b/block/qed.h
index 62624a1..c716772 100644
--- a/block/qed.h
+++ b/block/qed.h
@@ -169,8 +169,6 @@ typedef struct {
 
     /* Periodic flush and clear need check flag */
     QEMUTimer *need_check_timer;
-
-    Error *migration_blocker;
 } BDRVQEDState;
 
 enum {
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH V3 0/7] Make QED with live migration safe

[Stefan Hajnoczi]

On Fri, Mar 23, 2012 at 7:36 AM, Benoît Canet <benoit.canet@gmail.com> wrote:
> This is the third version of a patchset aiming at making the combined
> usage of QED and live migration safe.
>
> v3:
>
> -qed: Drop the flags qed structure member and use bs->open_flags to reopen (stefana)
> -qed: When opening honor flags parameter instead of bs->open_flags
>
> v2:
>
> -The block layer is not aware anymore of the migration state. (stefanha)
> -No bdrv_invalidate_cache renaming since the semantic do not change. (stefanha)
> -The qed bdrv_invalidate_cache function does a reopening of the image to flush
>  metadata and to do the image integrity check. (stefanha)
>
> Benoît Canet (7):
>  block: Add new BDRV_O_INCOMING flag to notice incoming live migration
>  block: add a function to clear incoming live migration flags
>  blockdev: open images with BDRV_O_INCOMING on incoming live migration
>  qed: add bdrv_invalidate_cache to be called after incoming live
>    migration
>  migration: clear BDRV_O_INCOMING flags on end of incoming live
>    migration
>  qed: honor BDRV_O_INCOMING for incoming live migration
>  qed: remove incoming live migration blocker
>
>  block.c     |    9 +++++++++
>  block.h     |    3 +++
>  block/qed.c |   24 +++++++++++++-----------
>  block/qed.h |    2 --
>  blockdev.c  |    4 ++++
>  migration.c |    1 +
>  6 files changed, 30 insertions(+), 13 deletions(-)

I tested qed live migration as well as confirming that bs->open_flags
BDRV_O_INCOMING works correctly in gdb.

Reviewed-by: Stefan Hajnoczi <stefanha@gmail.com>

Re: [Qemu-devel] [PATCH V3 0/7] Make QED with live migration safe

[Benoît Canet]

[-- Attachment #1: Type: text/plain, Size: 248 bytes --]

>
>
>
> I tested qed live migration as well as confirming that bs->open_flags
> BDRV_O_INCOMING works correctly in gdb.
>
> Reviewed-by: Stefan Hajnoczi <stefanha@gmail.com>
>

ping.

Kevin Wolf: will it be merged in your next for-anthony branch ?

[-- Attachment #2: Type: text/html, Size: 872 bytes --]

Re: [Qemu-devel] [PATCH V3 0/7] Make QED with live migration safe

[Kevin Wolf]

Am 23.03.2012 08:36, schrieb Benoît Canet:
> This is the third version of a patchset aiming at making the combined
> usage of QED and live migration safe.
> 
> v3:
> 
> -qed: Drop the flags qed structure member and use bs->open_flags to reopen (stefana)
> -qed: When opening honor flags parameter instead of bs->open_flags
> 
> v2:
> 
> -The block layer is not aware anymore of the migration state. (stefanha)
> -No bdrv_invalidate_cache renaming since the semantic do not change. (stefanha)
> -The qed bdrv_invalidate_cache function does a reopening of the image to flush
>  metadata and to do the image integrity check. (stefanha)
> 
> Benoît Canet (7):
>   block: Add new BDRV_O_INCOMING flag to notice incoming live migration
>   block: add a function to clear incoming live migration flags
>   blockdev: open images with BDRV_O_INCOMING on incoming live migration
>   qed: add bdrv_invalidate_cache to be called after incoming live
>     migration
>   migration: clear BDRV_O_INCOMING flags on end of incoming live
>     migration
>   qed: honor BDRV_O_INCOMING for incoming live migration
>   qed: remove incoming live migration blocker
> 
>  block.c     |    9 +++++++++
>  block.h     |    3 +++
>  block/qed.c |   24 +++++++++++++-----------
>  block/qed.h |    2 --
>  blockdev.c  |    4 ++++
>  migration.c |    1 +
>  6 files changed, 30 insertions(+), 13 deletions(-)

Thanks, applied to the block branch.

Kevin