* [Qemu-devel] [PATCH v2 0/2] bugfixes of sheepdog driver for a case of live snapshot
@ 2014-06-03 4:54 Hitoshi Mitake
2014-06-03 4:54 ` [Qemu-devel] [PATCH v2 1/2] sheepdog: fix vdi object update after " Hitoshi Mitake
2014-06-03 4:54 ` [Qemu-devel] [PATCH v2 2/2] sheepdog: reload only header in a case of " Hitoshi Mitake
0 siblings, 2 replies; 6+ messages in thread
From: Hitoshi Mitake @ 2014-06-03 4:54 UTC (permalink / raw)
To: qemu-devel; +Cc: Hitoshi Mitake, sheepdog, mitake.hitoshi
Current sheepdog driver has two problems in a mechanism of inode
object reloading for live snapshot. It causes inconsistent state of
snapshot volumes. A new GC algorithm implemented in sheepdog exposes
the problems. This patchset contains bugfixes for them.
v2:
- corrrect wrong spelling in the commit log of 2nd patch
Hitoshi Mitake (2):
sheepdog: fix vdi object update after live snapshot
sheepdog: reload only header in a case of live snapshot
block/sheepdog.c | 49 +++++++++++++++++++++++++++++--------------------
1 files changed, 29 insertions(+), 20 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] sheepdog: fix vdi object update after live snapshot
2014-06-03 4:54 [Qemu-devel] [PATCH v2 0/2] bugfixes of sheepdog driver for a case of live snapshot Hitoshi Mitake
@ 2014-06-03 4:54 ` Hitoshi Mitake
2014-06-03 12:41 ` Liu Yuan
2014-06-03 4:54 ` [Qemu-devel] [PATCH v2 2/2] sheepdog: reload only header in a case of " Hitoshi Mitake
1 sibling, 1 reply; 6+ messages in thread
From: Hitoshi Mitake @ 2014-06-03 4:54 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, sheepdog, mitake.hitoshi, Hitoshi Mitake,
Stefan Hajnoczi, Liu Yuan, MORITA Kazutaka
sheepdog driver should decide a write request is COW or not based on
inode object which is active when the write request is issued.
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Liu Yuan <namei.unix@gmail.com>
Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
---
block/sheepdog.c | 40 +++++++++++++++++++++++-----------------
1 files changed, 23 insertions(+), 17 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 4ecbf5f..637e57f 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -282,6 +282,7 @@ typedef struct AIOReq {
unsigned int data_len;
uint8_t flags;
uint32_t id;
+ bool create;
QLIST_ENTRY(AIOReq) aio_siblings;
} AIOReq;
@@ -404,7 +405,7 @@ static const char * sd_strerror(int err)
static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
uint64_t oid, unsigned int data_len,
- uint64_t offset, uint8_t flags,
+ uint64_t offset, uint8_t flags, bool create,
uint64_t base_oid, unsigned int iov_offset)
{
AIOReq *aio_req;
@@ -418,6 +419,7 @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
aio_req->data_len = data_len;
aio_req->flags = flags;
aio_req->id = s->aioreq_seq_num++;
+ aio_req->create = create;
acb->nr_pending++;
return aio_req;
@@ -664,8 +666,8 @@ static int do_req(int sockfd, SheepdogReq *hdr, void *data,
}
static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
- struct iovec *iov, int niov, bool create,
- enum AIOCBState aiocb_type);
+ struct iovec *iov, int niov,
+ enum AIOCBState aiocb_type);
static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req);
static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag);
static int get_sheep_fd(BDRVSheepdogState *s, Error **errp);
@@ -698,7 +700,7 @@ static void coroutine_fn send_pending_req(BDRVSheepdogState *s, uint64_t oid)
/* move aio_req from pending list to inflight one */
QLIST_REMOVE(aio_req, aio_siblings);
QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
- add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, false,
+ add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
acb->aiocb_type);
}
}
@@ -797,7 +799,7 @@ static void coroutine_fn aio_read_response(void *opaque)
}
idx = data_oid_to_idx(aio_req->oid);
- if (s->inode.data_vdi_id[idx] != s->inode.vdi_id) {
+ if (aio_req->create) {
/*
* If the object is newly created one, we need to update
* the vdi object (metadata object). min_dirty_data_idx
@@ -1117,8 +1119,8 @@ out:
}
static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
- struct iovec *iov, int niov, bool create,
- enum AIOCBState aiocb_type)
+ struct iovec *iov, int niov,
+ enum AIOCBState aiocb_type)
{
int nr_copies = s->inode.nr_copies;
SheepdogObjReq hdr;
@@ -1129,6 +1131,7 @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
uint64_t offset = aio_req->offset;
uint8_t flags = aio_req->flags;
uint64_t old_oid = aio_req->base_oid;
+ bool create = aio_req->create;
if (!nr_copies) {
error_report("bug");
@@ -1315,6 +1318,7 @@ static bool check_simultaneous_create(BDRVSheepdogState *s, AIOReq *aio_req)
DPRINTF("simultaneous create to %" PRIx64 "\n", aio_req->oid);
aio_req->flags = 0;
aio_req->base_oid = 0;
+ aio_req->create = false;
QLIST_REMOVE(aio_req, aio_siblings);
QLIST_INSERT_HEAD(&s->pending_aio_head, aio_req, aio_siblings);
return true;
@@ -1327,7 +1331,8 @@ static bool check_simultaneous_create(BDRVSheepdogState *s, AIOReq *aio_req)
static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
{
SheepdogAIOCB *acb = aio_req->aiocb;
- bool create = false;
+
+ aio_req->create = false;
/* check whether this request becomes a CoW one */
if (acb->aiocb_type == AIOCB_WRITE_UDATA && is_data_obj(aio_req->oid)) {
@@ -1345,17 +1350,17 @@ static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
aio_req->base_oid = vid_to_data_oid(s->inode.data_vdi_id[idx], idx);
aio_req->flags |= SD_FLAG_CMD_COW;
}
- create = true;
+ aio_req->create = true;
}
out:
if (is_data_obj(aio_req->oid)) {
- add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, create,
+ add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
acb->aiocb_type);
} else {
struct iovec iov;
iov.iov_base = &s->inode;
iov.iov_len = sizeof(s->inode);
- add_aio_request(s, aio_req, &iov, 1, false, AIOCB_WRITE_UDATA);
+ add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
}
}
@@ -1849,9 +1854,9 @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
iov.iov_base = &s->inode;
iov.iov_len = sizeof(s->inode);
aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
- data_len, offset, 0, 0, offset);
+ data_len, offset, 0, false, 0, offset);
QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
- add_aio_request(s, aio_req, &iov, 1, false, AIOCB_WRITE_UDATA);
+ add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
acb->aio_done_func = sd_finish_aiocb;
acb->aiocb_type = AIOCB_WRITE_UDATA;
@@ -2049,7 +2054,8 @@ static int coroutine_fn sd_co_rw_vector(void *p)
DPRINTF("new oid %" PRIx64 "\n", oid);
}
- aio_req = alloc_aio_req(s, acb, oid, len, offset, flags, old_oid, done);
+ aio_req = alloc_aio_req(s, acb, oid, len, offset, flags, create,
+ old_oid, done);
QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
if (create) {
@@ -2058,7 +2064,7 @@ static int coroutine_fn sd_co_rw_vector(void *p)
}
}
- add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, create,
+ add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
acb->aiocb_type);
done:
offset = 0;
@@ -2138,9 +2144,9 @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
acb->aio_done_func = sd_finish_aiocb;
aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
- 0, 0, 0, 0, 0);
+ 0, 0, 0, false, 0, 0);
QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
- add_aio_request(s, aio_req, NULL, 0, false, acb->aiocb_type);
+ add_aio_request(s, aio_req, NULL, 0, acb->aiocb_type);
qemu_coroutine_yield();
return acb->ret;
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] sheepdog: reload only header in a case of live snapshot
2014-06-03 4:54 [Qemu-devel] [PATCH v2 0/2] bugfixes of sheepdog driver for a case of live snapshot Hitoshi Mitake
2014-06-03 4:54 ` [Qemu-devel] [PATCH v2 1/2] sheepdog: fix vdi object update after " Hitoshi Mitake
@ 2014-06-03 4:54 ` Hitoshi Mitake
1 sibling, 0 replies; 6+ messages in thread
From: Hitoshi Mitake @ 2014-06-03 4:54 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, sheepdog, mitake.hitoshi, Hitoshi Mitake,
Stefan Hajnoczi, Liu Yuan, MORITA Kazutaka
sheepdog driver doesn't need to read data_vdi_id[] when a live
snapshot is created.
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Liu Yuan <namei.unix@gmail.com>
Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
---
v2:
- correct wrong spelling in the commit log
block/sheepdog.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 637e57f..bb38e6b 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -200,6 +200,8 @@ typedef struct SheepdogInode {
uint32_t data_vdi_id[MAX_DATA_OBJS];
} SheepdogInode;
+#define SD_INODE_HEADER_SIZE offsetof(SheepdogInode, data_vdi_id)
+
/*
* 64 bit FNV-1a non-zero initial basis
*/
@@ -1278,7 +1280,7 @@ static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag)
return -EIO;
}
- inode = g_malloc(sizeof(s->inode));
+ inode = g_malloc(SD_INODE_HEADER_SIZE);
ret = find_vdi_name(s, s->name, snapid, tag, &vid, false, &local_err);
if (ret) {
@@ -1288,13 +1290,14 @@ static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag)
}
ret = read_object(fd, (char *)inode, vid_to_vdi_oid(vid),
- s->inode.nr_copies, sizeof(*inode), 0, s->cache_flags);
+ s->inode.nr_copies, SD_INODE_HEADER_SIZE, 0,
+ s->cache_flags);
if (ret < 0) {
goto out;
}
if (inode->vdi_id != s->inode.vdi_id) {
- memcpy(&s->inode, inode, sizeof(s->inode));
+ memcpy(&s->inode, inode, SD_INODE_HEADER_SIZE);
}
out:
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] sheepdog: fix vdi object update after live snapshot
2014-06-03 4:54 ` [Qemu-devel] [PATCH v2 1/2] sheepdog: fix vdi object update after " Hitoshi Mitake
@ 2014-06-03 12:41 ` Liu Yuan
2014-06-03 14:58 ` Hitoshi Mitake
0 siblings, 1 reply; 6+ messages in thread
From: Liu Yuan @ 2014-06-03 12:41 UTC (permalink / raw)
To: Hitoshi Mitake
Cc: Kevin Wolf, sheepdog, mitake.hitoshi, qemu-devel, Stefan Hajnoczi,
MORITA Kazutaka
On Tue, Jun 03, 2014 at 01:54:21PM +0900, Hitoshi Mitake wrote:
> sheepdog driver should decide a write request is COW or not based on
> inode object which is active when the write request is issued.
>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Liu Yuan <namei.unix@gmail.com>
> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
> Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
> ---
> block/sheepdog.c | 40 +++++++++++++++++++++++-----------------
> 1 files changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 4ecbf5f..637e57f 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -282,6 +282,7 @@ typedef struct AIOReq {
> unsigned int data_len;
> uint8_t flags;
> uint32_t id;
> + bool create;
>
> QLIST_ENTRY(AIOReq) aio_siblings;
> } AIOReq;
> @@ -404,7 +405,7 @@ static const char * sd_strerror(int err)
>
> static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
> uint64_t oid, unsigned int data_len,
> - uint64_t offset, uint8_t flags,
> + uint64_t offset, uint8_t flags, bool create,
> uint64_t base_oid, unsigned int iov_offset)
> {
> AIOReq *aio_req;
> @@ -418,6 +419,7 @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
> aio_req->data_len = data_len;
> aio_req->flags = flags;
> aio_req->id = s->aioreq_seq_num++;
> + aio_req->create = create;
>
> acb->nr_pending++;
> return aio_req;
> @@ -664,8 +666,8 @@ static int do_req(int sockfd, SheepdogReq *hdr, void *data,
> }
>
> static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
> - struct iovec *iov, int niov, bool create,
> - enum AIOCBState aiocb_type);
> + struct iovec *iov, int niov,
> + enum AIOCBState aiocb_type);
> static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req);
> static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag);
> static int get_sheep_fd(BDRVSheepdogState *s, Error **errp);
> @@ -698,7 +700,7 @@ static void coroutine_fn send_pending_req(BDRVSheepdogState *s, uint64_t oid)
> /* move aio_req from pending list to inflight one */
> QLIST_REMOVE(aio_req, aio_siblings);
> QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
> - add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, false,
> + add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
> acb->aiocb_type);
> }
> }
> @@ -797,7 +799,7 @@ static void coroutine_fn aio_read_response(void *opaque)
> }
> idx = data_oid_to_idx(aio_req->oid);
>
> - if (s->inode.data_vdi_id[idx] != s->inode.vdi_id) {
> + if (aio_req->create) {
> /*
> * If the object is newly created one, we need to update
> * the vdi object (metadata object). min_dirty_data_idx
> @@ -1117,8 +1119,8 @@ out:
> }
>
> static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
> - struct iovec *iov, int niov, bool create,
> - enum AIOCBState aiocb_type)
> + struct iovec *iov, int niov,
> + enum AIOCBState aiocb_type)
> {
> int nr_copies = s->inode.nr_copies;
> SheepdogObjReq hdr;
> @@ -1129,6 +1131,7 @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
> uint64_t offset = aio_req->offset;
> uint8_t flags = aio_req->flags;
> uint64_t old_oid = aio_req->base_oid;
> + bool create = aio_req->create;
>
> if (!nr_copies) {
> error_report("bug");
> @@ -1315,6 +1318,7 @@ static bool check_simultaneous_create(BDRVSheepdogState *s, AIOReq *aio_req)
> DPRINTF("simultaneous create to %" PRIx64 "\n", aio_req->oid);
> aio_req->flags = 0;
> aio_req->base_oid = 0;
> + aio_req->create = false;
> QLIST_REMOVE(aio_req, aio_siblings);
> QLIST_INSERT_HEAD(&s->pending_aio_head, aio_req, aio_siblings);
> return true;
> @@ -1327,7 +1331,8 @@ static bool check_simultaneous_create(BDRVSheepdogState *s, AIOReq *aio_req)
> static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
> {
> SheepdogAIOCB *acb = aio_req->aiocb;
> - bool create = false;
> +
> + aio_req->create = false;
>
> /* check whether this request becomes a CoW one */
> if (acb->aiocb_type == AIOCB_WRITE_UDATA && is_data_obj(aio_req->oid)) {
> @@ -1345,17 +1350,17 @@ static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
> aio_req->base_oid = vid_to_data_oid(s->inode.data_vdi_id[idx], idx);
> aio_req->flags |= SD_FLAG_CMD_COW;
> }
> - create = true;
> + aio_req->create = true;
> }
> out:
> if (is_data_obj(aio_req->oid)) {
> - add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, create,
> + add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
> acb->aiocb_type);
> } else {
> struct iovec iov;
> iov.iov_base = &s->inode;
> iov.iov_len = sizeof(s->inode);
> - add_aio_request(s, aio_req, &iov, 1, false, AIOCB_WRITE_UDATA);
> + add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
> }
> }
>
> @@ -1849,9 +1854,9 @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
> iov.iov_base = &s->inode;
> iov.iov_len = sizeof(s->inode);
> aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
> - data_len, offset, 0, 0, offset);
> + data_len, offset, 0, false, 0, offset);
> QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
> - add_aio_request(s, aio_req, &iov, 1, false, AIOCB_WRITE_UDATA);
> + add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
>
> acb->aio_done_func = sd_finish_aiocb;
> acb->aiocb_type = AIOCB_WRITE_UDATA;
> @@ -2049,7 +2054,8 @@ static int coroutine_fn sd_co_rw_vector(void *p)
> DPRINTF("new oid %" PRIx64 "\n", oid);
> }
>
> - aio_req = alloc_aio_req(s, acb, oid, len, offset, flags, old_oid, done);
> + aio_req = alloc_aio_req(s, acb, oid, len, offset, flags, create,
> + old_oid, done);
> QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
>
> if (create) {
> @@ -2058,7 +2064,7 @@ static int coroutine_fn sd_co_rw_vector(void *p)
> }
> }
>
> - add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, create,
> + add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
> acb->aiocb_type);
> done:
> offset = 0;
> @@ -2138,9 +2144,9 @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
> acb->aio_done_func = sd_finish_aiocb;
>
> aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
> - 0, 0, 0, 0, 0);
> + 0, 0, 0, false, 0, 0);
> QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
> - add_aio_request(s, aio_req, NULL, 0, false, acb->aiocb_type);
> + add_aio_request(s, aio_req, NULL, 0, acb->aiocb_type);
>
> qemu_coroutine_yield();
> return acb->ret;
> --
> 1.7.1
>
Which line is the problem and which line fixes it? Seems not easy to find it out.
I just saw some restructuring of 'create' field.
Thanks
Yuan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] sheepdog: fix vdi object update after live snapshot
2014-06-03 12:41 ` Liu Yuan
@ 2014-06-03 14:58 ` Hitoshi Mitake
2014-06-04 6:24 ` Liu Yuan
0 siblings, 1 reply; 6+ messages in thread
From: Hitoshi Mitake @ 2014-06-03 14:58 UTC (permalink / raw)
To: Liu Yuan
Cc: Kevin Wolf, sheepdog, Hitoshi Mitake, qemu-devel, Stefan Hajnoczi,
MORITA Kazutaka
On Tue, Jun 3, 2014 at 9:41 PM, Liu Yuan <namei.unix@gmail.com> wrote:
> On Tue, Jun 03, 2014 at 01:54:21PM +0900, Hitoshi Mitake wrote:
>> sheepdog driver should decide a write request is COW or not based on
>> inode object which is active when the write request is issued.
>>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> Cc: Liu Yuan <namei.unix@gmail.com>
>> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
>> Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
>> ---
>> block/sheepdog.c | 40 +++++++++++++++++++++++-----------------
>> 1 files changed, 23 insertions(+), 17 deletions(-)
>>
>> diff --git a/block/sheepdog.c b/block/sheepdog.c
>> index 4ecbf5f..637e57f 100644
>> --- a/block/sheepdog.c
>> +++ b/block/sheepdog.c
>> @@ -282,6 +282,7 @@ typedef struct AIOReq {
>> unsigned int data_len;
>> uint8_t flags;
>> uint32_t id;
>> + bool create;
>>
>> QLIST_ENTRY(AIOReq) aio_siblings;
>> } AIOReq;
>> @@ -404,7 +405,7 @@ static const char * sd_strerror(int err)
>>
>> static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
>> uint64_t oid, unsigned int data_len,
>> - uint64_t offset, uint8_t flags,
>> + uint64_t offset, uint8_t flags, bool create,
>> uint64_t base_oid, unsigned int iov_offset)
>> {
>> AIOReq *aio_req;
>> @@ -418,6 +419,7 @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
>> aio_req->data_len = data_len;
>> aio_req->flags = flags;
>> aio_req->id = s->aioreq_seq_num++;
>> + aio_req->create = create;
>>
>> acb->nr_pending++;
>> return aio_req;
>> @@ -664,8 +666,8 @@ static int do_req(int sockfd, SheepdogReq *hdr, void *data,
>> }
>>
>> static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
>> - struct iovec *iov, int niov, bool create,
>> - enum AIOCBState aiocb_type);
>> + struct iovec *iov, int niov,
>> + enum AIOCBState aiocb_type);
>> static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req);
>> static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag);
>> static int get_sheep_fd(BDRVSheepdogState *s, Error **errp);
>> @@ -698,7 +700,7 @@ static void coroutine_fn send_pending_req(BDRVSheepdogState *s, uint64_t oid)
>> /* move aio_req from pending list to inflight one */
>> QLIST_REMOVE(aio_req, aio_siblings);
>> QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
>> - add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, false,
>> + add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
>> acb->aiocb_type);
>> }
>> }
>> @@ -797,7 +799,7 @@ static void coroutine_fn aio_read_response(void *opaque)
>> }
>> idx = data_oid_to_idx(aio_req->oid);
>>
>> - if (s->inode.data_vdi_id[idx] != s->inode.vdi_id) {
>> + if (aio_req->create) {
>> /*
>> * If the object is newly created one, we need to update
>> * the vdi object (metadata object). min_dirty_data_idx
>> @@ -1117,8 +1119,8 @@ out:
>> }
>>
>> static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
>> - struct iovec *iov, int niov, bool create,
>> - enum AIOCBState aiocb_type)
>> + struct iovec *iov, int niov,
>> + enum AIOCBState aiocb_type)
>> {
>> int nr_copies = s->inode.nr_copies;
>> SheepdogObjReq hdr;
>> @@ -1129,6 +1131,7 @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
>> uint64_t offset = aio_req->offset;
>> uint8_t flags = aio_req->flags;
>> uint64_t old_oid = aio_req->base_oid;
>> + bool create = aio_req->create;
>>
>> if (!nr_copies) {
>> error_report("bug");
>> @@ -1315,6 +1318,7 @@ static bool check_simultaneous_create(BDRVSheepdogState *s, AIOReq *aio_req)
>> DPRINTF("simultaneous create to %" PRIx64 "\n", aio_req->oid);
>> aio_req->flags = 0;
>> aio_req->base_oid = 0;
>> + aio_req->create = false;
>> QLIST_REMOVE(aio_req, aio_siblings);
>> QLIST_INSERT_HEAD(&s->pending_aio_head, aio_req, aio_siblings);
>> return true;
>> @@ -1327,7 +1331,8 @@ static bool check_simultaneous_create(BDRVSheepdogState *s, AIOReq *aio_req)
>> static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
>> {
>> SheepdogAIOCB *acb = aio_req->aiocb;
>> - bool create = false;
>> +
>> + aio_req->create = false;
>>
>> /* check whether this request becomes a CoW one */
>> if (acb->aiocb_type == AIOCB_WRITE_UDATA && is_data_obj(aio_req->oid)) {
>> @@ -1345,17 +1350,17 @@ static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
>> aio_req->base_oid = vid_to_data_oid(s->inode.data_vdi_id[idx], idx);
>> aio_req->flags |= SD_FLAG_CMD_COW;
>> }
>> - create = true;
>> + aio_req->create = true;
>> }
>> out:
>> if (is_data_obj(aio_req->oid)) {
>> - add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, create,
>> + add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
>> acb->aiocb_type);
>> } else {
>> struct iovec iov;
>> iov.iov_base = &s->inode;
>> iov.iov_len = sizeof(s->inode);
>> - add_aio_request(s, aio_req, &iov, 1, false, AIOCB_WRITE_UDATA);
>> + add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
>> }
>> }
>>
>> @@ -1849,9 +1854,9 @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
>> iov.iov_base = &s->inode;
>> iov.iov_len = sizeof(s->inode);
>> aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
>> - data_len, offset, 0, 0, offset);
>> + data_len, offset, 0, false, 0, offset);
>> QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
>> - add_aio_request(s, aio_req, &iov, 1, false, AIOCB_WRITE_UDATA);
>> + add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
>>
>> acb->aio_done_func = sd_finish_aiocb;
>> acb->aiocb_type = AIOCB_WRITE_UDATA;
>> @@ -2049,7 +2054,8 @@ static int coroutine_fn sd_co_rw_vector(void *p)
>> DPRINTF("new oid %" PRIx64 "\n", oid);
>> }
>>
>> - aio_req = alloc_aio_req(s, acb, oid, len, offset, flags, old_oid, done);
>> + aio_req = alloc_aio_req(s, acb, oid, len, offset, flags, create,
>> + old_oid, done);
>> QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
>>
>> if (create) {
>> @@ -2058,7 +2064,7 @@ static int coroutine_fn sd_co_rw_vector(void *p)
>> }
>> }
>>
>> - add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, create,
>> + add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
>> acb->aiocb_type);
>> done:
>> offset = 0;
>> @@ -2138,9 +2144,9 @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
>> acb->aio_done_func = sd_finish_aiocb;
>>
>> aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
>> - 0, 0, 0, 0, 0);
>> + 0, 0, 0, false, 0, 0);
>> QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
>> - add_aio_request(s, aio_req, NULL, 0, false, acb->aiocb_type);
>> + add_aio_request(s, aio_req, NULL, 0, acb->aiocb_type);
>>
>> qemu_coroutine_yield();
>> return acb->ret;
>> --
>> 1.7.1
>>
>
> Which line is the problem and which line fixes it? Seems not easy to find it out.
> I just saw some restructuring of 'create' field.
>
The below line in aio_read_response() is the problem:
> @@ -797,7 +799,7 @@ static void coroutine_fn aio_read_response(void *opaque)
> }
> idx = data_oid_to_idx(aio_req->oid);
>
> - if (s->inode.data_vdi_id[idx] != s->inode.vdi_id) {
Thanks,
Hitoshi
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] sheepdog: fix vdi object update after live snapshot
2014-06-03 14:58 ` Hitoshi Mitake
@ 2014-06-04 6:24 ` Liu Yuan
0 siblings, 0 replies; 6+ messages in thread
From: Liu Yuan @ 2014-06-04 6:24 UTC (permalink / raw)
To: Hitoshi Mitake
Cc: Kevin Wolf, sheepdog, Hitoshi Mitake, qemu-devel, Stefan Hajnoczi,
MORITA Kazutaka
On Tue, Jun 03, 2014 at 11:58:21PM +0900, Hitoshi Mitake wrote:
> On Tue, Jun 3, 2014 at 9:41 PM, Liu Yuan <namei.unix@gmail.com> wrote:
> > On Tue, Jun 03, 2014 at 01:54:21PM +0900, Hitoshi Mitake wrote:
> >> sheepdog driver should decide a write request is COW or not based on
> >> inode object which is active when the write request is issued.
> >>
> >> Cc: Kevin Wolf <kwolf@redhat.com>
> >> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> >> Cc: Liu Yuan <namei.unix@gmail.com>
> >> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
> >> Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>
> >> ---
> >> block/sheepdog.c | 40 +++++++++++++++++++++++-----------------
> >> 1 files changed, 23 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/block/sheepdog.c b/block/sheepdog.c
> >> index 4ecbf5f..637e57f 100644
> >> --- a/block/sheepdog.c
> >> +++ b/block/sheepdog.c
> >> @@ -282,6 +282,7 @@ typedef struct AIOReq {
> >> unsigned int data_len;
> >> uint8_t flags;
> >> uint32_t id;
> >> + bool create;
> >>
> >> QLIST_ENTRY(AIOReq) aio_siblings;
> >> } AIOReq;
> >> @@ -404,7 +405,7 @@ static const char * sd_strerror(int err)
> >>
> >> static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
> >> uint64_t oid, unsigned int data_len,
> >> - uint64_t offset, uint8_t flags,
> >> + uint64_t offset, uint8_t flags, bool create,
> >> uint64_t base_oid, unsigned int iov_offset)
> >> {
> >> AIOReq *aio_req;
> >> @@ -418,6 +419,7 @@ static inline AIOReq *alloc_aio_req(BDRVSheepdogState *s, SheepdogAIOCB *acb,
> >> aio_req->data_len = data_len;
> >> aio_req->flags = flags;
> >> aio_req->id = s->aioreq_seq_num++;
> >> + aio_req->create = create;
> >>
> >> acb->nr_pending++;
> >> return aio_req;
> >> @@ -664,8 +666,8 @@ static int do_req(int sockfd, SheepdogReq *hdr, void *data,
> >> }
> >>
> >> static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
> >> - struct iovec *iov, int niov, bool create,
> >> - enum AIOCBState aiocb_type);
> >> + struct iovec *iov, int niov,
> >> + enum AIOCBState aiocb_type);
> >> static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req);
> >> static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag);
> >> static int get_sheep_fd(BDRVSheepdogState *s, Error **errp);
> >> @@ -698,7 +700,7 @@ static void coroutine_fn send_pending_req(BDRVSheepdogState *s, uint64_t oid)
> >> /* move aio_req from pending list to inflight one */
> >> QLIST_REMOVE(aio_req, aio_siblings);
> >> QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
> >> - add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, false,
> >> + add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
> >> acb->aiocb_type);
> >> }
> >> }
> >> @@ -797,7 +799,7 @@ static void coroutine_fn aio_read_response(void *opaque)
> >> }
> >> idx = data_oid_to_idx(aio_req->oid);
> >>
> >> - if (s->inode.data_vdi_id[idx] != s->inode.vdi_id) {
> >> + if (aio_req->create) {
> >> /*
> >> * If the object is newly created one, we need to update
> >> * the vdi object (metadata object). min_dirty_data_idx
> >> @@ -1117,8 +1119,8 @@ out:
> >> }
> >>
> >> static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
> >> - struct iovec *iov, int niov, bool create,
> >> - enum AIOCBState aiocb_type)
> >> + struct iovec *iov, int niov,
> >> + enum AIOCBState aiocb_type)
> >> {
> >> int nr_copies = s->inode.nr_copies;
> >> SheepdogObjReq hdr;
> >> @@ -1129,6 +1131,7 @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
> >> uint64_t offset = aio_req->offset;
> >> uint8_t flags = aio_req->flags;
> >> uint64_t old_oid = aio_req->base_oid;
> >> + bool create = aio_req->create;
> >>
> >> if (!nr_copies) {
> >> error_report("bug");
> >> @@ -1315,6 +1318,7 @@ static bool check_simultaneous_create(BDRVSheepdogState *s, AIOReq *aio_req)
> >> DPRINTF("simultaneous create to %" PRIx64 "\n", aio_req->oid);
> >> aio_req->flags = 0;
> >> aio_req->base_oid = 0;
> >> + aio_req->create = false;
> >> QLIST_REMOVE(aio_req, aio_siblings);
> >> QLIST_INSERT_HEAD(&s->pending_aio_head, aio_req, aio_siblings);
> >> return true;
> >> @@ -1327,7 +1331,8 @@ static bool check_simultaneous_create(BDRVSheepdogState *s, AIOReq *aio_req)
> >> static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
> >> {
> >> SheepdogAIOCB *acb = aio_req->aiocb;
> >> - bool create = false;
> >> +
> >> + aio_req->create = false;
> >>
> >> /* check whether this request becomes a CoW one */
> >> if (acb->aiocb_type == AIOCB_WRITE_UDATA && is_data_obj(aio_req->oid)) {
> >> @@ -1345,17 +1350,17 @@ static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
> >> aio_req->base_oid = vid_to_data_oid(s->inode.data_vdi_id[idx], idx);
> >> aio_req->flags |= SD_FLAG_CMD_COW;
> >> }
> >> - create = true;
> >> + aio_req->create = true;
> >> }
> >> out:
> >> if (is_data_obj(aio_req->oid)) {
> >> - add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, create,
> >> + add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
> >> acb->aiocb_type);
> >> } else {
> >> struct iovec iov;
> >> iov.iov_base = &s->inode;
> >> iov.iov_len = sizeof(s->inode);
> >> - add_aio_request(s, aio_req, &iov, 1, false, AIOCB_WRITE_UDATA);
> >> + add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
> >> }
> >> }
> >>
> >> @@ -1849,9 +1854,9 @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
> >> iov.iov_base = &s->inode;
> >> iov.iov_len = sizeof(s->inode);
> >> aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
> >> - data_len, offset, 0, 0, offset);
> >> + data_len, offset, 0, false, 0, offset);
> >> QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
> >> - add_aio_request(s, aio_req, &iov, 1, false, AIOCB_WRITE_UDATA);
> >> + add_aio_request(s, aio_req, &iov, 1, AIOCB_WRITE_UDATA);
> >>
> >> acb->aio_done_func = sd_finish_aiocb;
> >> acb->aiocb_type = AIOCB_WRITE_UDATA;
> >> @@ -2049,7 +2054,8 @@ static int coroutine_fn sd_co_rw_vector(void *p)
> >> DPRINTF("new oid %" PRIx64 "\n", oid);
> >> }
> >>
> >> - aio_req = alloc_aio_req(s, acb, oid, len, offset, flags, old_oid, done);
> >> + aio_req = alloc_aio_req(s, acb, oid, len, offset, flags, create,
> >> + old_oid, done);
> >> QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
> >>
> >> if (create) {
> >> @@ -2058,7 +2064,7 @@ static int coroutine_fn sd_co_rw_vector(void *p)
> >> }
> >> }
> >>
> >> - add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, create,
> >> + add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
> >> acb->aiocb_type);
> >> done:
> >> offset = 0;
> >> @@ -2138,9 +2144,9 @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
> >> acb->aio_done_func = sd_finish_aiocb;
> >>
> >> aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
> >> - 0, 0, 0, 0, 0);
> >> + 0, 0, 0, false, 0, 0);
> >> QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
> >> - add_aio_request(s, aio_req, NULL, 0, false, acb->aiocb_type);
> >> + add_aio_request(s, aio_req, NULL, 0, acb->aiocb_type);
> >>
> >> qemu_coroutine_yield();
> >> return acb->ret;
> >> --
> >> 1.7.1
> >>
> >
> > Which line is the problem and which line fixes it? Seems not easy to find it out.
> > I just saw some restructuring of 'create' field.
> >
>
> The below line in aio_read_response() is the problem:
>
> > @@ -797,7 +799,7 @@ static void coroutine_fn aio_read_response(void *opaque)
> > }
> > idx = data_oid_to_idx(aio_req->oid);
> >
> > - if (s->inode.data_vdi_id[idx] != s->inode.vdi_id) {
Seems it is not very obvious and to be honest I still don't get the catch why it
is buggy. Could you please describe it in the commit log why this line is
problematic and we have to use ->create to fix it.
Thanks
Yuan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-06-04 6:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-03 4:54 [Qemu-devel] [PATCH v2 0/2] bugfixes of sheepdog driver for a case of live snapshot Hitoshi Mitake
2014-06-03 4:54 ` [Qemu-devel] [PATCH v2 1/2] sheepdog: fix vdi object update after " Hitoshi Mitake
2014-06-03 12:41 ` Liu Yuan
2014-06-03 14:58 ` Hitoshi Mitake
2014-06-04 6:24 ` Liu Yuan
2014-06-03 4:54 ` [Qemu-devel] [PATCH v2 2/2] sheepdog: reload only header in a case of " Hitoshi Mitake
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).