[Qemu-devel] [PATCH 0/4] Virtio-9p live migration patchset

[Qemu-devel] [PATCH 0/4] Virtio-9p live migration patchset

[Benoît Canet]

This patchset is a rework of the 9p live migration patchs made a few years ago
by Aneesh.
As the new vmstate API doesn't support linked list so the old API is used.

Benoît Canet (4):
  migration: Create the pre migration flush hook infrastructure.
  virtio-9p: Add support for 9p migration.
  virtio-9p: Wait for 9p operations to complete before migration.
  virtio-9p: Remove migration blockers.

 Makefile                                  |    6 +-
 Makefile.objs                             |    2 +-
 block.c                                   |    8 ++
 cpus.c                                    |    6 +-
 hw/9pfs/virtio-9p-device.c                |  154 +++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p.c                       |   93 ++++++++++++-----
 hw/9pfs/virtio-9p.h                       |    4 +-
 include/migration/migration-flush-hooks.h |   30 ++++++
 migration-flush-hooks.c                   |   62 ++++++++++++
 vl.c                                      |    6 ++
 10 files changed, 340 insertions(+), 31 deletions(-)
 create mode 100644 include/migration/migration-flush-hooks.h
 create mode 100644 migration-flush-hooks.c

-- 
1.7.10.4

[Qemu-devel] [PATCH 1/4] migration: Create the pre migration flush hook infrastructure.

[Benoît Canet]

This patch will allow the block layer and virtio-9p to cleanly register io flush
hooks to be executed by cpus.c.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 Makefile                                  |    6 +--
 Makefile.objs                             |    2 +-
 block.c                                   |    8 ++++
 cpus.c                                    |    6 ++-
 include/migration/migration-flush-hooks.h |   30 ++++++++++++++
 migration-flush-hooks.c                   |   62 +++++++++++++++++++++++++++++
 vl.c                                      |    6 +++
 7 files changed, 114 insertions(+), 6 deletions(-)
 create mode 100644 include/migration/migration-flush-hooks.h
 create mode 100644 migration-flush-hooks.c

diff --git a/Makefile b/Makefile
index 80344d9..bb11da5 100644
--- a/Makefile
+++ b/Makefile
@@ -170,9 +170,9 @@ libqemuutil.a: $(util-obj-y)
 
 qemu-img.o: qemu-img-cmds.h
 
-qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a
-qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a
-qemu-io$(EXESUF): qemu-io.o cmd.o $(block-obj-y) libqemuutil.a libqemustub.a
+qemu-img$(EXESUF): qemu-img.o $(block-obj-y) libqemuutil.a libqemustub.a migration-flush-hooks.o
+qemu-nbd$(EXESUF): qemu-nbd.o $(block-obj-y) libqemuutil.a libqemustub.a migration-flush-hooks.o
+qemu-io$(EXESUF): qemu-io.o cmd.o $(block-obj-y) libqemuutil.a libqemustub.a migration-flush-hooks.o
 
 qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o
 
diff --git a/Makefile.objs b/Makefile.objs
index f99841c..707faa4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -57,7 +57,7 @@ common-obj-$(CONFIG_POSIX) += os-posix.o
 
 common-obj-$(CONFIG_LINUX) += fsdev/
 
-common-obj-y += migration.o migration-tcp.o
+common-obj-y += migration.o migration-tcp.o migration-flush-hooks.o
 common-obj-y += qemu-char.o #aio.o
 common-obj-y += block-migration.o
 common-obj-y += page_cache.o xbzrle.o
diff --git a/block.c b/block.c
index 0ae2e93..b8d8ccc 100644
--- a/block.c
+++ b/block.c
@@ -34,6 +34,7 @@
 #include "block/coroutine.h"
 #include "qmp-commands.h"
 #include "qemu/timer.h"
+#include "migration/migration-flush-hooks.h"
 
 #ifdef CONFIG_BSD
 #include <sys/types.h>
@@ -4131,8 +4132,15 @@ BlockDriverAIOCB *bdrv_aio_discard(BlockDriverState *bs,
     return &acb->common;
 }
 
+static void bdrv_migration_flush_hook(void)
+{
+    bdrv_drain_all();
+    bdrv_flush_all();
+}
+
 void bdrv_init(void)
 {
+    register_migration_flush_hook(bdrv_migration_flush_hook);
     module_call_init(MODULE_INIT_BLOCK);
 }
 
diff --git a/cpus.c b/cpus.c
index e919dd7..9beaebc 100644
--- a/cpus.c
+++ b/cpus.c
@@ -38,6 +38,8 @@
 #include "qemu/main-loop.h"
 #include "qemu/bitmap.h"
 
+#include "migration/migration-flush-hooks.h"
+
 #ifndef _WIN32
 #include "qemu/compatfd.h"
 #endif
@@ -444,8 +446,8 @@ static void do_vm_stop(RunState state)
         pause_all_vcpus();
         runstate_set(state);
         vm_state_notify(0, state);
-        bdrv_drain_all();
-        bdrv_flush_all();
+        /* Here we will flush remaining ios */
+        exec_migration_flush_hooks();
         monitor_protocol_event(QEVENT_STOP, NULL);
     }
 }
diff --git a/include/migration/migration-flush-hooks.h b/include/migration/migration-flush-hooks.h
new file mode 100644
index 0000000..be9e597
--- /dev/null
+++ b/include/migration/migration-flush-hooks.h
@@ -0,0 +1,30 @@
+/*
+ * QEMU live pre migration flush hooks
+ *
+ * Copyright Nodalink, SARL. 2013
+ *
+ * Authors:
+ *  Benoît Canet <benoit@irqsave.net>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_MIGRATION_FLUSH_HOOKS_H
+#define QEMU_MIGRATION_FLUSH_HOOKS_H
+
+#include "qemu/queue.h"
+
+typedef struct MigrationFlushHookEntry {
+    void (*flush_hook)(void);
+    QTAILQ_ENTRY(MigrationFlushHookEntry) node;
+} MigrationFlushHookEntry;
+
+void init_migration_flush_hooks(void);
+
+void register_migration_flush_hook(void (*fn)(void));
+
+void exec_migration_flush_hooks(void);
+
+#endif
diff --git a/migration-flush-hooks.c b/migration-flush-hooks.c
new file mode 100644
index 0000000..b8e1bf4
--- /dev/null
+++ b/migration-flush-hooks.c
@@ -0,0 +1,62 @@
+/*
+ * QEMU live pre migration flush hooks
+ *
+ * Copyright Nodalink, SARL. 2013
+ *
+ * Modeled after utils/modules.c
+ *
+ * Authors:
+ *  Benoît Canet <benoit@irqsave.net>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ * The purpose of this file is to allow various part of QEMU to register hooks
+ * used to flush ios after vcpus are paused on live migration preparation.
+ *
+ */
+#include <glib.h>
+#include "migration/migration-flush-hooks.h"
+
+
+typedef QTAILQ_HEAD(, MigrationFlushHookEntry) FlushHookList;
+
+/* the static NULL pointer will disable the feature if not initialized */
+static FlushHookList *flush_hooks;
+
+void init_migration_flush_hooks(void)
+{
+    if (flush_hooks) {
+        return;
+    }
+
+    flush_hooks = g_new0(FlushHookList, 1);
+    QTAILQ_INIT(flush_hooks);
+}
+
+void register_migration_flush_hook(void (*fn)(void))
+{
+    MigrationFlushHookEntry *e;
+
+    if (!flush_hooks) {
+        return;
+    }
+
+    e = g_new0(MigrationFlushHookEntry, 1);
+    e->flush_hook = fn;
+
+    QTAILQ_INSERT_TAIL(flush_hooks, e, node);
+}
+
+void exec_migration_flush_hooks(void)
+{
+    MigrationFlushHookEntry *e;
+
+    if (!flush_hooks) {
+        return;
+    }
+
+    QTAILQ_FOREACH(e, flush_hooks, node) {
+        e->flush_hook();
+    }
+}
diff --git a/vl.c b/vl.c
index e2c9706..b4fbcf7 100644
--- a/vl.c
+++ b/vl.c
@@ -139,6 +139,7 @@ int main(int argc, char **argv)
 #include "sysemu/blockdev.h"
 #include "hw/block-common.h"
 #include "migration/block.h"
+#include "migration/migration-flush-hooks.h"
 #include "tpm/tpm.h"
 #include "sysemu/dma.h"
 #include "audio/audio.h"
@@ -2941,6 +2942,11 @@ int main(int argc, char **argv, char **envp)
     nb_numa_nodes = 0;
     nb_nics = 0;
 
+    /* Initialize the list containing pre migration flush hooks before
+     * registeriring the first hook in bdrv_init().
+     */
+    init_migration_flush_hooks();
+
     bdrv_init_with_whitelist();
 
     autostart= 1;
-- 
1.7.10.4

[Qemu-devel] [PATCH 2/4] virtio-9p: Add support for 9p migration.

[Benoît Canet]

This patch use the old migration framework because of lack
of support for migrating linked list in the new generic framework.

This patch is a rebase of Aneesh Kumar's patch.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 hw/9pfs/virtio-9p-device.c |  154 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 154 insertions(+)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index d321c80..7d4aefd 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -20,6 +20,151 @@
 #include "virtio-9p-xattr.h"
 #include "virtio-9p-coth.h"
 
+static void virtio_9p_save_path(QEMUFile *f, V9fsPath *path)
+{
+    qemu_put_be16(f, path->size);
+    qemu_put_buffer(f, (const uint8_t *) path->data, path->size);
+}
+
+static void virtio_9p_save_string(QEMUFile *f, V9fsString *s)
+{
+    qemu_put_be16(f, s->size);
+    qemu_put_buffer(f, (const uint8_t *) s->data, s->size);
+}
+
+static void virtio_9p_save_xattr(QEMUFile *f, V9fsXattr *xattr)
+{
+    qemu_put_be64(f, xattr->copied_len);
+    qemu_put_be64(f, xattr->len);
+    qemu_put_buffer(f, (const uint8_t *) xattr->value, xattr->len);
+    virtio_9p_save_string(f, &xattr->name);
+    qemu_put_be32(f, xattr->flags);
+}
+
+static void virtio_9p_save_fid(QEMUFile *f, V9fsFidState *fid)
+{
+    /* First close the fid and mark it for reopen if migration fail */
+    if (fid->fid_type == P9_FID_FILE) {
+        close(fid->fs.fd);
+        fid->fs.fd = -1;
+    } else if (fid->fid_type == P9_FID_DIR) {
+        closedir(fid->fs.dir);
+        fid->fs.dir = NULL;
+    }
+
+    qemu_put_be32(f, fid->fid_type);
+    if (fid->fid_type == P9_FID_XATTR) {
+        /* we don't save fs_reclaim */
+        virtio_9p_save_xattr(f, &fid->fs.xattr);
+    }
+    qemu_put_be32(f, fid->fid);
+    virtio_9p_save_path(f, &fid->path);
+    qemu_put_be32(f, fid->flags);
+    qemu_put_be32(f, fid->open_flags);
+    qemu_put_be32(f, fid->uid);
+    qemu_put_be32(f, fid->ref);
+    qemu_put_be32(f, fid->clunked);
+}
+
+static void virtio_9p_save(QEMUFile *f, void *opaque)
+{
+    int fidcount = 0;
+    V9fsState *s = opaque;
+    V9fsFidState *fid;
+
+    virtio_save(&s->vdev, f);
+
+    for (fid = s->fid_list; fid; fid = fid->next) {
+        fidcount++;
+    }
+    /* Write the total number of fid structure */
+    qemu_put_be32(f, fidcount);
+
+    for (fid = s->fid_list; fid; fid = fid->next) {
+        virtio_9p_save_fid(f, fid);
+    }
+
+    qemu_put_be32(f, s->proto_version);
+    qemu_put_be32(f, s->msize);
+    qemu_put_be32(f, s->root_fid);
+}
+
+static void virtio_9p_load_path(QEMUFile *f, V9fsPath *path)
+{
+    path->size = qemu_get_be16(f);
+    path->data = g_malloc0(path->size);
+    qemu_get_buffer(f, (uint8_t *) path->data, path->size);
+}
+
+static void virtio_9p_load_string(QEMUFile *f, V9fsString *s)
+{
+    s->size = qemu_get_be16(f);
+    s->data = g_malloc0(s->size);
+    qemu_get_buffer(f, (uint8_t *) s->data, s->size);
+}
+
+static void virtio_9p_load_xattr(QEMUFile *f, V9fsXattr *xattr)
+{
+    xattr->copied_len = qemu_get_be64(f);
+    xattr->len = qemu_get_be64(f);
+    qemu_get_buffer(f, (uint8_t *) xattr->value, xattr->len);
+    virtio_9p_load_string(f, &xattr->name);
+    xattr->flags = qemu_get_be32(f);
+}
+
+static V9fsFidState *virtio_9p_load_fid(QEMUFile *f)
+{
+    V9fsFidState *fid;
+    fid = g_new0(V9fsFidState, 1);
+
+    fid->fid_type    = qemu_get_be32(f);
+    if (fid->fid_type == P9_FID_XATTR) {
+        virtio_9p_load_xattr(f, &fid->fs.xattr);
+    }
+    fid->fid         = qemu_get_be32(f);
+    virtio_9p_load_path(f, &fid->path);
+    fid->flags       = qemu_get_be32(f);
+    fid->open_flags  = qemu_get_be32(f);
+    fid->uid         = qemu_get_be32(f);
+    fid->ref         = qemu_get_be32(f);
+    fid->clunked     = qemu_get_be32(f);
+
+    /* If it's a file fid mark the file descriptors as closed.
+     * DIR is null thanks to g_new0.
+     * When doing get_fid v9fs_reopen_fid will reopen the file or the directory.
+     */
+    if (fid->fid_type == P9_FID_FILE) {
+        fid->fs.fd = -1;
+    }
+    return fid;
+}
+
+static int virtio_9p_load(QEMUFile *f, void *opaque, int version_id)
+{
+    int fidcount;
+    V9fsState *s = opaque;
+    V9fsFidState **fid;
+
+    if (version_id != 1) {
+        return -EINVAL;
+    }
+    virtio_load(&s->vdev, f);
+    fidcount = qemu_get_be32(f);
+
+    fid = &s->fid_list;
+    while (fidcount) {
+        *fid = virtio_9p_load_fid(f);
+        fid = &((*fid)->next);
+        fidcount--;
+    }
+
+    s->proto_version  = qemu_get_be32(f);
+    s->msize          = qemu_get_be32(f);
+    s->root_fid       = qemu_get_be32(f);
+
+    return 0;
+}
+
 static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t features)
 {
     features |= 1 << VIRTIO_9P_MOUNT_TAG;
@@ -53,6 +198,7 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
     struct stat stat;
     FsDriverEntry *fse;
     V9fsPath path;
+    static int virtio_9p_id;
 
     s = (V9fsState *)virtio_common_init("virtio-9p",
                                     VIRTIO_ID_9P,
@@ -102,6 +248,14 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
     s->config_size = sizeof(struct virtio_9p_config) + len;
     s->vdev.get_config = virtio_9p_get_config;
     s->fid_list = NULL;
+
+    /* Original patch says that instance id must be derived of tag
+     * name. Hash functions do have collisions so why would it be better
+     * than an increment ?
+     */
+    register_savevm(dev, "virtio-9p", virtio_9p_id++, 1,
+                    virtio_9p_save, virtio_9p_load, s);
+
     qemu_co_rwlock_init(&s->rename_lock);
 
     if (s->ops->init(&s->ctx) < 0) {
-- 
1.7.10.4

[Qemu-devel] [PATCH 3/4] virtio-9p: Wait for 9p operations to complete before migration.

[Benoît Canet]

The function used to wait is registered as a pre migration flush hook.

The completion status is put in the virtio ring buffer which
will be send to the guest on resume by the viring migration code

This patch is a rewrite from the one written by Aneesh Kumar.

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 hw/9pfs/virtio-9p-device.c |    2 ++
 hw/9pfs/virtio-9p.c        |   70 ++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p.h        |    2 ++
 3 files changed, 74 insertions(+)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 7d4aefd..02b8630 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -200,6 +200,8 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
     V9fsPath path;
     static int virtio_9p_id;
 
+    v9fs_init_migration_helper();
+
     s = (V9fsState *)virtio_common_init("virtio-9p",
                                     VIRTIO_ID_9P,
                                     sizeof(struct virtio_9p_config)+
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 5cc4c92..3bf3ee4 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -20,11 +20,20 @@
 #include "virtio-9p-coth.h"
 #include "trace.h"
 #include "migration/migration.h"
+#include "migration/migration-flush-hooks.h"
 
 int open_fd_hw;
 int total_open_fd;
 static int open_fd_rc;
 
+typedef struct MigrationHelper {
+    int32_t pending_requests;
+    QemuCond complete;
+    QemuMutex mutex;
+} MigrationHelper;
+
+MigrationHelper *migration;
+
 enum {
     Oread   = 0x00,
     Owrite  = 0x01,
@@ -37,6 +46,62 @@ enum {
     Oappend = 0x80,
 };
 
+static void v9fs_wait_for_requests_drain(void)
+{
+    if (!migration) {
+        return;
+    }
+
+    qemu_mutex_lock(&migration->mutex);
+    while (migration->pending_requests) {
+        /* At this point ticks and vcpus will be stopped so we can safely
+         * release the BQL so pending 9p callbacks will be executed and the
+         * condition signaled.
+         */
+        qemu_mutex_unlock_iothread();
+        qemu_cond_wait(&migration->complete, &migration->mutex);
+        qemu_mutex_lock_iothread();
+    }
+    qemu_mutex_unlock(&migration->mutex);
+}
+
+void v9fs_init_migration_helper(void)
+{
+    if (migration) {
+        return;
+    }
+
+    migration = g_new0(MigrationHelper, 1);
+    qemu_mutex_init(&migration->mutex);
+    qemu_cond_init(&migration->complete);
+    register_migration_flush_hook(v9fs_wait_for_requests_drain);
+}
+
+static void v9fs_inc_pending_requests(void)
+{
+    if (!migration) {
+        return;
+    }
+
+    qemu_mutex_lock(&migration->mutex);
+    migration->pending_requests++;
+    qemu_mutex_unlock(&migration->mutex);
+}
+
+static void v9fs_dec_pending_requests(void)
+{
+    if (!migration) {
+        return;
+    }
+
+    qemu_mutex_lock(&migration->mutex);
+    migration->pending_requests--;
+    if (!migration->pending_requests) {
+        qemu_cond_signal(&migration->complete);
+    }
+    qemu_mutex_unlock(&migration->mutex);
+}
+
 static int omode_to_uflags(int8_t mode)
 {
     int ret = 0;
@@ -637,6 +702,8 @@ static void complete_pdu(V9fsState *s, V9fsPDU *pdu, ssize_t len)
     qemu_co_queue_next(&pdu->complete);
 
     free_pdu(s, pdu);
+
+    v9fs_dec_pending_requests();
 }
 
 static mode_t v9mode_to_mode(uint32_t mode, V9fsString *extension)
@@ -3240,6 +3307,9 @@ static void submit_pdu(V9fsState *s, V9fsPDU *pdu)
     if (is_ro_export(&s->ctx) && !is_read_only_op(pdu)) {
         handler = v9fs_fs_ro;
     }
+
+    v9fs_inc_pending_requests();
+
     co = qemu_coroutine_create(handler);
     qemu_coroutine_enter(co, pdu);
 }
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 52b1c69..4d16d46 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -401,4 +401,6 @@ extern int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath,
 #define pdu_unmarshal(pdu, offset, fmt, args...)  \
     v9fs_unmarshal(pdu->elem.out_sg, pdu->elem.out_num, offset, 1, fmt, ##args)
 
+void v9fs_init_migration_helper(void);
+
 #endif
-- 
1.7.10.4

[Qemu-devel] [PATCH 4/4] virtio-9p: Remove migration blockers.

[Benoît Canet]

Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
 hw/9pfs/virtio-9p-device.c |    2 --
 hw/9pfs/virtio-9p.c        |   23 -----------------------
 hw/9pfs/virtio-9p.h        |    2 --
 3 files changed, 27 deletions(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 02b8630..f91a9b8 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -86,7 +86,6 @@ static void virtio_9p_save(QEMUFile *f, void *opaque)
 
     qemu_put_be32(f, s->proto_version);
     qemu_put_be32(f, s->msize);
-    qemu_put_be32(f, s->root_fid);
 }
 
 static void virtio_9p_load_path(QEMUFile *f, V9fsPath *path)
@@ -160,7 +159,6 @@ static int virtio_9p_load(QEMUFile *f, void *opaque, int version_id)
 
     s->proto_version  = qemu_get_be32(f);
     s->msize          = qemu_get_be32(f);
-    s->root_fid       = qemu_get_be32(f);
 
     return 0;
 }
diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 3bf3ee4..b46924f 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -399,19 +399,6 @@ static int put_fid(V9fsPDU *pdu, V9fsFidState *fidp)
      * Don't free the fid if it is in reclaim list
      */
     if (!fidp->ref && fidp->clunked) {
-        if (fidp->fid == pdu->s->root_fid) {
-            /*
-             * if the clunked fid is root fid then we
-             * have unmounted the fs on the client side.
-             * delete the migration blocker. Ideally, this
-             * should be hooked to transport close notification
-             */
-            if (pdu->s->migration_blocker) {
-                migrate_del_blocker(pdu->s->migration_blocker);
-                error_free(pdu->s->migration_blocker);
-                pdu->s->migration_blocker = NULL;
-            }
-        }
         return free_fid(pdu, fidp);
     }
     return 0;
@@ -1048,16 +1035,6 @@ static void v9fs_attach(void *opaque)
     err += offset;
     trace_v9fs_attach_return(pdu->tag, pdu->id,
                              qid.type, qid.version, qid.path);
-    /*
-     * disable migration if we haven't done already.
-     * attach could get called multiple times for the same export.
-     */
-    if (!s->migration_blocker) {
-        s->root_fid = fid;
-        error_set(&s->migration_blocker, QERR_VIRTFS_FEATURE_BLOCKS_MIGRATION,
-                  s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
-        migrate_add_blocker(s->migration_blocker);
-    }
 out:
     put_fid(pdu, fidp);
 out_nofid:
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 4d16d46..94d6f76 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -223,8 +223,6 @@ typedef struct V9fsState
      * on rename.
      */
     CoRwlock rename_lock;
-    int32_t root_fid;
-    Error *migration_blocker;
 } V9fsState;
 
 typedef struct V9fsStatState {
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH 1/4] migration: Create the pre migration flush hook infrastructure.

[Peter Maydell]

On 11 April 2013 13:14, Benoît Canet <benoit@irqsave.net> wrote:
> --- /dev/null
> +++ b/include/migration/migration-flush-hooks.h
> @@ -0,0 +1,30 @@
> +/*
> + * QEMU live pre migration flush hooks
> + *
> + * Copyright Nodalink, SARL. 2013
> + *
> + * Authors:
> + *  Benoît Canet <benoit@irqsave.net>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef QEMU_MIGRATION_FLUSH_HOOKS_H
> +#define QEMU_MIGRATION_FLUSH_HOOKS_H
> +
> +#include "qemu/queue.h"
> +
> +typedef struct MigrationFlushHookEntry {
> +    void (*flush_hook)(void);
> +    QTAILQ_ENTRY(MigrationFlushHookEntry) node;
> +} MigrationFlushHookEntry;
> +
> +void init_migration_flush_hooks(void);
> +
> +void register_migration_flush_hook(void (*fn)(void));
> +
> +void exec_migration_flush_hooks(void);

As this is a new public (ie exposed to the rest of QEMU) API
please could you provide documentation comments for these
functions?

thanks
-- PMM

Re: [Qemu-devel] [PATCH 4/4] virtio-9p: Remove migration blockers.

[Peter Maydell]

On 11 April 2013 13:14, Benoît Canet <benoit@irqsave.net> wrote:
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>  hw/9pfs/virtio-9p-device.c |    2 --
>  hw/9pfs/virtio-9p.c        |   23 -----------------------
>  hw/9pfs/virtio-9p.h        |    2 --
>  3 files changed, 27 deletions(-)
>
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 02b8630..f91a9b8 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -86,7 +86,6 @@ static void virtio_9p_save(QEMUFile *f, void *opaque)
>
>      qemu_put_be32(f, s->proto_version);
>      qemu_put_be32(f, s->msize);
> -    qemu_put_be32(f, s->root_fid);

Why do these need to be added in an earlier patch and then
taken out again here?


-- PMM

Re: [Qemu-devel] [PATCH 1/4] migration: Create the pre migration flush hook infrastructure.

[Paolo Bonzini]

Il 11/04/2013 14:14, Benoît Canet ha scritto:
> diff --git a/include/migration/migration-flush-hooks.h b/include/migration/migration-flush-hooks.h
> new file mode 100644
> index 0000000..be9e597
> --- /dev/null
> +++ b/include/migration/migration-flush-hooks.h
> @@ -0,0 +1,30 @@
> +/*
> + * QEMU live pre migration flush hooks
> + *
> + * Copyright Nodalink, SARL. 2013
> + *
> + * Authors:
> + *  Benoît Canet <benoit@irqsave.net>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef QEMU_MIGRATION_FLUSH_HOOKS_H
> +#define QEMU_MIGRATION_FLUSH_HOOKS_H
> +
> +#include "qemu/queue.h"
> +
> +typedef struct MigrationFlushHookEntry {
> +    void (*flush_hook)(void);
> +    QTAILQ_ENTRY(MigrationFlushHookEntry) node;
> +} MigrationFlushHookEntry;
> +
> +void init_migration_flush_hooks(void);
> +
> +void register_migration_flush_hook(void (*fn)(void));
> +
> +void exec_migration_flush_hooks(void);

Note that the point where you execute this (do_vm_stop) is not just for
migration.

So, can you just use a VMState change notifier?

If not, please make this a Notifier instead of using your own data
structure.

Paolo

> +#endif

Re: [Qemu-devel] [PATCH 3/4] virtio-9p: Wait for 9p operations to complete before migration.

[Paolo Bonzini]

Il 11/04/2013 14:14, Benoît Canet ha scritto:
> The function used to wait is registered as a pre migration flush hook.
> 
> The completion status is put in the virtio ring buffer which
> will be send to the guest on resume by the viring migration code
> 
> This patch is a rewrite from the one written by Aneesh Kumar.
> 
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
>  hw/9pfs/virtio-9p-device.c |    2 ++
>  hw/9pfs/virtio-9p.c        |   70 ++++++++++++++++++++++++++++++++++++++++++++
>  hw/9pfs/virtio-9p.h        |    2 ++
>  3 files changed, 74 insertions(+)
> 
> diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> index 7d4aefd..02b8630 100644
> --- a/hw/9pfs/virtio-9p-device.c
> +++ b/hw/9pfs/virtio-9p-device.c
> @@ -200,6 +200,8 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
>      V9fsPath path;
>      static int virtio_9p_id;
>  
> +    v9fs_init_migration_helper();
> +
>      s = (V9fsState *)virtio_common_init("virtio-9p",
>                                      VIRTIO_ID_9P,
>                                      sizeof(struct virtio_9p_config)+
> diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
> index 5cc4c92..3bf3ee4 100644
> --- a/hw/9pfs/virtio-9p.c
> +++ b/hw/9pfs/virtio-9p.c
> @@ -20,11 +20,20 @@
>  #include "virtio-9p-coth.h"
>  #include "trace.h"
>  #include "migration/migration.h"
> +#include "migration/migration-flush-hooks.h"
>  
>  int open_fd_hw;
>  int total_open_fd;
>  static int open_fd_rc;
>  
> +typedef struct MigrationHelper {
> +    int32_t pending_requests;
> +    QemuCond complete;
> +    QemuMutex mutex;
> +} MigrationHelper;
> +
> +MigrationHelper *migration;

I say, just use three variables instead of a struct.

Paolo

>  enum {
>      Oread   = 0x00,
>      Owrite  = 0x01,
> @@ -37,6 +46,62 @@ enum {
>      Oappend = 0x80,
>  };
>  
> +static void v9fs_wait_for_requests_drain(void)
> +{
> +    if (!migration) {
> +        return;
> +    }
> +
> +    qemu_mutex_lock(&migration->mutex);
> +    while (migration->pending_requests) {
> +        /* At this point ticks and vcpus will be stopped so we can safely
> +         * release the BQL so pending 9p callbacks will be executed and the
> +         * condition signaled.
> +         */
> +        qemu_mutex_unlock_iothread();
> +        qemu_cond_wait(&migration->complete, &migration->mutex);
> +        qemu_mutex_lock_iothread();
> +    }
> +    qemu_mutex_unlock(&migration->mutex);
> +}
> +
> +void v9fs_init_migration_helper(void)
> +{
> +    if (migration) {
> +        return;
> +    }
> +
> +    migration = g_new0(MigrationHelper, 1);
> +    qemu_mutex_init(&migration->mutex);
> +    qemu_cond_init(&migration->complete);
> +    register_migration_flush_hook(v9fs_wait_for_requests_drain);
> +}
> +
> +static void v9fs_inc_pending_requests(void)
> +{
> +    if (!migration) {
> +        return;
> +    }
> +
> +    qemu_mutex_lock(&migration->mutex);
> +    migration->pending_requests++;
> +    qemu_mutex_unlock(&migration->mutex);
> +}
> +
> +static void v9fs_dec_pending_requests(void)
> +{
> +    if (!migration) {
> +        return;
> +    }
> +
> +    qemu_mutex_lock(&migration->mutex);
> +    migration->pending_requests--;
> +    if (!migration->pending_requests) {
> +        qemu_cond_signal(&migration->complete);
> +    }
> +    qemu_mutex_unlock(&migration->mutex);
> +}
> +
>  static int omode_to_uflags(int8_t mode)
>  {
>      int ret = 0;
> @@ -637,6 +702,8 @@ static void complete_pdu(V9fsState *s, V9fsPDU *pdu, ssize_t len)
>      qemu_co_queue_next(&pdu->complete);
>  
>      free_pdu(s, pdu);
> +
> +    v9fs_dec_pending_requests();
>  }
>  
>  static mode_t v9mode_to_mode(uint32_t mode, V9fsString *extension)
> @@ -3240,6 +3307,9 @@ static void submit_pdu(V9fsState *s, V9fsPDU *pdu)
>      if (is_ro_export(&s->ctx) && !is_read_only_op(pdu)) {
>          handler = v9fs_fs_ro;
>      }
> +
> +    v9fs_inc_pending_requests();
> +
>      co = qemu_coroutine_create(handler);
>      qemu_coroutine_enter(co, pdu);
>  }
> diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
> index 52b1c69..4d16d46 100644
> --- a/hw/9pfs/virtio-9p.h
> +++ b/hw/9pfs/virtio-9p.h
> @@ -401,4 +401,6 @@ extern int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath,
>  #define pdu_unmarshal(pdu, offset, fmt, args...)  \
>      v9fs_unmarshal(pdu->elem.out_sg, pdu->elem.out_num, offset, 1, fmt, ##args)
>  
> +void v9fs_init_migration_helper(void);
> +
>  #endif
> 

Re: [Qemu-devel] [PATCH 3/4] virtio-9p: Wait for 9p operations to complete before migration.

[Peter Maydell]

On 11 April 2013 13:38, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 11/04/2013 14:14, Benoīt Canet ha scritto:
>>  int open_fd_hw;
>>  int total_open_fd;
>>  static int open_fd_rc;
>>
>> +typedef struct MigrationHelper {
>> +    int32_t pending_requests;
>> +    QemuCond complete;
>> +    QemuMutex mutex;
>> +} MigrationHelper;
>> +
>> +MigrationHelper *migration;
>
> I say, just use three variables instead of a struct.

They could all be marked 'static' for file local
scoping as well, right?

-- PMM