[Qemu-devel] [RFC PATCH 00/10] make qed and live migration usage safe

[Qemu-devel] [RFC PATCH 00/10] make qed and live migration usage safe

[Benoît Canet]

QED + live migration is an unsafe and disabled mix.

This patchset make qed and live migration safe to use.

The check of QED images is delayed during the incoming migration.
After the migration complete the QED images are checked.

Benoît Canet (10):
  block: Add new BDRV_O_INCOMING flag to notice incoming live migration
  block: add a function to set incoming live migration
  block: add a function to clear incoming live migration
  block: rename *_invalidate_cache_* to *_post_incoming_migration_*
  migration: inform the block layer of incoming live status
  block: open images with BDRV_O_INCOMING on incoming live migration
  qed: extract image checking into check_image_if_needed
  qed: add bdrv_post_incoming_migration operation checking the image
  qed: honor BDRV_O_INCOMING for incoming live migration
  qed: remove incoming live migration blocker

 block.c       |   35 +++++++++++++++++++++---
 block.h       |   13 +++++++--
 block/qcow2.c |    7 ++++-
 block/qed.c   |   81 +++++++++++++++++++++++++++++++++++----------------------
 block/qed.h   |    2 -
 block_int.h   |    4 +-
 migration.c   |    9 +++++-
 vl.c          |    5 +++
 8 files changed, 110 insertions(+), 46 deletions(-)

-- 
1.7.7.6

[Qemu-devel] [RFC PATCH 01/10] 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 48d0bf3..e320d8f 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] [RFC PATCH 02/10] block: add a function to set incoming live migration

[Benoît Canet]

This function will help to inform the block layer that an incoming
live migration is coming in order to make proper usage of the
BDRV_O_INCOMING flag.

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

diff --git a/block.c b/block.c
index 52ffe14..3e2260d 100644
--- a/block.c
+++ b/block.c
@@ -100,6 +100,8 @@ static BlockDriverState *bs_snapshots;
 /* If non-zero, use only whitelisted block drivers */
 static int use_bdrv_whitelist;
 
+static bool incoming_migration;
+
 #ifdef _WIN32
 static int is_windows_drive_prefix(const char *filename)
 {
@@ -3580,6 +3582,12 @@ void bdrv_invalidate_cache_all(void)
     }
 }
 
+void bdrv_set_incoming_migration(void)
+{
+    assert(incoming_migration == false);
+    incoming_migration = true;
+}
+
 int bdrv_flush(BlockDriverState *bs)
 {
     Coroutine *co;
diff --git a/block.h b/block.h
index e320d8f..23af1cc 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_set_incoming_migration(void);
+
 /* Ensure contents are flushed to disk.  */
 int bdrv_flush(BlockDriverState *bs);
 int coroutine_fn bdrv_co_flush(BlockDriverState *bs);
-- 
1.7.7.6

Re: [Qemu-devel] [RFC PATCH 02/10] block: add a function to set incoming live migration

[Stefan Hajnoczi]

On Tue, Mar 06, 2012 at 06:32:21PM +0100, Benoît Canet wrote:
> This function will help to inform the block layer that an incoming
> live migration is coming in order to make proper usage of the
> BDRV_O_INCOMING flag.
> 
> Signed-off-by: Benoit Canet <benoit.canet@gmail.com>
> ---
>  block.c |    8 ++++++++
>  block.h |    2 ++
>  2 files changed, 10 insertions(+), 0 deletions(-)

The block layer doesn't need to keep track of incoming migrations:

blockdev.c:drive_init() or vl.c:drive_init_func() could use
runstate_check(RUN_STATE_INMIGRATE) to check whether incoming migration
is in progress.  If so, they would pass in BDRV_O_INCOMING during image
creation.  As a result we don't need to introduce this global variable
or functions to manipulate it in the block layer.

The block layer post migration function will still need to clear
BDRV_O_INCOMING from open_flags.

Stefan

[Qemu-devel] [RFC PATCH 03/10] block: add a function to clear incoming live migration

[Benoît Canet]

This function is to be called to inform the block layer that an
incoming live migration has been canceled or has just finished.
It will also clear all BDRV_O_INCOMING flags.

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

diff --git a/block.c b/block.c
index 3e2260d..889d464 100644
--- a/block.c
+++ b/block.c
@@ -3588,6 +3588,19 @@ void bdrv_set_incoming_migration(void)
     incoming_migration = true;
 }
 
+void bdrv_clear_incoming_migration_all(void)
+{
+    BlockDriverState *bs;
+
+    assert(incoming_migration == true);
+
+    incoming_migration = false;
+
+    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 23af1cc..2ddf9c5 100644
--- a/block.h
+++ b/block.h
@@ -224,6 +224,7 @@ void bdrv_invalidate_cache(BlockDriverState *bs);
 void bdrv_invalidate_cache_all(void);
 
 void bdrv_set_incoming_migration(void);
+void bdrv_clear_incoming_migration_all(void);
 
 /* Ensure contents are flushed to disk.  */
 int bdrv_flush(BlockDriverState *bs);
-- 
1.7.7.6

[Qemu-devel] [RFC PATCH 04/10] block: rename *_invalidate_cache_* to *_post_incoming_migration_*

[Benoît Canet]

This patch will allow to do a post incoming live migration check
on QED images and keep invalidating caches on qcow2 images.

Signed-off-by: Benoit Canet <benoit.canet@gmail.com>
---
 block.c       |   10 +++++-----
 block.h       |    8 +++++---
 block/qcow2.c |    7 ++++++-
 block_int.h   |    4 ++--
 migration.c   |    6 ++++--
 5 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/block.c b/block.c
index 889d464..b0f0288 100644
--- a/block.c
+++ b/block.c
@@ -3566,19 +3566,19 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
     }
 }
 
-void bdrv_invalidate_cache(BlockDriverState *bs)
+void bdrv_post_incoming_migration(BlockDriverState *bs)
 {
-    if (bs->drv && bs->drv->bdrv_invalidate_cache) {
-        bs->drv->bdrv_invalidate_cache(bs);
+    if (bs->drv && bs->drv->bdrv_post_incoming_migration) {
+        bs->drv->bdrv_post_incoming_migration(bs);
     }
 }
 
-void bdrv_invalidate_cache_all(void)
+void bdrv_post_incoming_migration_all(void)
 {
     BlockDriverState *bs;
 
     QTAILQ_FOREACH(bs, &bdrv_states, list) {
-        bdrv_invalidate_cache(bs);
+        bdrv_post_incoming_migration(bs);
     }
 }
 
diff --git a/block.h b/block.h
index 2ddf9c5..1323086 100644
--- a/block.h
+++ b/block.h
@@ -219,9 +219,11 @@ BlockDriverAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
         unsigned long int req, void *buf,
         BlockDriverCompletionFunc *cb, void *opaque);
 
-/* Invalidate any cached metadata used by image formats */
-void bdrv_invalidate_cache(BlockDriverState *bs);
-void bdrv_invalidate_cache_all(void);
+/* Do post incoming migration operations
+ * Ex: flush mutable metadata
+ */
+void bdrv_post_incoming_migration(BlockDriverState *bs);
+void bdrv_post_incoming_migration_all(void);
 
 void bdrv_set_incoming_migration(void);
 void bdrv_clear_incoming_migration_all(void);
diff --git a/block/qcow2.c b/block/qcow2.c
index eb5ea48..bbe6032 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -700,6 +700,11 @@ static void qcow2_invalidate_cache(BlockDriverState *bs)
     }
 }
 
+static void qcow2_post_incoming_migration(BlockDriverState *bs)
+{
+    qcow2_invalidate_cache(bs);
+}
+
 static size_t header_ext_add(char *buf, uint32_t magic, const void *s,
     size_t len, size_t buflen)
 {
@@ -1383,7 +1388,7 @@ static BlockDriver bdrv_qcow2 = {
 
     .bdrv_change_backing_file   = qcow2_change_backing_file,
 
-    .bdrv_invalidate_cache      = qcow2_invalidate_cache,
+    .bdrv_post_incoming_migration = qcow2_post_incoming_migration,
 
     .create_options = qcow2_create_options,
     .bdrv_check = qcow2_check,
diff --git a/block_int.h b/block_int.h
index b460c36..e68d42e 100644
--- a/block_int.h
+++ b/block_int.h
@@ -145,9 +145,9 @@ struct BlockDriver {
         int64_t sector_num, int nb_sectors, int *pnum);
 
     /*
-     * Invalidate any cached meta-data.
+     * Do the post incoming migration processing
      */
-    void (*bdrv_invalidate_cache)(BlockDriverState *bs);
+    void (*bdrv_post_incoming_migration)(BlockDriverState *bs);
 
     /*
      * Flushes all data that was already written to the OS all the way down to
diff --git a/migration.c b/migration.c
index 00fa1e3..45af364 100644
--- a/migration.c
+++ b/migration.c
@@ -91,8 +91,10 @@ void process_incoming_migration(QEMUFile *f)
     qemu_announce_self();
     DPRINTF("successfully loaded vm state\n");
 
-    /* Make sure all file formats flush their mutable metadata */
-    bdrv_invalidate_cache_all();
+    /* Execute post incoming migration operations :
+     * for qcow2 make sure to flush mutable metadata
+     **/
+    bdrv_post_incoming_migration_all();
 
     if (autostart) {
         vm_start();
-- 
1.7.7.6

[Qemu-devel] [RFC PATCH 05/10] migration: inform the block layer of incoming live status

[Benoît Canet]

Signed-off-by: Benoit Canet <benoit.canet@gmail.com>
---
 migration.c |    2 ++
 vl.c        |    5 +++++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/migration.c b/migration.c
index 45af364..5ac31a3 100644
--- a/migration.c
+++ b/migration.c
@@ -76,6 +76,7 @@ int qemu_start_incoming_migration(const char *uri)
         ret = fd_start_incoming_migration(p);
 #endif
     else {
+        bdrv_clear_incoming_migration_all();
         fprintf(stderr, "unknown migration protocol: %s\n", uri);
         ret = -EPROTONOSUPPORT;
     }
@@ -91,6 +92,7 @@ void process_incoming_migration(QEMUFile *f)
     qemu_announce_self();
     DPRINTF("successfully loaded vm state\n");
 
+    bdrv_clear_incoming_migration_all();
     /* Execute post incoming migration operations :
      * for qcow2 make sure to flush mutable metadata
      **/
diff --git a/vl.c b/vl.c
index 97ab2b9..b1d3dd2 100644
--- a/vl.c
+++ b/vl.c
@@ -3389,6 +3389,11 @@ int main(int argc, char **argv, char **envp)
 
     blk_mig_init();
 
+    /* make the block layer ready for incoming live migration */
+    if (incoming) {
+        bdrv_set_incoming_migration();
+    }
+
     /* open the virtual block devices */
     if (snapshot)
         qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot, NULL, 0);
-- 
1.7.7.6

[Qemu-devel] [RFC PATCH 06/10] block: 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>
---
 block.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index b0f0288..78287df 100644
--- a/block.c
+++ b/block.c
@@ -575,6 +575,10 @@ static int bdrv_open_common(BlockDriverState *bs, const char *filename,
 
     trace_bdrv_open_common(bs, filename, flags, drv->format_name);
 
+    if (incoming_migration) {
+        flags |= BDRV_O_INCOMING;
+    }
+
     bs->file = NULL;
     bs->total_sectors = 0;
     bs->encrypted = 0;
-- 
1.7.7.6

[Qemu-devel] [RFC PATCH 07/10] qed: extract image checking into check_image_if_needed

[Benoît Canet]

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

diff --git a/block/qed.c b/block/qed.c
index a041d31..25558fc 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -367,6 +367,37 @@ static void qed_cancel_need_check_timer(BDRVQEDState *s)
     qemu_del_timer(s->need_check_timer);
 }
 
+static int check_image_if_needed(BlockDriverState *bs)
+{
+    int ret;
+    BDRVQEDState *s = bs->opaque;
+
+    if (s->header.features & QED_F_NEED_CHECK) {
+        /* Read-only images cannot be fixed.  There is no risk of corruption
+         * since write operations are not possible.  Therefore, allow
+         * 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)) {
+            BdrvCheckResult result = {0};
+
+            ret = qed_check(s, &result, true);
+            if (ret) {
+                return ret;
+            }
+            if (!result.corruptions && !result.check_errors) {
+                /* Ensure fixes reach storage before clearing check bit */
+                bdrv_flush(s->bs);
+
+                s->header.features &= ~QED_F_NEED_CHECK;
+                qed_write_header_sync(s);
+            }
+        }
+    }
+
+    return 0;
+}
+
 static int bdrv_qed_open(BlockDriverState *bs, int flags)
 {
     BDRVQEDState *s = bs->opaque;
@@ -471,27 +502,9 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags)
     }
 
     /* If image was not closed cleanly, check consistency */
-    if (s->header.features & QED_F_NEED_CHECK) {
-        /* Read-only images cannot be fixed.  There is no risk of corruption
-         * since write operations are not possible.  Therefore, allow
-         * 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)) {
-            BdrvCheckResult result = {0};
-
-            ret = qed_check(s, &result, true);
-            if (ret) {
-                goto out;
-            }
-            if (!result.corruptions && !result.check_errors) {
-                /* Ensure fixes reach storage before clearing check bit */
-                bdrv_flush(s->bs);
-
-                s->header.features &= ~QED_F_NEED_CHECK;
-                qed_write_header_sync(s);
-            }
-        }
+    ret = check_image_if_needed(bs);
+    if (ret) {
+        goto out;
     }
 
     s->need_check_timer = qemu_new_timer_ns(vm_clock,
-- 
1.7.7.6

[Qemu-devel] [RFC PATCH 08/10] qed: add bdrv_post_incoming_migration operation checking the image

[Benoît Canet]

The bs is closed on verification failure to avoid further corruptions.

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

diff --git a/block.h b/block.h
index 1323086..25b66f0 100644
--- a/block.h
+++ b/block.h
@@ -221,6 +221,7 @@ BlockDriverAIOCB *bdrv_aio_ioctl(BlockDriverState *bs,
 
 /* Do post incoming migration operations
  * Ex: flush mutable metadata
+ * Ex: check image
  */
 void bdrv_post_incoming_migration(BlockDriverState *bs);
 void bdrv_post_incoming_migration_all(void);
diff --git a/block/qed.c b/block/qed.c
index 25558fc..f6300af 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1529,6 +1529,19 @@ static int bdrv_qed_change_backing_file(BlockDriverState *bs,
     return ret;
 }
 
+static void bdrv_qed_check_if_needed(BlockDriverState *bs)
+{
+    /* close the block device if the verification fail */
+    if (check_image_if_needed(bs)) {
+        bdrv_close(bs);
+    }
+}
+
+static void bdrv_qed_post_incoming_migration(BlockDriverState *bs)
+{
+    bdrv_qed_check_if_needed(bs);
+}
+
 static int bdrv_qed_check(BlockDriverState *bs, BdrvCheckResult *result)
 {
     BDRVQEDState *s = bs->opaque;
@@ -1581,6 +1594,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_post_incoming_migration = bdrv_qed_post_incoming_migration,
     .bdrv_check               = bdrv_qed_check,
 };
 
diff --git a/migration.c b/migration.c
index 5ac31a3..d59042c 100644
--- a/migration.c
+++ b/migration.c
@@ -95,6 +95,7 @@ void process_incoming_migration(QEMUFile *f)
     bdrv_clear_incoming_migration_all();
     /* Execute post incoming migration operations :
      * for qcow2 make sure to flush mutable metadata
+     * for QED check image
      **/
     bdrv_post_incoming_migration_all();
 
-- 
1.7.7.6

Re: [Qemu-devel] [RFC PATCH 08/10] qed: add bdrv_post_incoming_migration operation checking the image

[Stefan Hajnoczi]

On Tue, Mar 06, 2012 at 06:32:27PM +0100, Benoît Canet wrote:
> @@ -1529,6 +1529,19 @@ static int bdrv_qed_change_backing_file(BlockDriverState *bs,
>      return ret;
>  }
>  
> +static void bdrv_qed_check_if_needed(BlockDriverState *bs)
> +{
> +    /* close the block device if the verification fail */
> +    if (check_image_if_needed(bs)) {
> +        bdrv_close(bs);
> +    }

We open the image for incoming migration while the VM is still running.
That means the metadata and header can still change before migration
switchover.  So we need to drop everything we know about this image and
start from scratch.

qcow2 does it like this:

qcow2_close(bs);
memset(s, 0, sizeof(BDRVQcowState));
qcow2_open(bs, flags);

We should do something similar so that the QED header is re-read.  This
probably means check_image_if_needed() doesn't need to be factored out
of qed_open().

(Note that the underlying file and BlockDriverState don't get reopened,
we're simply reinitializing the QED/QCOW2 image format layer here.)

Stefan

Re: [Qemu-devel] [RFC PATCH 08/10] qed: add bdrv_post_incoming_migration operation checking the image

[Benoît Canet]

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

So should I drop the "block: rename *_invalidate_cache_* to
*_post_incoming_migration_*" commit ?

On Tue, Mar 20, 2012 at 4:42 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Tue, Mar 06, 2012 at 06:32:27PM +0100, Benoît Canet wrote:
> > @@ -1529,6 +1529,19 @@ static int
> bdrv_qed_change_backing_file(BlockDriverState *bs,
> >      return ret;
> >  }
> >
> > +static void bdrv_qed_check_if_needed(BlockDriverState *bs)
> > +{
> > +    /* close the block device if the verification fail */
> > +    if (check_image_if_needed(bs)) {
> > +        bdrv_close(bs);
> > +    }
>
> We open the image for incoming migration while the VM is still running.
> That means the metadata and header can still change before migration
> switchover.  So we need to drop everything we know about this image and
> start from scratch.
>
> qcow2 does it like this:
>
> qcow2_close(bs);
> memset(s, 0, sizeof(BDRVQcowState));
> qcow2_open(bs, flags);
>
> We should do something similar so that the QED header is re-read.  This
> probably means check_image_if_needed() doesn't need to be factored out
> of qed_open().
>
> (Note that the underlying file and BlockDriverState don't get reopened,
> we're simply reinitializing the QED/QCOW2 image format layer here.)
>
> Stefan
>

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

Re: [Qemu-devel] [RFC PATCH 08/10] qed: add bdrv_post_incoming_migration operation checking the image

[Stefan Hajnoczi]

2012/3/20 Benoît Canet <benoit.canet@gmail.com>:
>
> So should I drop the "block: rename *_invalidate_cache_* to
> *_post_incoming_migration_*" commit ?

Yes, I think there is no semantic change so keeping the name unchanged is good.

Stefan

[Qemu-devel] [RFC PATCH 09/10] qed: honor BDRV_O_INCOMING for incoming live migration

[Benoît Canet]

>From original commit is 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 f6300af..4bfda24 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -378,7 +378,8 @@ static int check_image_if_needed(BlockDriverState *bs)
          * 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) &&
+            !(bs->open_flags & BDRV_O_INCOMING)) {
             BdrvCheckResult result = {0};
 
             ret = qed_check(s, &result, true);
@@ -481,7 +482,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);
-- 
1.7.7.6

[Qemu-devel] [RFC PATCH 10/10] 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 4bfda24..a7964f0 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -511,12 +511,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);
@@ -529,9 +523,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] [RFC PATCH 00/10] make qed and live migration usage safe

[Benoît Canet]

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

I'm resending you this one in private because your emails where missing in
the first posting.

Regards

Benoît

On Tue, Mar 6, 2012 at 6:32 PM, Benoît Canet <benoit.canet@gmail.com> wrote:

> QED + live migration is an unsafe and disabled mix.
>
> This patchset make qed and live migration safe to use.
>
> The check of QED images is delayed during the incoming migration.
> After the migration complete the QED images are checked.
>
> Benoît Canet (10):
>  block: Add new BDRV_O_INCOMING flag to notice incoming live migration
>  block: add a function to set incoming live migration
>  block: add a function to clear incoming live migration
>  block: rename *_invalidate_cache_* to *_post_incoming_migration_*
>  migration: inform the block layer of incoming live status
>  block: open images with BDRV_O_INCOMING on incoming live migration
>  qed: extract image checking into check_image_if_needed
>  qed: add bdrv_post_incoming_migration operation checking the image
>  qed: honor BDRV_O_INCOMING for incoming live migration
>  qed: remove incoming live migration blocker
>
>  block.c       |   35 +++++++++++++++++++++---
>  block.h       |   13 +++++++--
>  block/qcow2.c |    7 ++++-
>  block/qed.c   |   81
> +++++++++++++++++++++++++++++++++++----------------------
>  block/qed.h   |    2 -
>  block_int.h   |    4 +-
>  migration.c   |    9 +++++-
>  vl.c          |    5 +++
>  8 files changed, 110 insertions(+), 46 deletions(-)
>
> --
> 1.7.7.6
>
>

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

Re: [Qemu-devel] [RFC PATCH 00/10] make qed and live migration usage safe

[Benoît Canet]

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

gentle ping :)

On Tue, Mar 6, 2012 at 6:32 PM, Benoît Canet <benoit.canet@gmail.com> wrote:

> QED + live migration is an unsafe and disabled mix.
>
> This patchset make qed and live migration safe to use.
>
> The check of QED images is delayed during the incoming migration.
> After the migration complete the QED images are checked.
>
> Benoît Canet (10):
>  block: Add new BDRV_O_INCOMING flag to notice incoming live migration
>  block: add a function to set incoming live migration
>  block: add a function to clear incoming live migration
>  block: rename *_invalidate_cache_* to *_post_incoming_migration_*
>  migration: inform the block layer of incoming live status
>  block: open images with BDRV_O_INCOMING on incoming live migration
>  qed: extract image checking into check_image_if_needed
>  qed: add bdrv_post_incoming_migration operation checking the image
>  qed: honor BDRV_O_INCOMING for incoming live migration
>  qed: remove incoming live migration blocker
>
>  block.c       |   35 +++++++++++++++++++++---
>  block.h       |   13 +++++++--
>  block/qcow2.c |    7 ++++-
>  block/qed.c   |   81
> +++++++++++++++++++++++++++++++++++----------------------
>  block/qed.h   |    2 -
>  block_int.h   |    4 +-
>  migration.c   |    9 +++++-
>  vl.c          |    5 +++
>  8 files changed, 110 insertions(+), 46 deletions(-)
>
> --
> 1.7.7.6
>
>

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

Re: [Qemu-devel] [RFC PATCH 00/10] make qed and live migration usage safe

[Stefan Hajnoczi]

On Tue, Mar 06, 2012 at 06:32:19PM +0100, Benoît Canet wrote:
> QED + live migration is an unsafe and disabled mix.
> 
> This patchset make qed and live migration safe to use.
> 
> The check of QED images is delayed during the incoming migration.
> After the migration complete the QED images are checked.
> 
> Benoît Canet (10):
>   block: Add new BDRV_O_INCOMING flag to notice incoming live migration
>   block: add a function to set incoming live migration
>   block: add a function to clear incoming live migration
>   block: rename *_invalidate_cache_* to *_post_incoming_migration_*
>   migration: inform the block layer of incoming live status
>   block: open images with BDRV_O_INCOMING on incoming live migration
>   qed: extract image checking into check_image_if_needed
>   qed: add bdrv_post_incoming_migration operation checking the image
>   qed: honor BDRV_O_INCOMING for incoming live migration
>   qed: remove incoming live migration blocker
> 
>  block.c       |   35 +++++++++++++++++++++---
>  block.h       |   13 +++++++--
>  block/qcow2.c |    7 ++++-
>  block/qed.c   |   81 +++++++++++++++++++++++++++++++++++----------------------
>  block/qed.h   |    2 -
>  block_int.h   |    4 +-
>  migration.c   |    9 +++++-
>  vl.c          |    5 +++
>  8 files changed, 110 insertions(+), 46 deletions(-)

We talked about image format live migration approaches on IRC today with
Kevin.  I think the approach you have here will do the job.

Stefan