[Qemu-devel] [PATCH 3/7] qcow2: add a migration blocker

[Qemu-devel] [PATCH 3/7] qcow2: add a migration blocker

[Anthony Liguori]

Now when you try to migrate with qcow2, you get:

(qemu) migrate tcp:localhost:1025
Block format 'qcow2' used by device 'ide0-hd0' does not support feature 'live migration'
(qemu)

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 block/qcow2.c |    9 +++++++++
 block/qcow2.h |    2 ++
 qemu-tool.c   |    9 +++++++++
 qerror.c      |    4 ++++
 qerror.h      |    3 +++
 5 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 5c784ee..e4126b7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -29,6 +29,7 @@
 #include "block/qcow2.h"
 #include "qemu-error.h"
 #include "qerror.h"
+#include "migration.h"
 
 /*
   Differences with QCOW:
@@ -277,6 +278,11 @@ static int qcow2_open(BlockDriverState *bs, int flags)
         goto fail;
     }
 
+    error_set(&s->migration_blocker,
+              QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+              "qcow2", bs->device_name, "live migration");
+    migrate_add_blocker(s->migration_blocker);
+
     /* Initialise locks */
     qemu_co_mutex_init(&s->lock);
 
@@ -621,6 +627,9 @@ static void qcow2_close(BlockDriverState *bs)
     BDRVQcowState *s = bs->opaque;
     g_free(s->l1_table);
 
+    migrate_del_blocker(s->migration_blocker);
+    error_free(s->migration_blocker);
+
     qcow2_cache_flush(bs, s->l2_table_cache);
     qcow2_cache_flush(bs, s->refcount_block_cache);
 
diff --git a/block/qcow2.h b/block/qcow2.h
index 531af39..0734b23 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -125,6 +125,8 @@ typedef struct BDRVQcowState {
     int snapshots_size;
     int nb_snapshots;
     QCowSnapshot *snapshots;
+
+    Error *migration_blocker;
 } BDRVQcowState;
 
 /* XXX: use std qcow open function ? */
diff --git a/qemu-tool.c b/qemu-tool.c
index e9f7fe1..5df7279 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -15,6 +15,7 @@
 #include "monitor.h"
 #include "qemu-timer.h"
 #include "qemu-log.h"
+#include "migration.h"
 
 #include <sys/time.h>
 
@@ -92,3 +93,11 @@ int64_t qemu_get_clock_ns(QEMUClock *clock)
 {
     return 0;
 }
+
+void migrate_add_blocker(Error *reason)
+{
+}
+
+void migrate_del_blocker(Error *reason)
+{
+}
diff --git a/qerror.c b/qerror.c
index 8e30e2d..fdf62b9 100644
--- a/qerror.c
+++ b/qerror.c
@@ -49,6 +49,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Device '%(device)' can't go on a %(bad_bus_type) bus",
     },
     {
+        .error_fmt = QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+        .desc      = "Block format '%(format)' used by device '%(name)' does not support feature '%(feature)'",
+    },
+    {
         .error_fmt = QERR_BUS_NOT_FOUND,
         .desc      = "Bus '%(bus)' not found",
     },
diff --git a/qerror.h b/qerror.h
index 7e2eebf..2d3d43b 100644
--- a/qerror.h
+++ b/qerror.h
@@ -54,6 +54,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_BAD_BUS_FOR_DEVICE \
     "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }"
 
+#define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
+    "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s, 'name': %s, 'feature': %s } }"
+
 #define QERR_BUS_NOT_FOUND \
     "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
 
-- 
1.7.4.1

[Qemu-devel] [PATCH 1/7] migrate: add migration blockers

[Anthony Liguori]

This lets different subsystems register an Error that is thrown whenever
migration is attempted.  This works nicely because it gracefully supports
things like hotplug.

Right now, if multiple errors are registered, only one of them is reported.
I expect that for 1.1, we'll extend query-migrate to return all of the reasons
why migration is disabled at any given point in time.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 migration.c |   18 ++++++++++++++++++
 migration.h |   15 +++++++++++++++
 2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/migration.c b/migration.c
index 41c3c24..6764d3a 100644
--- a/migration.c
+++ b/migration.c
@@ -398,6 +398,18 @@ static MigrationState *migrate_init(Monitor *mon, int detach, int blk, int inc)
     return s;
 }
 
+static GSList *migration_blockers;
+
+void migrate_add_blocker(Error *reason)
+{
+    migration_blockers = g_slist_prepend(migration_blockers, reason);
+}
+
+void migrate_del_blocker(Error *reason)
+{
+    migration_blockers = g_slist_remove(migration_blockers, reason);
+}
+
 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     MigrationState *s = migrate_get_current();
@@ -417,6 +429,12 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
         return -1;
     }
 
+    if (migration_blockers) {
+        Error *err = migration_blockers->data;
+        qerror_report_err(err);
+        return -1;
+    }
+
     s = migrate_init(mon, detach, blk, inc);
 
     if (strstart(uri, "tcp:", &p)) {
diff --git a/migration.h b/migration.h
index 1b8ee58..0682179 100644
--- a/migration.h
+++ b/migration.h
@@ -17,6 +17,7 @@
 #include "qdict.h"
 #include "qemu-common.h"
 #include "notify.h"
+#include "error.h"
 
 typedef struct MigrationState MigrationState;
 
@@ -89,4 +90,18 @@ int ram_load(QEMUFile *f, void *opaque, int version_id);
 
 extern int incoming_expected;
 
+/**
+ * @migrate_add_blocker - prevent migration from proceeding
+ *
+ * @reason - an error to be returned whenever migration is attempted
+ */
+void migrate_add_blocker(Error *reason);
+
+/**
+ * @migrate_del_blocker - remove a blocking error from migration
+ *
+ * @reason - the error blocking migration
+ */
+void migrate_del_blocker(Error *reason);
+
 #endif
-- 
1.7.4.1

[Qemu-devel] [PATCH 2/7] ivshmem: use migration blockers to prevent live migration in peer mode

[Anthony Liguori]

Now when you try to migrate with ivshmem, you get a proper QMP error:

(qemu) migrate tcp:localhost:1025
Migration is disabled when using feature 'peer mode' in device 'ivshmem'
(qemu)

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/ivshmem.c |   12 +++++++++++-
 qerror.c     |    4 ++++
 qerror.h     |    3 +++
 3 files changed, 18 insertions(+), 1 deletions(-)

diff --git a/hw/ivshmem.c b/hw/ivshmem.c
index 242fbea..a3a0e98 100644
--- a/hw/ivshmem.c
+++ b/hw/ivshmem.c
@@ -18,6 +18,8 @@
 #include "pci.h"
 #include "msix.h"
 #include "kvm.h"
+#include "migration.h"
+#include "qerror.h"
 
 #include <sys/mman.h>
 #include <sys/types.h>
@@ -78,6 +80,8 @@ typedef struct IVShmemState {
     uint32_t features;
     EventfdEntry *eventfd_table;
 
+    Error *migration_blocker;
+
     char * shmobj;
     char * sizearg;
     char * role;
@@ -646,7 +650,8 @@ static int pci_ivshmem_init(PCIDevice *dev)
     }
 
     if (s->role_val == IVSHMEM_PEER) {
-        register_device_unmigratable(&s->dev.qdev, "ivshmem", s);
+        error_set(&s->migration_blocker, QERR_DEVICE_FEATURE_BLOCKS_MIGRATION, "ivshmem", "peer mode");
+        migrate_add_blocker(s->migration_blocker);
     }
 
     pci_conf = s->dev.config;
@@ -741,6 +746,11 @@ static int pci_ivshmem_uninit(PCIDevice *dev)
 {
     IVShmemState *s = DO_UPCAST(IVShmemState, dev, dev);
 
+    if (s->migration_blocker) {
+        migrate_del_blocker(s->migration_blocker);
+        error_free(s->migration_blocker);
+    }
+
     memory_region_destroy(&s->ivshmem_mmio);
     memory_region_del_subregion(&s->bar, &s->ivshmem);
     memory_region_destroy(&s->ivshmem);
diff --git a/qerror.c b/qerror.c
index 4b48b39..8e30e2d 100644
--- a/qerror.c
+++ b/qerror.c
@@ -73,6 +73,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Device '%(device)' is in use",
     },
     {
+        .error_fmt = QERR_DEVICE_FEATURE_BLOCKS_MIGRATION,
+        .desc      = "Migration is disabled when using feature '%(feature)' in device '%(device)'",
+    },
+    {
         .error_fmt = QERR_DEVICE_LOCKED,
         .desc      = "Device '%(device)' is locked",
     },
diff --git a/qerror.h b/qerror.h
index d4bfcfd..7e2eebf 100644
--- a/qerror.h
+++ b/qerror.h
@@ -72,6 +72,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_DEVICE_IN_USE \
     "{ 'class': 'DeviceInUse', 'data': { 'device': %s } }"
 
+#define QERR_DEVICE_FEATURE_BLOCKS_MIGRATION \
+    "{ 'class': 'DeviceFeatureBlocksMigration', 'data': { 'device': %s, 'feature': %s } }"
+
 #define QERR_DEVICE_LOCKED \
     "{ 'class': 'DeviceLocked', 'data': { 'device': %s } }"
 
-- 
1.7.4.1

[Qemu-devel] [PATCH 3/7] qcow2: add a migration blocker

[Anthony Liguori]

Now when you try to migrate with qcow2, you get:

(qemu) migrate tcp:localhost:1025
Block format 'qcow2' used by device 'ide0-hd0' does not support feature 'live migration'
(qemu)

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 block/qcow2.c |    9 +++++++++
 block/qcow2.h |    2 ++
 qemu-tool.c   |    9 +++++++++
 qerror.c      |    4 ++++
 qerror.h      |    3 +++
 5 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 5c784ee..e4126b7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -29,6 +29,7 @@
 #include "block/qcow2.h"
 #include "qemu-error.h"
 #include "qerror.h"
+#include "migration.h"
 
 /*
   Differences with QCOW:
@@ -277,6 +278,11 @@ static int qcow2_open(BlockDriverState *bs, int flags)
         goto fail;
     }
 
+    error_set(&s->migration_blocker,
+              QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+              "qcow2", bs->device_name, "live migration");
+    migrate_add_blocker(s->migration_blocker);
+
     /* Initialise locks */
     qemu_co_mutex_init(&s->lock);
 
@@ -621,6 +627,9 @@ static void qcow2_close(BlockDriverState *bs)
     BDRVQcowState *s = bs->opaque;
     g_free(s->l1_table);
 
+    migrate_del_blocker(s->migration_blocker);
+    error_free(s->migration_blocker);
+
     qcow2_cache_flush(bs, s->l2_table_cache);
     qcow2_cache_flush(bs, s->refcount_block_cache);
 
diff --git a/block/qcow2.h b/block/qcow2.h
index 531af39..0734b23 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -125,6 +125,8 @@ typedef struct BDRVQcowState {
     int snapshots_size;
     int nb_snapshots;
     QCowSnapshot *snapshots;
+
+    Error *migration_blocker;
 } BDRVQcowState;
 
 /* XXX: use std qcow open function ? */
diff --git a/qemu-tool.c b/qemu-tool.c
index e9f7fe1..5df7279 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -15,6 +15,7 @@
 #include "monitor.h"
 #include "qemu-timer.h"
 #include "qemu-log.h"
+#include "migration.h"
 
 #include <sys/time.h>
 
@@ -92,3 +93,11 @@ int64_t qemu_get_clock_ns(QEMUClock *clock)
 {
     return 0;
 }
+
+void migrate_add_blocker(Error *reason)
+{
+}
+
+void migrate_del_blocker(Error *reason)
+{
+}
diff --git a/qerror.c b/qerror.c
index 8e30e2d..fdf62b9 100644
--- a/qerror.c
+++ b/qerror.c
@@ -49,6 +49,10 @@ static const QErrorStringTable qerror_table[] = {
         .desc      = "Device '%(device)' can't go on a %(bad_bus_type) bus",
     },
     {
+        .error_fmt = QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+        .desc      = "Block format '%(format)' used by device '%(name)' does not support feature '%(feature)'",
+    },
+    {
         .error_fmt = QERR_BUS_NOT_FOUND,
         .desc      = "Bus '%(bus)' not found",
     },
diff --git a/qerror.h b/qerror.h
index 7e2eebf..2d3d43b 100644
--- a/qerror.h
+++ b/qerror.h
@@ -54,6 +54,9 @@ QError *qobject_to_qerror(const QObject *obj);
 #define QERR_BAD_BUS_FOR_DEVICE \
     "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }"
 
+#define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
+    "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s, 'name': %s, 'feature': %s } }"
+
 #define QERR_BUS_NOT_FOUND \
     "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
 
-- 
1.7.4.1

[Qemu-devel] [PATCH 4/7] qed: add migration blocker

[Anthony Liguori]

Now when you try to migrate with qed, you get:

(qemu) migrate tcp:localhost:1025
Block format 'qed' used by device 'ide0-hd0' does not support feature 'live migration'
(qemu)

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 block/qed.c |   10 ++++++++++
 block/qed.h |    2 ++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/block/qed.c b/block/qed.c
index d032a45..7e22e77 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -16,6 +16,7 @@
 #include "trace.h"
 #include "qed.h"
 #include "qerror.h"
+#include "migration.h"
 
 static void qed_aio_cancel(BlockDriverAIOCB *blockacb)
 {
@@ -504,6 +505,12 @@ 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,6 +523,9 @@ 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 388fdb3..62cbd3b 100644
--- a/block/qed.h
+++ b/block/qed.h
@@ -164,6 +164,8 @@ typedef struct {
 
     /* Periodic flush and clear need check flag */
     QEMUTimer *need_check_timer;
+
+    Error *migration_blocker;
 } BDRVQEDState;
 
 enum {
-- 
1.7.4.1

[Qemu-devel] [PATCH 5/7] block: allow migration to work with image files (v2)

[Anthony Liguori]

Image files have two types of data: immutable data that describes things like
image size, backing files, etc. and mutable data that includes offset and
reference count tables.

Today, image formats aggressively cache mutable data to improve performance.  In
some cases, this happens before a guest even starts.  When dealing with live
migration, since a file is open on two machines, the caching of meta data can
lead to data corruption.

This patch addresses this by introducing a mechanism to invalidate any cached
mutable data a block driver may have which is then used by the live migration
code.

NB, this still requires coherent shared storage.  Addressing migration without
coherent shared storage (i.e. NFS) requires additional work.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
v1 -> v2
 - rebase to latest master
---
 block.c     |   16 ++++++++++++++++
 block.h     |    4 ++++
 block_int.h |    5 +++++
 cpus.c      |    1 +
 migration.c |    3 +++
 5 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 86910b0..d015887 100644
--- a/block.c
+++ b/block.c
@@ -2839,6 +2839,22 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
     }
 }
 
+void bdrv_invalidate_cache(BlockDriverState *bs)
+{
+    if (bs->drv && bs->drv->bdrv_invalidate_cache) {
+        bs->drv->bdrv_invalidate_cache(bs);
+    }
+}
+
+void bdrv_invalidate_cache_all(void)
+{
+    BlockDriverState *bs;
+
+    QTAILQ_FOREACH(bs, &bdrv_states, list) {
+        bdrv_invalidate_cache(bs);
+    }
+}
+
 int bdrv_flush(BlockDriverState *bs)
 {
     Coroutine *co;
diff --git a/block.h b/block.h
index 051a25d..a826059 100644
--- a/block.h
+++ b/block.h
@@ -197,6 +197,10 @@ 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);
+
 /* Ensure contents are flushed to disk.  */
 int bdrv_flush(BlockDriverState *bs);
 int coroutine_fn bdrv_co_flush(BlockDriverState *bs);
diff --git a/block_int.h b/block_int.h
index 1ec4921..77c0187 100644
--- a/block_int.h
+++ b/block_int.h
@@ -88,6 +88,11 @@ struct BlockDriver {
         int64_t sector_num, int nb_sectors);
 
     /*
+     * Invalidate any cached meta-data.
+     */
+    void (*bdrv_invalidate_cache)(BlockDriverState *bs);
+
+    /*
      * Flushes all data that was already written to the OS all the way down to
      * the disk (for example raw-posix calls fsync()).
      */
diff --git a/cpus.c b/cpus.c
index 82530c4..ae5ec99 100644
--- a/cpus.c
+++ b/cpus.c
@@ -398,6 +398,7 @@ static void do_vm_stop(RunState state)
         vm_state_notify(0, state);
         qemu_aio_flush();
         bdrv_flush_all();
+        bdrv_invalidate_cache_all();
         monitor_protocol_event(QEVENT_STOP, NULL);
     }
 }
diff --git a/migration.c b/migration.c
index 6764d3a..8280d71 100644
--- a/migration.c
+++ b/migration.c
@@ -89,6 +89,9 @@ 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();
+
     if (autostart) {
         vm_start();
     } else {
-- 
1.7.4.1

[Qemu-devel] [PATCH 6/7] qcow2: implement bdrv_invalidate_cache

[Anthony Liguori]

We don't reopen the actual file, but instead invoke the close and open routines.
We specifically ignore the backing file since it's contents are read-only and
therefore immutable.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 block/qcow2.c |   19 +++++++++++++++++++
 block/qcow2.h |    2 ++
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index e4126b7..b5171e0 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -241,6 +241,7 @@ static int qcow2_open(BlockDriverState *bs, int flags)
     s->cluster_data = qemu_blockalign(bs, QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size
                                   + 512);
     s->cluster_cache_offset = -1;
+    s->flags = flags;
 
     ret = qcow2_refcount_init(bs);
     if (ret != 0) {
@@ -641,6 +642,22 @@ static void qcow2_close(BlockDriverState *bs)
     qcow2_refcount_close(bs);
 }
 
+static void qcow2_invalidate_cache(BlockDriverState *bs)
+{
+    BDRVQcowState *s = bs->opaque;
+    int flags = s->flags;
+
+    /*
+     * Backing files are read-only which makes all of their metadata immutable,
+     * that means we don't have to worry about reopening them here.
+     */
+
+    qcow2_close(bs);
+
+    memset(s, 0, sizeof(BDRVQcowState));
+    qcow2_open(bs, flags);
+}
+
 /*
  * Updates the variable length parts of the qcow2 header, i.e. the backing file
  * name and all extensions. qcow2 was not designed to allow such changes, so if
@@ -1278,6 +1295,8 @@ static BlockDriver bdrv_qcow2 = {
 
     .bdrv_change_backing_file   = qcow2_change_backing_file,
 
+    .bdrv_invalidate_cache      = qcow2_invalidate_cache,
+
     .create_options = qcow2_create_options,
     .bdrv_check = qcow2_check,
 };
diff --git a/block/qcow2.h b/block/qcow2.h
index 0734b23..6312cd9 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -127,6 +127,8 @@ typedef struct BDRVQcowState {
     QCowSnapshot *snapshots;
 
     Error *migration_blocker;
+
+    int flags;
 } BDRVQcowState;
 
 /* XXX: use std qcow open function ? */
-- 
1.7.4.1

[Qemu-devel] [PATCH 7/7] qcow2: relax migration blocker

[Anthony Liguori]

Only block migration if we're using encrypted files.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 block/qcow2.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index b5171e0..8071f2c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -279,10 +279,12 @@ static int qcow2_open(BlockDriverState *bs, int flags)
         goto fail;
     }
 
-    error_set(&s->migration_blocker,
-              QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
-              "qcow2", bs->device_name, "live migration");
-    migrate_add_blocker(s->migration_blocker);
+    if (bs->encrypted) {
+        error_set(&s->migration_blocker,
+                  QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+                  "qcow2", bs->device_name, "live migration");
+        migrate_add_blocker(s->migration_blocker);
+    }
 
     /* Initialise locks */
     qemu_co_mutex_init(&s->lock);
@@ -628,8 +630,10 @@ static void qcow2_close(BlockDriverState *bs)
     BDRVQcowState *s = bs->opaque;
     g_free(s->l1_table);
 
-    migrate_del_blocker(s->migration_blocker);
-    error_free(s->migration_blocker);
+    if (s->migration_blocker) {
+        migrate_del_blocker(s->migration_blocker);
+        error_free(s->migration_blocker);
+    }
 
     qcow2_cache_flush(bs, s->l2_table_cache);
     qcow2_cache_flush(bs, s->refcount_block_cache);
-- 
1.7.4.1

Re: [Qemu-devel] [PATCH 3/7] qcow2: add a migration blocker

[Lucas Meneghel Rodrigues]

On 11/12/2011 01:57 PM, Anthony Liguori wrote:
> Now when you try to migrate with qcow2, you get:
>
> (qemu) migrate tcp:localhost:1025
> Block format 'qcow2' used by device 'ide0-hd0' does not support feature 'live migration'
> (qemu)

LGTM. From a testing perspective, it's clear and understandable, a lot 
better than throwing a generic error.

> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
> ---
>   block/qcow2.c |    9 +++++++++
>   block/qcow2.h |    2 ++
>   qemu-tool.c   |    9 +++++++++
>   qerror.c      |    4 ++++
>   qerror.h      |    3 +++
>   5 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 5c784ee..e4126b7 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -29,6 +29,7 @@
>   #include "block/qcow2.h"
>   #include "qemu-error.h"
>   #include "qerror.h"
> +#include "migration.h"
>
>   /*
>     Differences with QCOW:
> @@ -277,6 +278,11 @@ static int qcow2_open(BlockDriverState *bs, int flags)
>           goto fail;
>       }
>
> +    error_set(&s->migration_blocker,
> +              QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> +              "qcow2", bs->device_name, "live migration");
> +    migrate_add_blocker(s->migration_blocker);
> +
>       /* Initialise locks */
>       qemu_co_mutex_init(&s->lock);
>
> @@ -621,6 +627,9 @@ static void qcow2_close(BlockDriverState *bs)
>       BDRVQcowState *s = bs->opaque;
>       g_free(s->l1_table);
>
> +    migrate_del_blocker(s->migration_blocker);
> +    error_free(s->migration_blocker);
> +
>       qcow2_cache_flush(bs, s->l2_table_cache);
>       qcow2_cache_flush(bs, s->refcount_block_cache);
>
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 531af39..0734b23 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -125,6 +125,8 @@ typedef struct BDRVQcowState {
>       int snapshots_size;
>       int nb_snapshots;
>       QCowSnapshot *snapshots;
> +
> +    Error *migration_blocker;
>   } BDRVQcowState;
>
>   /* XXX: use std qcow open function ? */
> diff --git a/qemu-tool.c b/qemu-tool.c
> index e9f7fe1..5df7279 100644
> --- a/qemu-tool.c
> +++ b/qemu-tool.c
> @@ -15,6 +15,7 @@
>   #include "monitor.h"
>   #include "qemu-timer.h"
>   #include "qemu-log.h"
> +#include "migration.h"
>
>   #include<sys/time.h>
>
> @@ -92,3 +93,11 @@ int64_t qemu_get_clock_ns(QEMUClock *clock)
>   {
>       return 0;
>   }
> +
> +void migrate_add_blocker(Error *reason)
> +{
> +}
> +
> +void migrate_del_blocker(Error *reason)
> +{
> +}
> diff --git a/qerror.c b/qerror.c
> index 8e30e2d..fdf62b9 100644
> --- a/qerror.c
> +++ b/qerror.c
> @@ -49,6 +49,10 @@ static const QErrorStringTable qerror_table[] = {
>           .desc      = "Device '%(device)' can't go on a %(bad_bus_type) bus",
>       },
>       {
> +        .error_fmt = QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
> +        .desc      = "Block format '%(format)' used by device '%(name)' does not support feature '%(feature)'",
> +    },
> +    {
>           .error_fmt = QERR_BUS_NOT_FOUND,
>           .desc      = "Bus '%(bus)' not found",
>       },
> diff --git a/qerror.h b/qerror.h
> index 7e2eebf..2d3d43b 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -54,6 +54,9 @@ QError *qobject_to_qerror(const QObject *obj);
>   #define QERR_BAD_BUS_FOR_DEVICE \
>       "{ 'class': 'BadBusForDevice', 'data': { 'device': %s, 'bad_bus_type': %s } }"
>
> +#define QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED \
> +    "{ 'class': 'BlockFormatFeatureNotSupported', 'data': { 'format': %s, 'name': %s, 'feature': %s } }"
> +
>   #define QERR_BUS_NOT_FOUND \
>       "{ 'class': 'BusNotFound', 'data': { 'bus': %s } }"
>