Skip to contentSkip to conversation

migrate: add migration blockers

PATCH Applied Anthony Liguori 3 participants 9 messages Nov 14, 2011 – Nov 22, 2011 Block layer

lore
Share & tools
lore
#0 lore raw

PATCH 1/5 migrate: add migration blockers

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.

---
 migration.c |   18 ++++++++++++++++++
 migration.h |   15 +++++++++++++++
 2 files changed, 33 insertions(+), 0 deletions(-)
migration.c +18 -0
index 41c3c24..6764d3a 100644
@@ -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)) {
migration.h +15 -0
index 1b8ee58..0682179 100644
@@ -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
Signature · 1 line
1.7.4.1
#1 lore raw

PATCH 2/5 ivshmem: use migration blockers to prevent live migration in peer mode (v2)

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)

---
v1 -> v2
- remove register_device_unmigratable as ivshmem was the only user

---
 hw/ivshmem.c |   12 +-
 qerror.c     |    4 +
 qerror.h     |    3 +
 savevm.c     |   25 ---
 vmstate.h    |  610 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 628 insertions(+), 26 deletions(-)
 create mode 100644 vmstate.h
hw/ivshmem.c +11 -1
index 242fbea..a3a0e98 100644
@@ -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);
qerror.c +4 -0
index 4b48b39..8e30e2d 100644
@@ -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",
     },
qerror.h +3 -0
index d4bfcfd..7e2eebf 100644
@@ -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 } }"
 
savevm.c +0 -25
index bee16c0..f53cd4c 100644
@@ -1255,31 +1255,6 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
     }
 }
 
-/* mark a device as not to be migrated, that is the device should be
-   unplugged before migration */
-void register_device_unmigratable(DeviceState *dev, const char *idstr,
-                                                            void *opaque)
-{
-    SaveStateEntry *se;
-    char id[256] = "";
-
-    if (dev && dev->parent_bus && dev->parent_bus->info->get_dev_path) {
-        char *path = dev->parent_bus->info->get_dev_path(dev);
-        if (path) {
-            pstrcpy(id, sizeof(id), path);
-            pstrcat(id, sizeof(id), "/");
-            g_free(path);
-        }
-    }
-    pstrcat(id, sizeof(id), idstr);
-
-    QTAILQ_FOREACH(se, &savevm_handlers, entry) {
-        if (strcmp(se->idstr, id) == 0 && se->opaque == opaque) {
-            se->no_migrate = 1;
-        }
-    }
-}
-
 int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
                                    const VMStateDescription *vmsd,
                                    void *opaque, int alias_id,
vmstate.h new file +610 -0

610 lines not on this page · raw message

Signature · 1 line
1.7.4.1
#2 lore raw

Re: [Qemu-devel] [PATCH 2/5] ivshmem: use migration blockers to prevent live migration in peer mode (v2)

Quoted 134 lines · 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)

---
v1 -> v2
- remove register_device_unmigratable as ivshmem was the only user

---
  hw/ivshmem.c |   12 +-
  qerror.c     |    4 +
  qerror.h     |    3 +
  savevm.c     |   25 ---
  vmstate.h    |  610 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  5 files changed, 628 insertions(+), 26 deletions(-)

create mode 100644 vmstate.h

hw/ivshmem.c +11 -1
index 242fbea..a3a0e98 100644
@@ -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);
qerror.c +4 -0
index 4b48b39..8e30e2d 100644
@@ -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",
      },
qerror.h +3 -0
index d4bfcfd..7e2eebf 100644
@@ -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 } }"
 
savevm.c +0 -25
index bee16c0..f53cd4c 100644
@@ -1255,31 +1255,6 @@ void unregister_savevm(DeviceState *dev, const char *idstr, void *opaque)
      }
  }
 
-/* mark a device as not to be migrated, that is the device should be
-   unplugged before migration */
-void register_device_unmigratable(DeviceState *dev, const char *idstr,
-                                                            void *opaque)
-{
-    SaveStateEntry *se;
-    char id[256] = "";
-
-    if (dev&&  dev->parent_bus&&  dev->parent_bus->info->get_dev_path) {
-        char *path = dev->parent_bus->info->get_dev_path(dev);
-        if (path) {
-            pstrcpy(id, sizeof(id), path);
-            pstrcat(id, sizeof(id), "/");
-            g_free(path);
-        }
-    }
-    pstrcat(id, sizeof(id), idstr);
-
-    QTAILQ_FOREACH(se,&savevm_handlers, entry) {
-        if (strcmp(se->idstr, id) == 0&&  se->opaque == opaque) {
-            se->no_migrate = 1;
-        }
-    }
-}
-
  int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
                                     const VMStateDescription *vmsd,
                                     void *opaque, int alias_id,
vmstate.h new file +0 -0
new file mode 100644
index 0000000..9ea4783

vmstate.h line 3

Quoted 3 lines
vmstate.h new file +3 -0
@@ -0,0 +1,610 @@
+/*
+ * QEMU migration/snapshot declarations
+ *

I take it these were meant for a separate series? We still have these defs in hw/hw.h

#3 lore raw

Re: [Qemu-devel] [PATCH 2/5] ivshmem: use migration blockers to prevent live migration in peer mode (v2)

Quoted 137 lines · Michael Roth
Quoted 136 lines · 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)

---
v1 -> v2
- remove register_device_unmigratable as ivshmem was the only user
---
hw/ivshmem.c | 12 +-

qerror.c | 4 +
qerror.h | 3 +
savevm.c | 25 ---
vmstate.h | 610 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 628 insertions(+), 26 deletions(-)

create mode 100644 vmstate.h

hw/ivshmem.c +6 -1
index 242fbea..a3a0e98 100644
@@ -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;

hw/ivshmem.c +5 -0
@@ -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);
qerror.c +4 -0
index 4b48b39..8e30e2d 100644
@@ -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",

},

qerror.h +0 -0
index d4bfcfd..7e2eebf 100644
@@ -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 } }"

savevm.c +0 -25
index bee16c0..f53cd4c 100644
@@ -1255,31 +1255,6 @@ void unregister_savevm(DeviceState *dev, const char
 *idstr, void *opaque)
 }
 }
 
-/* mark a device as not to be migrated, that is the device should be
- unplugged before migration */
-void register_device_unmigratable(DeviceState *dev, const char *idstr,
- void *opaque)
-{
- SaveStateEntry *se;
- char id[256] = "";
-
- if (dev&& dev->parent_bus&& dev->parent_bus->info->get_dev_path) {
- char *path = dev->parent_bus->info->get_dev_path(dev);
- if (path) {
- pstrcpy(id, sizeof(id), path);
- pstrcat(id, sizeof(id), "/");
- g_free(path);
- }
- }
- pstrcat(id, sizeof(id), idstr);
-
- QTAILQ_FOREACH(se,&savevm_handlers, entry) {
- if (strcmp(se->idstr, id) == 0&& se->opaque == opaque) {
- se->no_migrate = 1;
- }
- }
-}
-
 int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
 const VMStateDescription *vmsd,

void *opaque, int alias_id,

vmstate.h new file +0 -1
new file mode 100644
index 0000000..9ea4783
--- /dev/null
Quoted 8 lines

vmstate.h line 3

Quoted 5 lines

+++ b/vmstate.h

vmstate.h new file +3 -0
@@ -0,0 +1,610 @@
+/*
+ * QEMU migration/snapshot declarations
+ *

I take it these were meant for a separate series? We still have these defs in hw/hw.h

*sigh*

I have no idea where that file came from. I fixed the patch locally, thanks for pointing it out!

Regards,

Anthony Liguori

Quoted 2 lines
#4 lore raw

PATCH 3/5 block: allow migration to work with image files (v3)

In reply to 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.

---
v2 -> v3
- remove bdrv_invalidate_cache_all() in vm_stop.
v1 -> v2
- rebase to latest master

---
 block.c     |   16 ++++++++++++++++
 block.h     |    4 ++++
 block_int.h |    5 +++++
 migration.c |    3 +++
 4 files changed, 28 insertions(+), 0 deletions(-)
block.c +16 -0
index 86910b0..d015887 100644
@@ -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;
block.h +4 -0
index 051a25d..a826059 100644
@@ -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);
block_int.h +5 -0
index 1ec4921..77c0187 100644
@@ -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()).
      */
migration.c +3 -0
index 6764d3a..8280d71 100644
@@ -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 {
Signature · 1 line
1.7.4.1
#5 lore raw

PATCH 4/5 qcow2: implement bdrv_invalidate_cache (v2)

In reply to 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.

---
v1 -> v2
- make invalidate_cache work with encrypted block devices

---
 block/qcow2.c |   34 ++++++++++++++++++++++++++++++++++
 block/qcow2.h |    2 ++
 2 files changed, 36 insertions(+), 0 deletions(-)
block/qcow2.c +34 -0
index 5c784ee..d7805ce 100644
@@ -240,6 +240,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) {
@@ -632,6 +633,37 @@ 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;
+    AES_KEY aes_encrypt_key;
+    AES_KEY aes_decrypt_key;
+    uint32_t crypt_method = 0;
+
+    /*
+     * Backing files are read-only which makes all of their metadata immutable,
+     * that means we don't have to worry about reopening them here.
+     */
+
+    if (s->crypt_method) {
+        crypt_method = s->crypt_method;
+        memcpy(&aes_encrypt_key, &s->aes_encrypt_key, sizeof(aes_encrypt_key));
+        memcpy(&aes_decrypt_key, &s->aes_decrypt_key, sizeof(aes_decrypt_key));
+    }
+
+    qcow2_close(bs);
+
+    memset(s, 0, sizeof(BDRVQcowState));
+    qcow2_open(bs, flags);
+
+    if (crypt_method) {
+        s->crypt_method = crypt_method;
+        memcpy(&s->aes_encrypt_key, &aes_encrypt_key, sizeof(aes_encrypt_key));
+        memcpy(&s->aes_decrypt_key, &aes_decrypt_key, sizeof(aes_decrypt_key));
+    }
+}
+
 /*
  * 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
@@ -1269,6 +1301,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,
 };
block/qcow2.h +2 -0
index 531af39..4e44eea 100644
@@ -125,6 +125,8 @@ typedef struct BDRVQcowState {
     int snapshots_size;
     int nb_snapshots;
     QCowSnapshot *snapshots;
+
+    int flags;
 } BDRVQcowState;
 
 /* XXX: use std qcow open function ? */
Signature · 1 line
1.7.4.1
#6 lore raw

PATCH 5/5 qed: add migration blocker (v2)

In reply to 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)

---
v1 -> v2
- Pull in QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED. I hope Stefan reviews this patch and Ack's a similar approach to qcow2 so we can avoid using a blocker for QED.

---
 block/qed.c |   10 ++++++++++
 block/qed.h |    2 ++
 qemu-tool.c |    9 +++++++++
 qerror.c    |    4 ++++
 qerror.h    |    3 +++
 5 files changed, 28 insertions(+), 0 deletions(-)
block/qed.c +10 -0
index d032a45..7e22e77 100644
@@ -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);
 
block/qed.h +2 -0
index 388fdb3..62cbd3b 100644
@@ -164,6 +164,8 @@ typedef struct {
 
     /* Periodic flush and clear need check flag */
     QEMUTimer *need_check_timer;
+
+    Error *migration_blocker;
 } BDRVQEDState;
 
 enum {
qemu-tool.c +9 -0
index e9f7fe1..5df7279 100644
@@ -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)
+{
+}
qerror.c +4 -0
index 8e30e2d..fdf62b9 100644
@@ -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",
     },
qerror.h +3 -0
index 7e2eebf..2d3d43b 100644
@@ -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 } }"
 
Signature · 1 line
1.7.4.1
#7 lore raw

cover migration: support migration with image files

In reply to Anthony Liguori

This series allows live migration to work with image formats. This series has two parts. The first part adds a new error infrastructure to migration which allows for different subsystems (other than qdev) to block migration when specific features are in use.

The second part adds block interfaces to flush metadata with image formats.

The migration error infrastructure is a no brainer for 1.0. It significantly improves the user experience and prevents a user from doing things that will cause a VM to crash during migration.

I'm less comfortable with the block interface for flushing metadata for 1.0. The code is completely isolated to the image formats and live migration. It should have no impact on normal use of an image format or live migration with raw images. That makes the risk for this very, very low.

Moreover, I'm sympathetic to the argument that this is a required feature for a 1.0 release. Image formats are a fairly normal way to use QEMU and not supporting live migration in this scenario means that live migration won't work for a large portion of QEMU's users.

#8 lore raw
In reply to Anthony Liguori
Quoted 9 lines · 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.

Applied.

Regards,

Anthony Liguori

Quoted 72 lines
---
  migration.c |   18 ++++++++++++++++++
  migration.h |   15 +++++++++++++++
  2 files changed, 33 insertions(+), 0 deletions(-)
migration.c +18 -0
index 41c3c24..6764d3a 100644
@@ -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)) {
migration.h +15 -0
index 1b8ee58..0682179 100644
@@ -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