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

[Qemu-devel] [PATCH V2 0/3] 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.

v2:
    Drop pre migration flush hook code (Paolo)
    Use vmstate change notifier (Paolo)
    Use static variables instead of a struct in patch 2 (Paolo, Peter)
    Drop spurious root_id related code (Peter)
    Rename functions in patch two (Benoît)

Benoît Canet (3):
  virtio-9p: Add support for 9p migration.
  virtio-9p: Wait for 9p operations to complete before migration and
    savevm.
  virtio-9p: Remove migration blockers.

 hw/9pfs/virtio-9p-device.c |  154 ++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p.c        |   98 +++++++++++++++++++++-------
 hw/9pfs/virtio-9p.h        |    4 +-
 3 files changed, 231 insertions(+), 25 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH V2 1/3] 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 |  152 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 152 insertions(+)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index d321c80..2af4858 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -20,6 +20,149 @@
 #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);
+}
+
+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);
+
+    return 0;
+}
+
 static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t features)
 {
     features |= 1 << VIRTIO_9P_MOUNT_TAG;
@@ -53,6 +196,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 +246,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 V2 2/3] virtio-9p: Wait for 9p operations to complete before migration and savevm.

[Benoît Canet]

The completion status is put in the virtio ring buffer which
will be send to the guest on resume by the viring vmstate 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        |   75 ++++++++++++++++++++++++++++++++++++++++++++
 hw/9pfs/virtio-9p.h        |    2 ++
 3 files changed, 79 insertions(+)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 2af4858..feaccd3 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -198,6 +198,8 @@ VirtIODevice *virtio_9p_init(DeviceState *dev, V9fsConf *conf)
     V9fsPath path;
     static int virtio_9p_id;
 
+    v9fs_init_vmstate_io_drain();
+
     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..66322ee 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -25,6 +25,11 @@ int open_fd_hw;
 int total_open_fd;
 static int open_fd_rc;
 
+static int32_t pending_initialized;
+static int32_t pending_requests;
+static QemuCond pending_cond;
+static QemuMutex pending_mutex;
+
 enum {
     Oread   = 0x00,
     Owrite  = 0x01,
@@ -37,6 +42,71 @@ enum {
     Oappend = 0x80,
 };
 
+static void v9fs_vm_change_state_handler(void *opaque, int running,
+                                         RunState state)
+{
+    if (!pending_initialized) {
+        return;
+    }
+
+    if (running) {
+        return;
+    }
+
+    if (state != RUN_STATE_FINISH_MIGRATE && state != RUN_STATE_SAVE_VM) {
+        return;
+    }
+
+    qemu_mutex_lock(&pending_mutex);
+    while (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(&pending_cond, &pending_mutex);
+        qemu_mutex_lock_iothread();
+    }
+    qemu_mutex_unlock(&pending_mutex);
+}
+
+void v9fs_init_vmstate_io_drain(void)
+{
+    if (pending_initialized) {
+        return;
+    }
+
+    pending_initialized = 1;
+    qemu_mutex_init(&pending_mutex);
+    qemu_cond_init(&pending_cond);
+    qemu_add_vm_change_state_handler(v9fs_vm_change_state_handler, NULL);
+}
+
+static void v9fs_inc_pending_requests(void)
+{
+    if (!pending_initialized) {
+        return;
+    }
+
+    qemu_mutex_lock(&pending_mutex);
+    pending_requests++;
+    qemu_mutex_unlock(&pending_mutex);
+}
+
+static void v9fs_dec_pending_requests(void)
+{
+    if (!pending_initialized) {
+        return;
+    }
+
+    qemu_mutex_lock(&pending_mutex);
+    pending_requests--;
+    if (!pending_requests) {
+        qemu_cond_signal(&pending_cond);
+    }
+    qemu_mutex_unlock(&pending_mutex);
+}
+
 static int omode_to_uflags(int8_t mode)
 {
     int ret = 0;
@@ -637,6 +707,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 +3312,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..604502a 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_vmstate_io_drain(void);
+
 #endif
-- 
1.7.10.4

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

[Benoît Canet]

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

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index 66322ee..d81087b 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -404,19 +404,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;
@@ -1053,16 +1040,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 604502a..acd6703 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 V2 2/3] virtio-9p: Wait for 9p operations to complete before migration and savevm.

[Paolo Bonzini]

Il 11/04/2013 16:41, Benoît Canet ha scritto:
> The completion status is put in the virtio ring buffer which
> will be send to the guest on resume by the viring vmstate code
> 
> This patch is a rewrite from the one written by Aneesh Kumar.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH V2 0/3] Virtio-9p live migration patchset

[Benoît Canet]

quintela: ping.

I really need your review to get this patchset in shape for merging.

Regards

Benoît

> Le Thursday 11 Apr 2013 à 16:41:05 (+0200), Benoît Canet a écrit :
> 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.
> 
> v2:
>     Drop pre migration flush hook code (Paolo)
>     Use vmstate change notifier (Paolo)
>     Use static variables instead of a struct in patch 2 (Paolo, Peter)
>     Drop spurious root_id related code (Peter)
>     Rename functions in patch two (Benoît)
> 
> Benoît Canet (3):
>   virtio-9p: Add support for 9p migration.
>   virtio-9p: Wait for 9p operations to complete before migration and
>     savevm.
>   virtio-9p: Remove migration blockers.
> 
>  hw/9pfs/virtio-9p-device.c |  154 ++++++++++++++++++++++++++++++++++++++++++++
>  hw/9pfs/virtio-9p.c        |   98 +++++++++++++++++++++-------
>  hw/9pfs/virtio-9p.h        |    4 +-
>  3 files changed, 231 insertions(+), 25 deletions(-)
> 
> -- 
> 1.7.10.4
>