* [Qemu-devel] [PATCH 0/2] sheepdog driver update
@ 2015-09-01 3:03 Hitoshi Mitake
2015-09-01 3:03 ` [Qemu-devel] [PATCH 1/2] sheepdog: use per AIOCB dirty indexes for non overlapping requests Hitoshi Mitake
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Hitoshi Mitake @ 2015-09-01 3:03 UTC (permalink / raw)
To: qemu-devel; +Cc: Hitoshi Mitake, sheepdog
This patchset has two patches. The first one is a defensive purpose
for rare cases. It is an improvement of the previous commit
96b14ff85acf. The second one is refining discard operation caused by
fstrim command, etc. Current sheepdog driver corrupts VDIs when the
discard operation is issued. This patch solves the problem.
Hitoshi Mitake (2):
sheepdog: use per AIOCB dirty indexes for non overlapping requests
sheepdog: refine discard support
block/sheepdog.c | 92 +++++++++++++++++++++++++++++++++-----------------------
1 file changed, 55 insertions(+), 37 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH 1/2] sheepdog: use per AIOCB dirty indexes for non overlapping requests 2015-09-01 3:03 [Qemu-devel] [PATCH 0/2] sheepdog driver update Hitoshi Mitake @ 2015-09-01 3:03 ` Hitoshi Mitake 2015-09-02 12:36 ` Vasiliy Tolstov 2015-09-01 3:03 ` [Qemu-devel] [PATCH 2/2] sheepdog: refine discard support Hitoshi Mitake 2015-09-25 14:32 ` [Qemu-devel] [PATCH 0/2] sheepdog driver update Jeff Cody 2 siblings, 1 reply; 10+ messages in thread From: Hitoshi Mitake @ 2015-09-01 3:03 UTC (permalink / raw) To: qemu-devel Cc: Hitoshi Mitake, Teruaki Ishizaki, Jeff Cody, sheepdog, Vasiliy Tolstov In the commit 96b14ff85acf, requests for overlapping areas are serialized. However, it cannot handle a case of non overlapping requests. In such a case, min_dirty_data_idx and max_dirty_data_idx can be overwritten by the requests and invalid inode update can happen e.g. a case like create(1, 2) and create(3, 4) are issued in parallel. This patch lets SheepdogAIOCB have dirty data indexes instead of BDRVSheepdogState for avoiding the above situation. This patch also does trivial renaming for better description: overwrapping -> overlapping Cc: Teruaki Ishizaki <ishizaki.teruaki@lab.ntt.co.jp> Cc: Vasiliy Tolstov <v.tolstov@selfip.ru> Cc: Jeff Cody <jcody@redhat.com> Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp> --- block/sheepdog.c | 63 +++++++++++++++++++++++++++++++------------------------- 1 file changed, 35 insertions(+), 28 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 9585beb..24341ea 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -318,7 +318,7 @@ enum AIOCBState { AIOCB_DISCARD_OBJ, }; -#define AIOCBOverwrapping(x, y) \ +#define AIOCBOverlapping(x, y) \ (!(x->max_affect_data_idx < y->min_affect_data_idx \ || y->max_affect_data_idx < x->min_affect_data_idx)) @@ -342,6 +342,15 @@ struct SheepdogAIOCB { uint32_t min_affect_data_idx; uint32_t max_affect_data_idx; + /* + * The difference between affect_data_idx and dirty_data_idx: + * affect_data_idx represents range of index of all request types. + * dirty_data_idx represents range of index updated by COW requests. + * dirty_data_idx is used for updating an inode object. + */ + uint32_t min_dirty_data_idx; + uint32_t max_dirty_data_idx; + QLIST_ENTRY(SheepdogAIOCB) aiocb_siblings; }; @@ -351,9 +360,6 @@ typedef struct BDRVSheepdogState { SheepdogInode inode; - uint32_t min_dirty_data_idx; - uint32_t max_dirty_data_idx; - char name[SD_MAX_VDI_LEN]; bool is_snapshot; uint32_t cache_flags; @@ -373,7 +379,7 @@ typedef struct BDRVSheepdogState { QLIST_HEAD(inflight_aio_head, AIOReq) inflight_aio_head; QLIST_HEAD(failed_aio_head, AIOReq) failed_aio_head; - CoQueue overwrapping_queue; + CoQueue overlapping_queue; QLIST_HEAD(inflight_aiocb_head, SheepdogAIOCB) inflight_aiocb_head; } BDRVSheepdogState; @@ -556,6 +562,9 @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov, acb->max_affect_data_idx = (acb->sector_num * BDRV_SECTOR_SIZE + acb->nb_sectors * BDRV_SECTOR_SIZE) / object_size; + acb->min_dirty_data_idx = UINT32_MAX; + acb->max_dirty_data_idx = 0; + return acb; } @@ -819,8 +828,8 @@ static void coroutine_fn aio_read_response(void *opaque) */ if (rsp.result == SD_RES_SUCCESS) { s->inode.data_vdi_id[idx] = s->inode.vdi_id; - s->max_dirty_data_idx = MAX(idx, s->max_dirty_data_idx); - s->min_dirty_data_idx = MIN(idx, s->min_dirty_data_idx); + acb->max_dirty_data_idx = MAX(idx, acb->max_dirty_data_idx); + acb->min_dirty_data_idx = MIN(idx, acb->min_dirty_data_idx); } } break; @@ -1466,13 +1475,11 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags, } memcpy(&s->inode, buf, sizeof(s->inode)); - s->min_dirty_data_idx = UINT32_MAX; - s->max_dirty_data_idx = 0; bs->total_sectors = s->inode.vdi_size / BDRV_SECTOR_SIZE; pstrcpy(s->name, sizeof(s->name), vdi); qemu_co_mutex_init(&s->lock); - qemu_co_queue_init(&s->overwrapping_queue); + qemu_co_queue_init(&s->overlapping_queue); qemu_opts_del(opts); g_free(buf); return 0; @@ -1923,16 +1930,16 @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb) AIOReq *aio_req; uint32_t offset, data_len, mn, mx; - mn = s->min_dirty_data_idx; - mx = s->max_dirty_data_idx; + mn = acb->min_dirty_data_idx; + mx = acb->max_dirty_data_idx; if (mn <= mx) { /* we need to update the vdi object. */ offset = sizeof(s->inode) - sizeof(s->inode.data_vdi_id) + mn * sizeof(s->inode.data_vdi_id[0]); data_len = (mx - mn + 1) * sizeof(s->inode.data_vdi_id[0]); - s->min_dirty_data_idx = UINT32_MAX; - s->max_dirty_data_idx = 0; + acb->min_dirty_data_idx = UINT32_MAX; + acb->max_dirty_data_idx = 0; iov.iov_base = &s->inode; iov.iov_len = sizeof(s->inode); @@ -2158,12 +2165,12 @@ out: return 1; } -static bool check_overwrapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb) +static bool check_overlapping_aiocb(BDRVSheepdogState *s, SheepdogAIOCB *aiocb) { SheepdogAIOCB *cb; QLIST_FOREACH(cb, &s->inflight_aiocb_head, aiocb_siblings) { - if (AIOCBOverwrapping(aiocb, cb)) { + if (AIOCBOverlapping(aiocb, cb)) { return true; } } @@ -2192,15 +2199,15 @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num, acb->aiocb_type = AIOCB_WRITE_UDATA; retry: - if (check_overwrapping_aiocb(s, acb)) { - qemu_co_queue_wait(&s->overwrapping_queue); + if (check_overlapping_aiocb(s, acb)) { + qemu_co_queue_wait(&s->overlapping_queue); goto retry; } ret = sd_co_rw_vector(acb); if (ret <= 0) { QLIST_REMOVE(acb, aiocb_siblings); - qemu_co_queue_restart_all(&s->overwrapping_queue); + qemu_co_queue_restart_all(&s->overlapping_queue); qemu_aio_unref(acb); return ret; } @@ -2208,7 +2215,7 @@ retry: qemu_coroutine_yield(); QLIST_REMOVE(acb, aiocb_siblings); - qemu_co_queue_restart_all(&s->overwrapping_queue); + qemu_co_queue_restart_all(&s->overlapping_queue); return acb->ret; } @@ -2225,15 +2232,15 @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num, acb->aio_done_func = sd_finish_aiocb; retry: - if (check_overwrapping_aiocb(s, acb)) { - qemu_co_queue_wait(&s->overwrapping_queue); + if (check_overlapping_aiocb(s, acb)) { + qemu_co_queue_wait(&s->overlapping_queue); goto retry; } ret = sd_co_rw_vector(acb); if (ret <= 0) { QLIST_REMOVE(acb, aiocb_siblings); - qemu_co_queue_restart_all(&s->overwrapping_queue); + qemu_co_queue_restart_all(&s->overlapping_queue); qemu_aio_unref(acb); return ret; } @@ -2241,7 +2248,7 @@ retry: qemu_coroutine_yield(); QLIST_REMOVE(acb, aiocb_siblings); - qemu_co_queue_restart_all(&s->overwrapping_queue); + qemu_co_queue_restart_all(&s->overlapping_queue); return acb->ret; } @@ -2590,15 +2597,15 @@ static coroutine_fn int sd_co_discard(BlockDriverState *bs, int64_t sector_num, acb->aio_done_func = sd_finish_aiocb; retry: - if (check_overwrapping_aiocb(s, acb)) { - qemu_co_queue_wait(&s->overwrapping_queue); + if (check_overlapping_aiocb(s, acb)) { + qemu_co_queue_wait(&s->overlapping_queue); goto retry; } ret = sd_co_rw_vector(acb); if (ret <= 0) { QLIST_REMOVE(acb, aiocb_siblings); - qemu_co_queue_restart_all(&s->overwrapping_queue); + qemu_co_queue_restart_all(&s->overlapping_queue); qemu_aio_unref(acb); return ret; } @@ -2606,7 +2613,7 @@ retry: qemu_coroutine_yield(); QLIST_REMOVE(acb, aiocb_siblings); - qemu_co_queue_restart_all(&s->overwrapping_queue); + qemu_co_queue_restart_all(&s->overlapping_queue); return acb->ret; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] sheepdog: use per AIOCB dirty indexes for non overlapping requests 2015-09-01 3:03 ` [Qemu-devel] [PATCH 1/2] sheepdog: use per AIOCB dirty indexes for non overlapping requests Hitoshi Mitake @ 2015-09-02 12:36 ` Vasiliy Tolstov 2015-09-24 2:20 ` [Qemu-devel] [sheepdog] " Hitoshi Mitake 0 siblings, 1 reply; 10+ messages in thread From: Vasiliy Tolstov @ 2015-09-02 12:36 UTC (permalink / raw) To: Hitoshi Mitake; +Cc: Teruaki Ishizaki, Jeff Cody, sheepdog, qemu-devel 2015-09-01 6:03 GMT+03:00 Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>: > n the commit 96b14ff85acf, requests for overlapping areas are > serialized. However, it cannot handle a case of non overlapping > requests. In such a case, min_dirty_data_idx and max_dirty_data_idx > can be overwritten by the requests and invalid inode update can > happen e.g. a case like create(1, 2) and create(3, 4) are issued in > parallel. > > This patch lets SheepdogAIOCB have dirty data indexes instead of > BDRVSheepdogState for avoiding the above situation. > > This patch also does trivial renaming for better description: > overwrapping -> overlapping > > Cc: Teruaki Ishizaki <ishizaki.teruaki@lab.ntt.co.jp> > Cc: Vasiliy Tolstov <v.tolstov@selfip.ru> > Cc: Jeff Cody <jcody@redhat.com> > Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp> I'm test this patch and now discard working properly and no errors in sheepdog log file. Tested-by: Vasiliy Tolstov <v.tolstov@selfip.ru> -- Vasiliy Tolstov, e-mail: v.tolstov@selfip.ru ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [sheepdog] [PATCH 1/2] sheepdog: use per AIOCB dirty indexes for non overlapping requests 2015-09-02 12:36 ` Vasiliy Tolstov @ 2015-09-24 2:20 ` Hitoshi Mitake 0 siblings, 0 replies; 10+ messages in thread From: Hitoshi Mitake @ 2015-09-24 2:20 UTC (permalink / raw) To: Vasiliy Tolstov; +Cc: sheepdog, Jeff Cody, Teruaki Ishizaki, QEMU Developers On Wed, Sep 2, 2015 at 9:36 PM, Vasiliy Tolstov <v.tolstov@selfip.ru> wrote: > 2015-09-01 6:03 GMT+03:00 Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>: >> n the commit 96b14ff85acf, requests for overlapping areas are >> serialized. However, it cannot handle a case of non overlapping >> requests. In such a case, min_dirty_data_idx and max_dirty_data_idx >> can be overwritten by the requests and invalid inode update can >> happen e.g. a case like create(1, 2) and create(3, 4) are issued in >> parallel. >> >> This patch lets SheepdogAIOCB have dirty data indexes instead of >> BDRVSheepdogState for avoiding the above situation. >> >> This patch also does trivial renaming for better description: >> overwrapping -> overlapping >> >> Cc: Teruaki Ishizaki <ishizaki.teruaki@lab.ntt.co.jp> >> Cc: Vasiliy Tolstov <v.tolstov@selfip.ru> >> Cc: Jeff Cody <jcody@redhat.com> >> Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp> > > I'm test this patch and now discard working properly and no errors in > sheepdog log file. > > Tested-by: Vasiliy Tolstov <v.tolstov@selfip.ru> Ping, Jeff? Thanks, Hitoshi > > -- > Vasiliy Tolstov, > e-mail: v.tolstov@selfip.ru > -- > sheepdog mailing list > sheepdog@lists.wpkg.org > https://lists.wpkg.org/mailman/listinfo/sheepdog ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/2] sheepdog: refine discard support 2015-09-01 3:03 [Qemu-devel] [PATCH 0/2] sheepdog driver update Hitoshi Mitake 2015-09-01 3:03 ` [Qemu-devel] [PATCH 1/2] sheepdog: use per AIOCB dirty indexes for non overlapping requests Hitoshi Mitake @ 2015-09-01 3:03 ` Hitoshi Mitake 2015-09-02 12:36 ` Vasiliy Tolstov 2015-09-25 14:32 ` [Qemu-devel] [PATCH 0/2] sheepdog driver update Jeff Cody 2 siblings, 1 reply; 10+ messages in thread From: Hitoshi Mitake @ 2015-09-01 3:03 UTC (permalink / raw) To: qemu-devel Cc: Hitoshi Mitake, Teruaki Ishizaki, Jeff Cody, sheepdog, Vasiliy Tolstov This patch refines discard support of the sheepdog driver. The existing discard mechanism was implemented on SD_OP_DISCARD_OBJ, which was introduced before fine grained reference counting on newer sheepdog. It doesn't care about relations of snapshots and clones and discards objects unconditionally. With this patch, the driver just updates an inode object for updating reference. Removing the object is done in sheep process side. Cc: Teruaki Ishizaki <ishizaki.teruaki@lab.ntt.co.jp> Cc: Vasiliy Tolstov <v.tolstov@selfip.ru> Cc: Jeff Cody <jcody@redhat.com> Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp> --- block/sheepdog.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 24341ea..e3c5cb5 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -28,7 +28,6 @@ #define SD_OP_READ_OBJ 0x02 #define SD_OP_WRITE_OBJ 0x03 /* 0x04 is used internally by Sheepdog */ -#define SD_OP_DISCARD_OBJ 0x05 #define SD_OP_NEW_VDI 0x11 #define SD_OP_LOCK_VDI 0x12 @@ -856,10 +855,6 @@ static void coroutine_fn aio_read_response(void *opaque) rsp.result = SD_RES_SUCCESS; s->discard_supported = false; break; - case SD_RES_SUCCESS: - idx = data_oid_to_idx(aio_req->oid); - s->inode.data_vdi_id[idx] = 0; - break; default: break; } @@ -1174,7 +1169,13 @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, hdr.flags = SD_FLAG_CMD_WRITE | flags; break; case AIOCB_DISCARD_OBJ: - hdr.opcode = SD_OP_DISCARD_OBJ; + hdr.opcode = SD_OP_WRITE_OBJ; + hdr.flags = SD_FLAG_CMD_WRITE | flags; + s->inode.data_vdi_id[data_oid_to_idx(oid)] = 0; + offset = offsetof(SheepdogInode, + data_vdi_id[data_oid_to_idx(oid)]); + oid = vid_to_vdi_oid(s->inode.vdi_id); + wlen = datalen = sizeof(uint32_t); break; } @@ -2148,7 +2149,9 @@ static int coroutine_fn sd_co_rw_vector(void *p) } aio_req = alloc_aio_req(s, acb, oid, len, offset, flags, create, - old_oid, done); + old_oid, + acb->aiocb_type == AIOCB_DISCARD_OBJ ? + 0 : done); QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings); add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, @@ -2584,15 +2587,23 @@ static coroutine_fn int sd_co_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors) { SheepdogAIOCB *acb; - QEMUIOVector dummy; BDRVSheepdogState *s = bs->opaque; int ret; + QEMUIOVector discard_iov; + struct iovec iov; + uint32_t zero = 0; if (!s->discard_supported) { return 0; } - acb = sd_aio_setup(bs, &dummy, sector_num, nb_sectors); + memset(&discard_iov, 0, sizeof(discard_iov)); + memset(&iov, 0, sizeof(iov)); + iov.iov_base = &zero; + iov.iov_len = sizeof(zero); + discard_iov.iov = &iov; + discard_iov.niov = 1; + acb = sd_aio_setup(bs, &discard_iov, sector_num, nb_sectors); acb->aiocb_type = AIOCB_DISCARD_OBJ; acb->aio_done_func = sd_finish_aiocb; -- 1.9.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] sheepdog: refine discard support 2015-09-01 3:03 ` [Qemu-devel] [PATCH 2/2] sheepdog: refine discard support Hitoshi Mitake @ 2015-09-02 12:36 ` Vasiliy Tolstov 2015-09-04 8:51 ` [Qemu-devel] [sheepdog] " Hitoshi Mitake 0 siblings, 1 reply; 10+ messages in thread From: Vasiliy Tolstov @ 2015-09-02 12:36 UTC (permalink / raw) To: Hitoshi Mitake; +Cc: Teruaki Ishizaki, Jeff Cody, sheepdog, qemu-devel 2015-09-01 6:03 GMT+03:00 Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>: > This patch refines discard support of the sheepdog driver. The > existing discard mechanism was implemented on SD_OP_DISCARD_OBJ, which > was introduced before fine grained reference counting on newer > sheepdog. It doesn't care about relations of snapshots and clones and > discards objects unconditionally. > > With this patch, the driver just updates an inode object for updating > reference. Removing the object is done in sheep process side. > > Cc: Teruaki Ishizaki <ishizaki.teruaki@lab.ntt.co.jp> > Cc: Vasiliy Tolstov <v.tolstov@selfip.ru> > Cc: Jeff Cody <jcody@redhat.com> > Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp> I'm test this patch and now discard working properly and no errors in sheepdog log file. Tested-by: Vasiliy Tolstov <v.tolstov@selfip.ru> -- Vasiliy Tolstov, e-mail: v.tolstov@selfip.ru ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [sheepdog] [PATCH 2/2] sheepdog: refine discard support 2015-09-02 12:36 ` Vasiliy Tolstov @ 2015-09-04 8:51 ` Hitoshi Mitake 2015-09-04 9:57 ` Hitoshi Mitake 0 siblings, 1 reply; 10+ messages in thread From: Hitoshi Mitake @ 2015-09-04 8:51 UTC (permalink / raw) To: Vasiliy Tolstov; +Cc: sheepdog, Jeff Cody, Teruaki Ishizaki, QEMU Developers On Wed, Sep 2, 2015 at 9:36 PM, Vasiliy Tolstov <v.tolstov@selfip.ru> wrote: > 2015-09-01 6:03 GMT+03:00 Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>: >> This patch refines discard support of the sheepdog driver. The >> existing discard mechanism was implemented on SD_OP_DISCARD_OBJ, which >> was introduced before fine grained reference counting on newer >> sheepdog. It doesn't care about relations of snapshots and clones and >> discards objects unconditionally. >> >> With this patch, the driver just updates an inode object for updating >> reference. Removing the object is done in sheep process side. >> >> Cc: Teruaki Ishizaki <ishizaki.teruaki@lab.ntt.co.jp> >> Cc: Vasiliy Tolstov <v.tolstov@selfip.ru> >> Cc: Jeff Cody <jcody@redhat.com> >> Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp> > > > I'm test this patch and now discard working properly and no errors in > sheepdog log file. > > Tested-by: Vasiliy Tolstov <v.tolstov@selfip.ru> On the second thought, this patch has a problem of handling snapshot. Please drop this one (1st patch is ok to apply). I'll solve the problem in sheepdog side. Thanks, Hitoshi > > -- > Vasiliy Tolstov, > e-mail: v.tolstov@selfip.ru > -- > sheepdog mailing list > sheepdog@lists.wpkg.org > https://lists.wpkg.org/mailman/listinfo/sheepdog ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [sheepdog] [PATCH 2/2] sheepdog: refine discard support 2015-09-04 8:51 ` [Qemu-devel] [sheepdog] " Hitoshi Mitake @ 2015-09-04 9:57 ` Hitoshi Mitake 2015-09-04 10:32 ` Vasiliy Tolstov 0 siblings, 1 reply; 10+ messages in thread From: Hitoshi Mitake @ 2015-09-04 9:57 UTC (permalink / raw) To: Vasiliy Tolstov; +Cc: sheepdog, Jeff Cody, Teruaki Ishizaki, QEMU Developers On Fri, Sep 4, 2015 at 5:51 PM, Hitoshi Mitake <mitake.hitoshi@gmail.com> wrote: > On Wed, Sep 2, 2015 at 9:36 PM, Vasiliy Tolstov <v.tolstov@selfip.ru> wrote: >> 2015-09-01 6:03 GMT+03:00 Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp>: >>> This patch refines discard support of the sheepdog driver. The >>> existing discard mechanism was implemented on SD_OP_DISCARD_OBJ, which >>> was introduced before fine grained reference counting on newer >>> sheepdog. It doesn't care about relations of snapshots and clones and >>> discards objects unconditionally. >>> >>> With this patch, the driver just updates an inode object for updating >>> reference. Removing the object is done in sheep process side. >>> >>> Cc: Teruaki Ishizaki <ishizaki.teruaki@lab.ntt.co.jp> >>> Cc: Vasiliy Tolstov <v.tolstov@selfip.ru> >>> Cc: Jeff Cody <jcody@redhat.com> >>> Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp> >> >> >> I'm test this patch and now discard working properly and no errors in >> sheepdog log file. >> >> Tested-by: Vasiliy Tolstov <v.tolstov@selfip.ru> > > On the second thought, this patch has a problem of handling snapshot. > Please drop this one (1st patch is ok to apply). > > I'll solve the problem in sheepdog side. On the third thought, this patch can work well ;) Please pick this patch, Jeff. I considered about a case of interleaving of snapshotting and discarding requests like below: 1. user invokes dog vdi snapshot, a command for making current VDI snapshot (updating inode objects) 2. discard request from VM before the request of 1 completes 3. discard completes 4. request of 1 completes In this case, some data_vdi_id of original inode can be overwritten before completion of snapshotting. However, this behavior is valid because dog vdi snapshot doesn't return ack to the user. Thanks, Hitoshi > > Thanks, > Hitoshi > >> >> -- >> Vasiliy Tolstov, >> e-mail: v.tolstov@selfip.ru >> -- >> sheepdog mailing list >> sheepdog@lists.wpkg.org >> https://lists.wpkg.org/mailman/listinfo/sheepdog ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [sheepdog] [PATCH 2/2] sheepdog: refine discard support 2015-09-04 9:57 ` Hitoshi Mitake @ 2015-09-04 10:32 ` Vasiliy Tolstov 0 siblings, 0 replies; 10+ messages in thread From: Vasiliy Tolstov @ 2015-09-04 10:32 UTC (permalink / raw) To: Hitoshi Mitake; +Cc: sheepdog, Jeff Cody, Teruaki Ishizaki, QEMU Developers [-- Attachment #1: Type: text/plain, Size: 2540 bytes --] 04 сент. 2015 г. 12:57 пользователь "Hitoshi Mitake" < mitake.hitoshi@gmail.com> написал: > > On Fri, Sep 4, 2015 at 5:51 PM, Hitoshi Mitake <mitake.hitoshi@gmail.com> wrote: > > On Wed, Sep 2, 2015 at 9:36 PM, Vasiliy Tolstov <v.tolstov@selfip.ru> wrote: > >> 2015-09-01 6:03 GMT+03:00 Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp >: > >>> This patch refines discard support of the sheepdog driver. The > >>> existing discard mechanism was implemented on SD_OP_DISCARD_OBJ, which > >>> was introduced before fine grained reference counting on newer > >>> sheepdog. It doesn't care about relations of snapshots and clones and > >>> discards objects unconditionally. > >>> > >>> With this patch, the driver just updates an inode object for updating > >>> reference. Removing the object is done in sheep process side. > >>> > >>> Cc: Teruaki Ishizaki <ishizaki.teruaki@lab.ntt.co.jp> > >>> Cc: Vasiliy Tolstov <v.tolstov@selfip.ru> > >>> Cc: Jeff Cody <jcody@redhat.com> > >>> Signed-off-by: Hitoshi Mitake <mitake.hitoshi@lab.ntt.co.jp> > >> > >> > >> I'm test this patch and now discard working properly and no errors in > >> sheepdog log file. > >> > >> Tested-by: Vasiliy Tolstov <v.tolstov@selfip.ru> > > > > On the second thought, this patch has a problem of handling snapshot. > > Please drop this one (1st patch is ok to apply). > > > > I'll solve the problem in sheepdog side. > > On the third thought, this patch can work well ;) Please pick this patch, Jeff. > > I considered about a case of interleaving of snapshotting and > discarding requests like below: > 1. user invokes dog vdi snapshot, a command for making current VDI > snapshot (updating inode objects) > 2. discard request from VM before the request of 1 completes > 3. discard completes > 4. request of 1 completes > > In this case, some data_vdi_id of original inode can be overwritten > before completion of snapshotting. However, this behavior is valid > because dog vdi snapshot doesn't return ack to the user. > This is normal for snapshots, to get consistent data you need to cooperate with guest system to get freeze it fs before snapshot. Quemu-ga already have this via sending freeze to fs beforw and thaw after snapshot . > Thanks, > Hitoshi > > > > > Thanks, > > Hitoshi > > > >> > >> -- > >> Vasiliy Tolstov, > >> e-mail: v.tolstov@selfip.ru > >> -- > >> sheepdog mailing list > >> sheepdog@lists.wpkg.org > >> https://lists.wpkg.org/mailman/listinfo/sheepdog [-- Attachment #2: Type: text/html, Size: 3867 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] sheepdog driver update 2015-09-01 3:03 [Qemu-devel] [PATCH 0/2] sheepdog driver update Hitoshi Mitake 2015-09-01 3:03 ` [Qemu-devel] [PATCH 1/2] sheepdog: use per AIOCB dirty indexes for non overlapping requests Hitoshi Mitake 2015-09-01 3:03 ` [Qemu-devel] [PATCH 2/2] sheepdog: refine discard support Hitoshi Mitake @ 2015-09-25 14:32 ` Jeff Cody 2 siblings, 0 replies; 10+ messages in thread From: Jeff Cody @ 2015-09-25 14:32 UTC (permalink / raw) To: Hitoshi Mitake; +Cc: sheepdog, qemu-devel On Tue, Sep 01, 2015 at 12:03:08PM +0900, Hitoshi Mitake wrote: > This patchset has two patches. The first one is a defensive purpose > for rare cases. It is an improvement of the previous commit > 96b14ff85acf. The second one is refining discard operation caused by > fstrim command, etc. Current sheepdog driver corrupts VDIs when the > discard operation is issued. This patch solves the problem. > > > Hitoshi Mitake (2): > sheepdog: use per AIOCB dirty indexes for non overlapping requests > sheepdog: refine discard support > > block/sheepdog.c | 92 +++++++++++++++++++++++++++++++++----------------------- > 1 file changed, 55 insertions(+), 37 deletions(-) > > -- > 1.9.1 > > Thanks, applied to my block branch: git://github.com/codyprime/qemu-kvm-jtc.git block -Jeff ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-09-25 14:32 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-09-01 3:03 [Qemu-devel] [PATCH 0/2] sheepdog driver update Hitoshi Mitake 2015-09-01 3:03 ` [Qemu-devel] [PATCH 1/2] sheepdog: use per AIOCB dirty indexes for non overlapping requests Hitoshi Mitake 2015-09-02 12:36 ` Vasiliy Tolstov 2015-09-24 2:20 ` [Qemu-devel] [sheepdog] " Hitoshi Mitake 2015-09-01 3:03 ` [Qemu-devel] [PATCH 2/2] sheepdog: refine discard support Hitoshi Mitake 2015-09-02 12:36 ` Vasiliy Tolstov 2015-09-04 8:51 ` [Qemu-devel] [sheepdog] " Hitoshi Mitake 2015-09-04 9:57 ` Hitoshi Mitake 2015-09-04 10:32 ` Vasiliy Tolstov 2015-09-25 14:32 ` [Qemu-devel] [PATCH 0/2] sheepdog driver update Jeff Cody
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).