[Qemu-devel] [PATCH v2 0/4] sheepdog: support online snapshot from qemu-img

[Qemu-devel] [PATCH v2 0/4] sheepdog: support online snapshot from qemu-img

[MORITA Kazutaka]

v2:
 - rename reload_vdi_obj to reload_inode and use it from
   sd_snapshot_goto (Yuan)
 - use g_free instead of free (Stefan)
 - fix typo in the comment (Stefan)
 - add coroutine_fn marker to resend_aioreq

Currently, we can take sheepdog snapshots of running VMs only from the
qemu monitor.  This series allows taking online snapshots from
qemu-img.

The first two patches prepare for the thrid patch.

MORITA Kazutaka (4):
  sheepdog: cleanup find_vdi_name
  sheepdog: add SD_RES_READONLY result code
  sheepdog: add helper function to reload inode
  sheepdog: resend write requests when SD_RES_READONLY is received

 block/sheepdog.c |  142 ++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 107 insertions(+), 35 deletions(-)

-- 
1.7.2.5

[Qemu-devel] [PATCH v2 1/4] sheepdog: cleanup find_vdi_name

[MORITA Kazutaka]

This makes 'filename' and 'tag' constant variables, and renames
'for_snapshot' to 'lock' to clear how it works.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block/sheepdog.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 9f30a87..4326664 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -941,8 +941,9 @@ static int parse_vdiname(BDRVSheepdogState *s, const char *filename,
     return ret;
 }
 
-static int find_vdi_name(BDRVSheepdogState *s, char *filename, uint32_t snapid,
-                         char *tag, uint32_t *vid, int for_snapshot)
+static int find_vdi_name(BDRVSheepdogState *s, const char *filename,
+                         uint32_t snapid, const char *tag, uint32_t *vid,
+                         bool lock)
 {
     int ret, fd;
     SheepdogVdiReq hdr;
@@ -963,10 +964,10 @@ static int find_vdi_name(BDRVSheepdogState *s, char *filename, uint32_t snapid,
     strncpy(buf + SD_MAX_VDI_LEN, tag, SD_MAX_VDI_TAG_LEN);
 
     memset(&hdr, 0, sizeof(hdr));
-    if (for_snapshot) {
-        hdr.opcode = SD_OP_GET_VDI_INFO;
-    } else {
+    if (lock) {
         hdr.opcode = SD_OP_LOCK_VDI;
+    } else {
+        hdr.opcode = SD_OP_GET_VDI_INFO;
     }
     wlen = SD_MAX_VDI_LEN + SD_MAX_VDI_TAG_LEN;
     hdr.proto_ver = SD_PROTO_VER;
@@ -1205,7 +1206,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags)
         goto out;
     }
 
-    ret = find_vdi_name(s, vdi, snapid, tag, &vid, 0);
+    ret = find_vdi_name(s, vdi, snapid, tag, &vid, true);
     if (ret) {
         goto out;
     }
@@ -1921,7 +1922,7 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)
         pstrcpy(tag, sizeof(tag), s->name);
     }
 
-    ret = find_vdi_name(s, vdi, snapid, tag, &vid, 1);
+    ret = find_vdi_name(s, vdi, snapid, tag, &vid, false);
     if (ret) {
         error_report("Failed to find_vdi_name");
         goto out;
-- 
1.7.2.5

[Qemu-devel] [PATCH v2 2/4] sheepdog: add SD_RES_READONLY result code

[MORITA Kazutaka]

Sheepdog returns SD_RES_READONLY when qemu sends write requests to the
snapshot vdi.  This adds the result code and makes sd_strerror() print
its error reason.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block/sheepdog.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 4326664..f4e7204 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -68,6 +68,7 @@
 #define SD_RES_WAIT_FOR_JOIN    0x17 /* Waiting for other nodes joining */
 #define SD_RES_JOIN_FAILED   0x18 /* Target node had failed to join sheepdog */
 #define SD_RES_HALT          0x19 /* Sheepdog is stopped serving IO request */
+#define SD_RES_READONLY      0x1A /* Object is read-only */
 
 /*
  * Object ID rules
@@ -349,6 +350,7 @@ static const char * sd_strerror(int err)
         {SD_RES_WAIT_FOR_JOIN, "Sheepdog is waiting for other nodes joining"},
         {SD_RES_JOIN_FAILED, "Target node had failed to join sheepdog"},
         {SD_RES_HALT, "Sheepdog is stopped serving IO request"},
+        {SD_RES_READONLY, "Object is read-only"},
     };
 
     for (i = 0; i < ARRAY_SIZE(errors); ++i) {
-- 
1.7.2.5

[Qemu-devel] [PATCH v2 3/4] sheepdog: add helper function to reload inode

[MORITA Kazutaka]

This adds a helper function to update the current inode state with the
specified vdi object.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block/sheepdog.c |   67 +++++++++++++++++++++++++++++++----------------------
 1 files changed, 39 insertions(+), 28 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index f4e7204..0eaf4c3 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1150,6 +1150,42 @@ static int write_object(int fd, char *buf, uint64_t oid, int copies,
                              create, cache_flags);
 }
 
+/* update inode with the latest state */
+static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag)
+{
+    SheepdogInode *inode;
+    int ret = 0, fd;
+    uint32_t vid = 0;
+
+    fd = connect_to_sdog(s);
+    if (fd < 0) {
+        return -EIO;
+    }
+
+    inode = g_malloc(sizeof(s->inode));
+
+    ret = find_vdi_name(s, s->name, snapid, tag, &vid, false);
+    if (ret) {
+        goto out;
+    }
+
+    ret = read_object(fd, (char *)inode, vid_to_vdi_oid(vid),
+                      s->inode.nr_copies, sizeof(*inode), 0, s->cache_flags);
+    if (ret < 0) {
+        goto out;
+    }
+
+    if (inode->vdi_id != s->inode.vdi_id) {
+        memcpy(&s->inode, inode, sizeof(s->inode));
+    }
+
+out:
+    g_free(inode);
+    closesocket(fd);
+
+    return ret;
+}
+
 /* TODO Convert to fine grained options */
 static QemuOptsList runtime_opts = {
     .name = "sheepdog",
@@ -1905,18 +1941,14 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)
 {
     BDRVSheepdogState *s = bs->opaque;
     BDRVSheepdogState *old_s;
-    char vdi[SD_MAX_VDI_LEN], tag[SD_MAX_VDI_TAG_LEN];
-    char *buf = NULL;
-    uint32_t vid;
+    char tag[SD_MAX_VDI_TAG_LEN];
     uint32_t snapid = 0;
-    int ret = 0, fd;
+    int ret = 0;
 
     old_s = g_malloc(sizeof(BDRVSheepdogState));
 
     memcpy(old_s, s, sizeof(BDRVSheepdogState));
 
-    pstrcpy(vdi, sizeof(vdi), s->name);
-
     snapid = strtoul(snapshot_id, NULL, 10);
     if (snapid) {
         tag[0] = 0;
@@ -1924,30 +1956,11 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)
         pstrcpy(tag, sizeof(tag), s->name);
     }
 
-    ret = find_vdi_name(s, vdi, snapid, tag, &vid, false);
+    ret = reload_inode(s, snapid, tag);
     if (ret) {
-        error_report("Failed to find_vdi_name");
         goto out;
     }
 
-    fd = connect_to_sdog(s);
-    if (fd < 0) {
-        ret = fd;
-        goto out;
-    }
-
-    buf = g_malloc(SD_INODE_SIZE);
-    ret = read_object(fd, buf, vid_to_vdi_oid(vid), s->inode.nr_copies,
-                      SD_INODE_SIZE, 0, s->cache_flags);
-
-    closesocket(fd);
-
-    if (ret) {
-        goto out;
-    }
-
-    memcpy(&s->inode, buf, sizeof(s->inode));
-
     if (!s->inode.vm_state_size) {
         error_report("Invalid snapshot");
         ret = -ENOENT;
@@ -1956,14 +1969,12 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)
 
     s->is_snapshot = true;
 
-    g_free(buf);
     g_free(old_s);
 
     return 0;
 out:
     /* recover bdrv_sd_state */
     memcpy(s, old_s, sizeof(BDRVSheepdogState));
-    g_free(buf);
     g_free(old_s);
 
     error_report("failed to open. recover old bdrv_sd_state.");
-- 
1.7.2.5

[Qemu-devel] [PATCH v2 4/4] sheepdog: resend write requests when SD_RES_READONLY is received

[MORITA Kazutaka]

When a snapshot is taken from out side of qemu (e.g. qemu-img
snapshot), write requests to the current vdi return SD_RES_READONLY.
In this case, the sheepdog block driver needs to update the current
inode to the latest one and resend the write requests.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block/sheepdog.c |   60 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 59 insertions(+), 1 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 0eaf4c3..77e21fd 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -605,6 +605,7 @@ static int do_req(int sockfd, SheepdogReq *hdr, void *data,
 static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
                            struct iovec *iov, int niov, bool create,
                            enum AIOCBState aiocb_type);
+static int coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req);
 
 
 static AIOReq *find_pending_req(BDRVSheepdogState *s, uint64_t oid)
@@ -749,9 +750,19 @@ static void coroutine_fn aio_read_response(void *opaque)
         }
     }
 
-    if (rsp.result != SD_RES_SUCCESS) {
+    switch (rsp.result) {
+    case SD_RES_SUCCESS:
+        break;
+    case SD_RES_READONLY:
+        ret = resend_aioreq(s, aio_req);
+        if (ret == SD_RES_SUCCESS) {
+            goto out;
+        }
+        /* fall through */
+    default:
         acb->ret = -EIO;
         error_report("%s", sd_strerror(rsp.result));
+        break;
     }
 
     free_aio_req(s, aio_req);
@@ -1186,6 +1197,53 @@ out:
     return ret;
 }
 
+static int coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
+{
+    SheepdogAIOCB *acb = aio_req->aiocb;
+    bool create = false;
+    int ret;
+
+    ret = reload_inode(s, 0, "");
+    if (ret < 0) {
+        return ret;
+    }
+
+    aio_req->oid = vid_to_data_oid(s->inode.vdi_id,
+                                   data_oid_to_idx(aio_req->oid));
+
+    /* check whether this request becomes a CoW one */
+    if (acb->aiocb_type == AIOCB_WRITE_UDATA) {
+        int idx = data_oid_to_idx(aio_req->oid);
+        AIOReq *areq;
+
+        if (s->inode.data_vdi_id[idx] == 0) {
+            create = true;
+            goto out;
+        }
+        if (is_data_obj_writable(&s->inode, idx)) {
+            goto out;
+        }
+
+        /* link to the pending list if there is another CoW request to
+         * the same object */
+        QLIST_FOREACH(areq, &s->inflight_aio_head, aio_siblings) {
+            if (areq != aio_req && areq->oid == aio_req->oid) {
+                dprintf("simultaneous CoW to %" PRIx64 "\n", aio_req->oid);
+                QLIST_REMOVE(aio_req, aio_siblings);
+                QLIST_INSERT_HEAD(&s->pending_aio_head, aio_req, aio_siblings);
+                return SD_RES_SUCCESS;
+            }
+        }
+
+        aio_req->base_oid = vid_to_data_oid(s->inode.data_vdi_id[idx], idx);
+        aio_req->flags |= SD_FLAG_CMD_COW;
+        create = true;
+    }
+out:
+    return add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
+                           create, acb->aiocb_type);
+}
+
 /* TODO Convert to fine grained options */
 static QemuOptsList runtime_opts = {
     .name = "sheepdog",
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH v2 0/4] sheepdog: support online snapshot from qemu-img

[Stefan Hajnoczi]

On Fri, Apr 26, 2013 at 01:19:50AM +0900, MORITA Kazutaka wrote:
> v2:
>  - rename reload_vdi_obj to reload_inode and use it from
>    sd_snapshot_goto (Yuan)
>  - use g_free instead of free (Stefan)
>  - fix typo in the comment (Stefan)
>  - add coroutine_fn marker to resend_aioreq
> 
> Currently, we can take sheepdog snapshots of running VMs only from the
> qemu monitor.  This series allows taking online snapshots from
> qemu-img.
> 
> The first two patches prepare for the thrid patch.
> 
> MORITA Kazutaka (4):
>   sheepdog: cleanup find_vdi_name
>   sheepdog: add SD_RES_READONLY result code
>   sheepdog: add helper function to reload inode
>   sheepdog: resend write requests when SD_RES_READONLY is received
> 
>  block/sheepdog.c |  142 ++++++++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 107 insertions(+), 35 deletions(-)
> 
> -- 
> 1.7.2.5
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan