* [Qemu-devel] [PATCH] sheepdog: implement SD_OP_FLUSH_VDI operation @ 2012-04-02 17:35 Liu Yuan 2012-04-02 20:01 ` Michael Tokarev 0 siblings, 1 reply; 19+ messages in thread From: Liu Yuan @ 2012-04-02 17:35 UTC (permalink / raw) To: qemu-devel, morita.kazutaka; +Cc: Kevin Wolf From: Liu Yuan <tailai.ly@taobao.com> Flush operation is supposed to flush the write-back cache of sheepdog cluster. By issuing flush operation, we can assure the Guest of data reaching the sheepdog cluster storage. Cc: Kevin Wolf <kwolf@redhat.com> Reviewd-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> Signed-off-by: Liu Yuan <tailai.ly@taobao.com> --- block/sheepdog.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 50 insertions(+), 0 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 00276f6f..c08c69b 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -32,9 +32,11 @@ #define SD_OP_RELEASE_VDI 0x13 #define SD_OP_GET_VDI_INFO 0x14 #define SD_OP_READ_VDIS 0x15 +#define SD_OP_FLUSH_VDI 0x16 #define SD_FLAG_CMD_WRITE 0x01 #define SD_FLAG_CMD_COW 0x02 +#define SD_FLAG_CMD_CACHE 0x04 #define SD_RES_SUCCESS 0x00 /* Success */ #define SD_RES_UNKNOWN 0x01 /* Unknown error */ @@ -179,6 +181,8 @@ typedef struct SheepdogInode { uint32_t data_vdi_id[MAX_DATA_OBJS]; } SheepdogInode; +static int cache_enabled; + /* * 64 bit FNV-1a non-zero initial basis */ @@ -900,6 +904,10 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, hdr.flags = SD_FLAG_CMD_WRITE | flags; } + if (cache_enabled) { + hdr.flags |= SD_FLAG_CMD_CACHE; + } + hdr.oid = oid; hdr.cow_oid = old_oid; hdr.copies = s->inode.nr_copies; @@ -965,6 +973,11 @@ static int read_write_object(int fd, char *buf, uint64_t oid, int copies, rlen = datalen; hdr.opcode = SD_OP_READ_OBJ; } + + if (cache_enabled) { + hdr.flags |= SD_FLAG_CMD_CACHE; + } + hdr.oid = oid; hdr.data_length = datalen; hdr.offset = offset; @@ -1011,6 +1024,10 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) QLIST_INIT(&s->outstanding_aio_head); s->fd = -1; + if (flags & BDRV_O_CACHE_WB) { + cache_enabled = 1; + } + memset(vdi, 0, sizeof(vdi)); memset(tag, 0, sizeof(tag)); if (parse_vdiname(s, filename, vdi, &snapid, tag) < 0) { @@ -1575,6 +1592,38 @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num, return acb->ret; } +static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs) +{ + BDRVSheepdogState *s = bs->opaque; + SheepdogObjReq hdr = { 0 }; + SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; + SheepdogInode *inode = &s->inode; + int fd, ret; + unsigned int wlen = 0, rlen = 0; + + fd = connect_to_sdog(s->addr, s->port); + if (fd < 0) { + return -1; + } + + hdr.opcode = SD_OP_FLUSH_VDI; + hdr.oid = vid_to_vdi_oid(inode->vdi_id); + + ret = do_req(fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen); + closesocket(fd); + if (ret) { + error_report("failed to send a request to the sheep"); + return -1; + } + + if (rsp->result != SD_RES_SUCCESS) { + error_report("%s", sd_strerror(rsp->result)); + return -1; + } + + return 0; +} + static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) { BDRVSheepdogState *s = bs->opaque; @@ -1904,6 +1953,7 @@ BlockDriver bdrv_sheepdog = { .bdrv_co_readv = sd_co_readv, .bdrv_co_writev = sd_co_writev, + .bdrv_co_flush_to_disk = sd_co_flush_to_disk, .bdrv_snapshot_create = sd_snapshot_create, .bdrv_snapshot_goto = sd_snapshot_goto, -- 1.7.8.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-02 17:35 [Qemu-devel] [PATCH] sheepdog: implement SD_OP_FLUSH_VDI operation Liu Yuan @ 2012-04-02 20:01 ` Michael Tokarev 2012-04-03 5:14 ` Liu Yuan 2012-04-03 5:15 ` [Qemu-devel] [PATCH v2] " Liu Yuan 0 siblings, 2 replies; 19+ messages in thread From: Michael Tokarev @ 2012-04-02 20:01 UTC (permalink / raw) To: Liu Yuan; +Cc: Kevin Wolf, qemu-devel, morita.kazutaka On 02.04.2012 21:35, Liu Yuan wrote: > From: Liu Yuan <tailai.ly@taobao.com> > > Flush operation is supposed to flush the write-back cache of > sheepdog cluster. > > By issuing flush operation, we can assure the Guest of data > reaching the sheepdog cluster storage. > > Cc: Kevin Wolf <kwolf@redhat.com> > Reviewd-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> > Signed-off-by: Liu Yuan <tailai.ly@taobao.com> > --- > block/sheepdog.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 50 insertions(+), 0 deletions(-) > > diff --git a/block/sheepdog.c b/block/sheepdog.c > index 00276f6f..c08c69b 100644 > --- a/block/sheepdog.c > +++ b/block/sheepdog.c > @@ -32,9 +32,11 @@ > #define SD_OP_RELEASE_VDI 0x13 > #define SD_OP_GET_VDI_INFO 0x14 > #define SD_OP_READ_VDIS 0x15 > +#define SD_OP_FLUSH_VDI 0x16 > > #define SD_FLAG_CMD_WRITE 0x01 > #define SD_FLAG_CMD_COW 0x02 > +#define SD_FLAG_CMD_CACHE 0x04 > > #define SD_RES_SUCCESS 0x00 /* Success */ > #define SD_RES_UNKNOWN 0x01 /* Unknown error */ > @@ -179,6 +181,8 @@ typedef struct SheepdogInode { > uint32_t data_vdi_id[MAX_DATA_OBJS]; > } SheepdogInode; > > +static int cache_enabled; Why it is a global property instead of per-device (in BDRVSheepdogState) property? > @@ -1011,6 +1024,10 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) > QLIST_INIT(&s->outstanding_aio_head); > s->fd = -1; > > + if (flags & BDRV_O_CACHE_WB) { > + cache_enabled = 1; > + } You test per-device flag here, and set a global variable, why? > +static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs) > +{ > + BDRVSheepdogState *s = bs->opaque; > + SheepdogObjReq hdr = { 0 }; > + SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; > + SheepdogInode *inode = &s->inode; > + int fd, ret; > + unsigned int wlen = 0, rlen = 0; > + > + fd = connect_to_sdog(s->addr, s->port); > + if (fd < 0) { > + return -1; > + } Do you really need _another_ connection here? How about resolving server names, handling connection timeouts (in case the server can't accept new connection for some reason) and other surrounding things? Thanks, /mjt ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-02 20:01 ` Michael Tokarev @ 2012-04-03 5:14 ` Liu Yuan 2012-04-03 5:15 ` [Qemu-devel] [PATCH v2] " Liu Yuan 1 sibling, 0 replies; 19+ messages in thread From: Liu Yuan @ 2012-04-03 5:14 UTC (permalink / raw) To: Michael Tokarev; +Cc: Kevin Wolf, qemu-devel, MORITA Kazutaka On 04/03/2012 04:01 AM, Michael Tokarev wrote: > On 02.04.2012 21:35, Liu Yuan wrote: >> From: Liu Yuan <tailai.ly@taobao.com> >> >> Flush operation is supposed to flush the write-back cache of >> sheepdog cluster. >> >> By issuing flush operation, we can assure the Guest of data >> reaching the sheepdog cluster storage. >> >> Cc: Kevin Wolf <kwolf@redhat.com> >> Reviewd-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> >> Signed-off-by: Liu Yuan <tailai.ly@taobao.com> >> --- >> block/sheepdog.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 files changed, 50 insertions(+), 0 deletions(-) >> >> diff --git a/block/sheepdog.c b/block/sheepdog.c >> index 00276f6f..c08c69b 100644 >> --- a/block/sheepdog.c >> +++ b/block/sheepdog.c >> @@ -32,9 +32,11 @@ >> #define SD_OP_RELEASE_VDI 0x13 >> #define SD_OP_GET_VDI_INFO 0x14 >> #define SD_OP_READ_VDIS 0x15 >> +#define SD_OP_FLUSH_VDI 0x16 >> >> #define SD_FLAG_CMD_WRITE 0x01 >> #define SD_FLAG_CMD_COW 0x02 >> +#define SD_FLAG_CMD_CACHE 0x04 >> >> #define SD_RES_SUCCESS 0x00 /* Success */ >> #define SD_RES_UNKNOWN 0x01 /* Unknown error */ >> @@ -179,6 +181,8 @@ typedef struct SheepdogInode { >> uint32_t data_vdi_id[MAX_DATA_OBJS]; >> } SheepdogInode; >> >> +static int cache_enabled; > > Why it is a global property instead of per-device > (in BDRVSheepdogState) property? > >> @@ -1011,6 +1024,10 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) >> QLIST_INIT(&s->outstanding_aio_head); >> s->fd = -1; >> >> + if (flags & BDRV_O_CACHE_WB) { >> + cache_enabled = 1; >> + } > > You test per-device flag here, and set a global variable, why? > Hi Michael, Yes, I'd better embed this variable into BDRVSheepdogState. >> +static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs) >> +{ >> + BDRVSheepdogState *s = bs->opaque; >> + SheepdogObjReq hdr = { 0 }; >> + SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; >> + SheepdogInode *inode = &s->inode; >> + int fd, ret; >> + unsigned int wlen = 0, rlen = 0; >> + >> + fd = connect_to_sdog(s->addr, s->port); >> + if (fd < 0) { >> + return -1; >> + } > > Do you really need _another_ connection here? I am unsure if I can use bs->fd or not, Kazum, would you please check this usage? I tried use bs->fd in V2 and everything goes okay. Thanks, Yuan ^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v2] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-02 20:01 ` Michael Tokarev 2012-04-03 5:14 ` Liu Yuan @ 2012-04-03 5:15 ` Liu Yuan 2012-04-03 7:23 ` Stefan Hajnoczi 1 sibling, 1 reply; 19+ messages in thread From: Liu Yuan @ 2012-04-03 5:15 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Michael Tokarev, MORITA Kazutaka From: Liu Yuan <tailai.ly@taobao.com> Flush operation is supposed to flush the write-back cache of sheepdog cluster. By issuing flush operation, we can assure the Guest of data reaching the sheepdog cluster storage. Cc: Kevin Wolf <kwolf@redhat.com> Cc: Michael Tokarev <mjt@tls.msk.ru> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> Signed-off-by: Liu Yuan <tailai.ly@taobao.com> --- v1 -> v2: address Michael Tokarev comments. Thanks ! - use per-device flag - use bs->fd instead of 'connect_to_sdog()' block/sheepdog.c | 81 ++++++++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 67 insertions(+), 14 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 00276f6f..dbac629 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -32,9 +32,11 @@ #define SD_OP_RELEASE_VDI 0x13 #define SD_OP_GET_VDI_INFO 0x14 #define SD_OP_READ_VDIS 0x15 +#define SD_OP_FLUSH_VDI 0x16 #define SD_FLAG_CMD_WRITE 0x01 #define SD_FLAG_CMD_COW 0x02 +#define SD_FLAG_CMD_CACHE 0x04 #define SD_RES_SUCCESS 0x00 /* Success */ #define SD_RES_UNKNOWN 0x01 /* Unknown error */ @@ -293,6 +295,7 @@ typedef struct BDRVSheepdogState { char name[SD_MAX_VDI_LEN]; int is_snapshot; + uint8_t cache_enabled; char *addr; char *port; @@ -900,6 +903,10 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, hdr.flags = SD_FLAG_CMD_WRITE | flags; } + if (s->cache_enabled) { + hdr.flags |= SD_FLAG_CMD_CACHE; + } + hdr.oid = oid; hdr.cow_oid = old_oid; hdr.copies = s->inode.nr_copies; @@ -942,7 +949,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, static int read_write_object(int fd, char *buf, uint64_t oid, int copies, unsigned int datalen, uint64_t offset, - int write, int create) + int write, int create, uint8_t cache) { SheepdogObjReq hdr; SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; @@ -965,6 +972,11 @@ static int read_write_object(int fd, char *buf, uint64_t oid, int copies, rlen = datalen; hdr.opcode = SD_OP_READ_OBJ; } + + if (cache) { + hdr.flags |= SD_FLAG_CMD_CACHE; + } + hdr.oid = oid; hdr.data_length = datalen; hdr.offset = offset; @@ -986,15 +998,18 @@ static int read_write_object(int fd, char *buf, uint64_t oid, int copies, } static int read_object(int fd, char *buf, uint64_t oid, int copies, - unsigned int datalen, uint64_t offset) + unsigned int datalen, uint64_t offset, uint8_t cache) { - return read_write_object(fd, buf, oid, copies, datalen, offset, 0, 0); + return read_write_object(fd, buf, oid, copies, datalen, offset, 0, 0, + cache); } static int write_object(int fd, char *buf, uint64_t oid, int copies, - unsigned int datalen, uint64_t offset, int create) + unsigned int datalen, uint64_t offset, int create, + uint8_t cache) { - return read_write_object(fd, buf, oid, copies, datalen, offset, 1, create); + return read_write_object(fd, buf, oid, copies, datalen, offset, 1, create, + cache); } static int sd_open(BlockDriverState *bs, const char *filename, int flags) @@ -1011,6 +1026,10 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) QLIST_INIT(&s->outstanding_aio_head); s->fd = -1; + if (flags & BDRV_O_CACHE_WB) { + s->cache_enabled = 1; + } + memset(vdi, 0, sizeof(vdi)); memset(tag, 0, sizeof(tag)); if (parse_vdiname(s, filename, vdi, &snapid, tag) < 0) { @@ -1038,7 +1057,8 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) } buf = g_malloc(SD_INODE_SIZE); - ret = read_object(fd, buf, vid_to_vdi_oid(vid), 0, SD_INODE_SIZE, 0); + ret = read_object(fd, buf, vid_to_vdi_oid(vid), 0, SD_INODE_SIZE, 0, + s->cache_enabled); closesocket(fd); @@ -1305,7 +1325,7 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset) datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id); s->inode.vdi_size = offset; ret = write_object(fd, (char *)&s->inode, vid_to_vdi_oid(s->inode.vdi_id), - s->inode.nr_copies, datalen, 0, 0); + s->inode.nr_copies, datalen, 0, 0, s->cache_enabled); close(fd); if (ret < 0) { @@ -1387,7 +1407,7 @@ static int sd_create_branch(BDRVSheepdogState *s) } ret = read_object(fd, buf, vid_to_vdi_oid(vid), s->inode.nr_copies, - SD_INODE_SIZE, 0); + SD_INODE_SIZE, 0, s->cache_enabled); closesocket(fd); @@ -1575,6 +1595,35 @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num, return acb->ret; } +static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs) +{ + BDRVSheepdogState *s = bs->opaque; + SheepdogObjReq hdr = { 0 }; + SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; + SheepdogInode *inode = &s->inode; + int ret; + unsigned int wlen = 0, rlen = 0; + + if (!s->cache_enabled) + return 0; + + hdr.opcode = SD_OP_FLUSH_VDI; + hdr.oid = vid_to_vdi_oid(inode->vdi_id); + + ret = do_req(s->fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen); + if (ret) { + error_report("failed to send a request to the sheep"); + return -1; + } + + if (rsp->result != SD_RES_SUCCESS) { + error_report("%s", sd_strerror(rsp->result)); + return -1; + } + + return 0; +} + static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) { BDRVSheepdogState *s = bs->opaque; @@ -1610,7 +1659,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) } ret = write_object(fd, (char *)&s->inode, vid_to_vdi_oid(s->inode.vdi_id), - s->inode.nr_copies, datalen, 0, 0); + s->inode.nr_copies, datalen, 0, 0, s->cache_enabled); if (ret < 0) { error_report("failed to write snapshot's inode."); ret = -EIO; @@ -1629,7 +1678,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) inode = (SheepdogInode *)g_malloc(datalen); ret = read_object(fd, (char *)inode, vid_to_vdi_oid(new_vid), - s->inode.nr_copies, datalen, 0); + s->inode.nr_copies, datalen, 0, s->cache_enabled); if (ret < 0) { error_report("failed to read new inode info. %s", strerror(errno)); @@ -1684,7 +1733,7 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) buf = g_malloc(SD_INODE_SIZE); ret = read_object(fd, buf, vid_to_vdi_oid(vid), s->inode.nr_copies, - SD_INODE_SIZE, 0); + SD_INODE_SIZE, 0, s->cache_enabled); closesocket(fd); @@ -1779,7 +1828,8 @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) /* we don't need to read entire object */ ret = read_object(fd, (char *)&inode, vid_to_vdi_oid(vid), - 0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0); + 0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0, + s->cache_enabled); if (ret) { continue; @@ -1835,10 +1885,12 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data, create = (offset == 0); if (load) { ret = read_object(fd, (char *)data, vmstate_oid, - s->inode.nr_copies, data_len, offset); + s->inode.nr_copies, data_len, offset, + s->cache_enabled); } else { ret = write_object(fd, (char *)data, vmstate_oid, - s->inode.nr_copies, data_len, offset, create); + s->inode.nr_copies, data_len, offset, create, + s->cache_enabled); } if (ret < 0) { @@ -1904,6 +1956,7 @@ BlockDriver bdrv_sheepdog = { .bdrv_co_readv = sd_co_readv, .bdrv_co_writev = sd_co_writev, + .bdrv_co_flush_to_disk = sd_co_flush_to_disk, .bdrv_snapshot_create = sd_snapshot_create, .bdrv_snapshot_goto = sd_snapshot_goto, -- 1.7.8.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v2] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-03 5:15 ` [Qemu-devel] [PATCH v2] " Liu Yuan @ 2012-04-03 7:23 ` Stefan Hajnoczi 2012-04-03 8:35 ` [Qemu-devel] [PATCH v3] " Liu Yuan 0 siblings, 1 reply; 19+ messages in thread From: Stefan Hajnoczi @ 2012-04-03 7:23 UTC (permalink / raw) To: Liu Yuan; +Cc: Kevin Wolf, Michael Tokarev, qemu-devel, MORITA Kazutaka On Tue, Apr 03, 2012 at 01:15:40PM +0800, Liu Yuan wrote: > +static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs) > +{ > + BDRVSheepdogState *s = bs->opaque; > + SheepdogObjReq hdr = { 0 }; > + SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; > + SheepdogInode *inode = &s->inode; > + int ret; > + unsigned int wlen = 0, rlen = 0; > + > + if (!s->cache_enabled) > + return 0; > + > + hdr.opcode = SD_OP_FLUSH_VDI; > + hdr.oid = vid_to_vdi_oid(inode->vdi_id); > + > + ret = do_req(s->fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen); > + if (ret) { > + error_report("failed to send a request to the sheep"); > + return -1; > + } > + > + if (rsp->result != SD_RES_SUCCESS) { > + error_report("%s", sd_strerror(rsp->result)); > + return -1; > + } > + > + return 0; > +} .bdrv_co_flush_to_disk() is a coroutine_fn so that it can yield while waiting for socket I/O. Using do_req() defeats its purpose and will pause both the guest and QEMU's own monitor/VNC/etc while waiting for the sheepdog server to communicate with us. You should use coroutine send/recv functions - they yield if the socket returns EAGAIN/EWOULDBLOCK. Also please return negative errno values instead of -1 (which is -EPERM). Stefan ^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v3] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-03 7:23 ` Stefan Hajnoczi @ 2012-04-03 8:35 ` Liu Yuan 2012-04-03 9:12 ` Kevin Wolf 2012-04-03 9:52 ` [Qemu-devel] [PATCH v3] sheepdog: implement SD_OP_FLUSH_VDI operation MORITA Kazutaka 0 siblings, 2 replies; 19+ messages in thread From: Liu Yuan @ 2012-04-03 8:35 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Michael Tokarev, MORITA Kazutaka From: Liu Yuan <tailai.ly@taobao.com> Flush operation is supposed to flush the write-back cache of sheepdog cluster. By issuing flush operation, we can assure the Guest of data reaching the sheepdog cluster storage. Cc: Kevin Wolf <kwolf@redhat.com> Cc: Michael Tokarev <mjt@tls.msk.ru> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> Cc: Stefan Hajnoczi <stefanha@gmail.com> Signed-off-by: Liu Yuan <tailai.ly@taobao.com> --- v2 -> v3: address Stefan Hajnoczi comments. Thanks ! - use qemu_co_send/recv to send/revc msg v1 -> v2: address Michael Tokarev comments. Thanks ! - use per-device flag - use bs->fd instead of 'connect_to_sdog()' block/sheepdog.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 119 insertions(+), 14 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 00276f6f..b2bb522 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -32,9 +32,11 @@ #define SD_OP_RELEASE_VDI 0x13 #define SD_OP_GET_VDI_INFO 0x14 #define SD_OP_READ_VDIS 0x15 +#define SD_OP_FLUSH_VDI 0x16 #define SD_FLAG_CMD_WRITE 0x01 #define SD_FLAG_CMD_COW 0x02 +#define SD_FLAG_CMD_CACHE 0x04 #define SD_RES_SUCCESS 0x00 /* Success */ #define SD_RES_UNKNOWN 0x01 /* Unknown error */ @@ -293,6 +295,7 @@ typedef struct BDRVSheepdogState { char name[SD_MAX_VDI_LEN]; int is_snapshot; + uint8_t cache_enabled; char *addr; char *port; @@ -516,6 +519,23 @@ static int send_req(int sockfd, SheepdogReq *hdr, void *data, return ret; } +static int send_co_req(int sockfd, SheepdogReq *hdr, void *data, + unsigned int *wlen) +{ + int ret; + + ret = qemu_co_send(sockfd, hdr, sizeof(*hdr)); + if (ret < sizeof(*hdr)) { + error_report("failed to send a req, %s", strerror(errno)); + } + + ret = qemu_co_send(sockfd, data, *wlen); + if (ret < *wlen) { + error_report("failed to send a req, %s", strerror(errno)); + } + + return ret; +} static int do_req(int sockfd, SheepdogReq *hdr, void *data, unsigned int *wlen, unsigned int *rlen) { @@ -550,6 +570,40 @@ out: return ret; } +static int do_co_req(int sockfd, SheepdogReq *hdr, void *data, + unsigned int *wlen, unsigned int *rlen) +{ + int ret; + + socket_set_block(sockfd); + ret = send_co_req(sockfd, hdr, data, wlen); + if (ret < 0) { + goto out; + } + + ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr)); + if (ret < sizeof(*hdr)) { + error_report("failed to get a rsp, %s", strerror(errno)); + goto out; + } + + if (*rlen > hdr->data_length) { + *rlen = hdr->data_length; + } + + if (*rlen) { + ret = qemu_co_recv(sockfd, data, *rlen); + if (ret < *rlen) { + error_report("failed to get the data, %s", strerror(errno)); + goto out; + } + } + ret = 0; +out: + socket_set_nonblock(sockfd); + return ret; +} + static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, struct iovec *iov, int niov, int create, enum AIOCBState aiocb_type); @@ -900,6 +954,10 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, hdr.flags = SD_FLAG_CMD_WRITE | flags; } + if (s->cache_enabled) { + hdr.flags |= SD_FLAG_CMD_CACHE; + } + hdr.oid = oid; hdr.cow_oid = old_oid; hdr.copies = s->inode.nr_copies; @@ -942,7 +1000,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, static int read_write_object(int fd, char *buf, uint64_t oid, int copies, unsigned int datalen, uint64_t offset, - int write, int create) + int write, int create, uint8_t cache) { SheepdogObjReq hdr; SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; @@ -965,6 +1023,11 @@ static int read_write_object(int fd, char *buf, uint64_t oid, int copies, rlen = datalen; hdr.opcode = SD_OP_READ_OBJ; } + + if (cache) { + hdr.flags |= SD_FLAG_CMD_CACHE; + } + hdr.oid = oid; hdr.data_length = datalen; hdr.offset = offset; @@ -986,15 +1049,18 @@ static int read_write_object(int fd, char *buf, uint64_t oid, int copies, } static int read_object(int fd, char *buf, uint64_t oid, int copies, - unsigned int datalen, uint64_t offset) + unsigned int datalen, uint64_t offset, uint8_t cache) { - return read_write_object(fd, buf, oid, copies, datalen, offset, 0, 0); + return read_write_object(fd, buf, oid, copies, datalen, offset, 0, 0, + cache); } static int write_object(int fd, char *buf, uint64_t oid, int copies, - unsigned int datalen, uint64_t offset, int create) + unsigned int datalen, uint64_t offset, int create, + uint8_t cache) { - return read_write_object(fd, buf, oid, copies, datalen, offset, 1, create); + return read_write_object(fd, buf, oid, copies, datalen, offset, 1, create, + cache); } static int sd_open(BlockDriverState *bs, const char *filename, int flags) @@ -1011,6 +1077,10 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) QLIST_INIT(&s->outstanding_aio_head); s->fd = -1; + if (flags & BDRV_O_CACHE_WB) { + s->cache_enabled = 1; + } + memset(vdi, 0, sizeof(vdi)); memset(tag, 0, sizeof(tag)); if (parse_vdiname(s, filename, vdi, &snapid, tag) < 0) { @@ -1038,7 +1108,8 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) } buf = g_malloc(SD_INODE_SIZE); - ret = read_object(fd, buf, vid_to_vdi_oid(vid), 0, SD_INODE_SIZE, 0); + ret = read_object(fd, buf, vid_to_vdi_oid(vid), 0, SD_INODE_SIZE, 0, + s->cache_enabled); closesocket(fd); @@ -1305,7 +1376,7 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset) datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id); s->inode.vdi_size = offset; ret = write_object(fd, (char *)&s->inode, vid_to_vdi_oid(s->inode.vdi_id), - s->inode.nr_copies, datalen, 0, 0); + s->inode.nr_copies, datalen, 0, 0, s->cache_enabled); close(fd); if (ret < 0) { @@ -1387,7 +1458,7 @@ static int sd_create_branch(BDRVSheepdogState *s) } ret = read_object(fd, buf, vid_to_vdi_oid(vid), s->inode.nr_copies, - SD_INODE_SIZE, 0); + SD_INODE_SIZE, 0, s->cache_enabled); closesocket(fd); @@ -1575,6 +1646,36 @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num, return acb->ret; } +static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs) +{ + BDRVSheepdogState *s = bs->opaque; + SheepdogObjReq hdr = { 0 }; + SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; + SheepdogInode *inode = &s->inode; + int ret; + unsigned int wlen = 0, rlen = 0; + + if (!s->cache_enabled) { + return 0; + } + + hdr.opcode = SD_OP_FLUSH_VDI; + hdr.oid = vid_to_vdi_oid(inode->vdi_id); + + ret = do_co_req(s->fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen); + if (ret) { + error_report("failed to send a request to the sheep"); + return ret; + } + + if (rsp->result != SD_RES_SUCCESS) { + error_report("%s", sd_strerror(rsp->result)); + return -EIO; + } + + return 0; +} + static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) { BDRVSheepdogState *s = bs->opaque; @@ -1610,7 +1711,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) } ret = write_object(fd, (char *)&s->inode, vid_to_vdi_oid(s->inode.vdi_id), - s->inode.nr_copies, datalen, 0, 0); + s->inode.nr_copies, datalen, 0, 0, s->cache_enabled); if (ret < 0) { error_report("failed to write snapshot's inode."); ret = -EIO; @@ -1629,7 +1730,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) inode = (SheepdogInode *)g_malloc(datalen); ret = read_object(fd, (char *)inode, vid_to_vdi_oid(new_vid), - s->inode.nr_copies, datalen, 0); + s->inode.nr_copies, datalen, 0, s->cache_enabled); if (ret < 0) { error_report("failed to read new inode info. %s", strerror(errno)); @@ -1684,7 +1785,7 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) buf = g_malloc(SD_INODE_SIZE); ret = read_object(fd, buf, vid_to_vdi_oid(vid), s->inode.nr_copies, - SD_INODE_SIZE, 0); + SD_INODE_SIZE, 0, s->cache_enabled); closesocket(fd); @@ -1779,7 +1880,8 @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) /* we don't need to read entire object */ ret = read_object(fd, (char *)&inode, vid_to_vdi_oid(vid), - 0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0); + 0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0, + s->cache_enabled); if (ret) { continue; @@ -1835,10 +1937,12 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data, create = (offset == 0); if (load) { ret = read_object(fd, (char *)data, vmstate_oid, - s->inode.nr_copies, data_len, offset); + s->inode.nr_copies, data_len, offset, + s->cache_enabled); } else { ret = write_object(fd, (char *)data, vmstate_oid, - s->inode.nr_copies, data_len, offset, create); + s->inode.nr_copies, data_len, offset, create, + s->cache_enabled); } if (ret < 0) { @@ -1904,6 +2008,7 @@ BlockDriver bdrv_sheepdog = { .bdrv_co_readv = sd_co_readv, .bdrv_co_writev = sd_co_writev, + .bdrv_co_flush_to_disk = sd_co_flush_to_disk, .bdrv_snapshot_create = sd_snapshot_create, .bdrv_snapshot_goto = sd_snapshot_goto, -- 1.7.8.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-03 8:35 ` [Qemu-devel] [PATCH v3] " Liu Yuan @ 2012-04-03 9:12 ` Kevin Wolf 2012-04-03 10:04 ` [Qemu-devel] RE. [PATCH] sheepdog: fix send req helpers Liu Yuan 2012-04-03 9:52 ` [Qemu-devel] [PATCH v3] sheepdog: implement SD_OP_FLUSH_VDI operation MORITA Kazutaka 1 sibling, 1 reply; 19+ messages in thread From: Kevin Wolf @ 2012-04-03 9:12 UTC (permalink / raw) To: Liu Yuan; +Cc: Stefan Hajnoczi, Michael Tokarev, qemu-devel, MORITA Kazutaka Am 03.04.2012 10:35, schrieb Liu Yuan: > From: Liu Yuan <tailai.ly@taobao.com> > > Flush operation is supposed to flush the write-back cache of > sheepdog cluster. > > By issuing flush operation, we can assure the Guest of data > reaching the sheepdog cluster storage. > > Cc: Kevin Wolf <kwolf@redhat.com> > Cc: Michael Tokarev <mjt@tls.msk.ru> > Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> > Cc: Stefan Hajnoczi <stefanha@gmail.com> > Signed-off-by: Liu Yuan <tailai.ly@taobao.com> > --- > v2 -> v3: > address Stefan Hajnoczi comments. Thanks ! > - use qemu_co_send/recv to send/revc msg > > v1 -> v2: > address Michael Tokarev comments. Thanks ! > - use per-device flag > - use bs->fd instead of 'connect_to_sdog()' This version looks okay to me now. Waiting for Kazutaka's Acked-by. > +static int send_co_req(int sockfd, SheepdogReq *hdr, void *data, > + unsigned int *wlen) > +{ > + int ret; > + > + ret = qemu_co_send(sockfd, hdr, sizeof(*hdr)); > + if (ret < sizeof(*hdr)) { > + error_report("failed to send a req, %s", strerror(errno)); > + } > + > + ret = qemu_co_send(sockfd, data, *wlen); > + if (ret < *wlen) { > + error_report("failed to send a req, %s", strerror(errno)); > + } > + > + return ret; > +} If sending the header fails, why do we still send the data? In fact, if sending the data succeeds then (which is unlikely, but who knows), we'll even return success. This only copies the logic of the existing send_req(), so if it needs to be fixed, another patch on top that fixes both instances would be okay. Kevin ^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] RE. [PATCH] sheepdog: fix send req helpers 2012-04-03 9:12 ` Kevin Wolf @ 2012-04-03 10:04 ` Liu Yuan 2012-04-03 11:13 ` MORITA Kazutaka 0 siblings, 1 reply; 19+ messages in thread From: Liu Yuan @ 2012-04-03 10:04 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, MORITA Kazutaka From: Liu Yuan <tailai.ly@taobao.com> Yes, I think so. Here is the patch. Subject: [PATCH] sheepdog: fix send req helpers From: Liu Yuan <tailai.ly@taobao.com> We should return if reading of the header fails. Cc: Kevin Wolf <kwolf@redhat.com> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> Signed-off-by: Liu Yuan <tailai.ly@taobao.com> --- block/sheepdog.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 62dfa48..73308b2 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -510,6 +510,7 @@ static int send_req(int sockfd, SheepdogReq *hdr, void *data, ret = qemu_send_full(sockfd, hdr, sizeof(*hdr), 0); if (ret < sizeof(*hdr)) { error_report("failed to send a req, %s", strerror(errno)); + return ret; } ret = qemu_send_full(sockfd, data, *wlen, 0); @@ -528,6 +529,7 @@ static int send_co_req(int sockfd, SheepdogReq *hdr, void *data, ret = qemu_co_send(sockfd, hdr, sizeof(*hdr)); if (ret < sizeof(*hdr)) { error_report("failed to send a req, %s", strerror(errno)); + return ret; } ret = qemu_co_send(sockfd, data, *wlen); -- 1.7.8.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] RE. [PATCH] sheepdog: fix send req helpers 2012-04-03 10:04 ` [Qemu-devel] RE. [PATCH] sheepdog: fix send req helpers Liu Yuan @ 2012-04-03 11:13 ` MORITA Kazutaka 2012-04-03 11:34 ` Kevin Wolf 0 siblings, 1 reply; 19+ messages in thread From: MORITA Kazutaka @ 2012-04-03 11:13 UTC (permalink / raw) To: Liu Yuan; +Cc: Kevin Wolf, qemu-devel, MORITA Kazutaka At Tue, 3 Apr 2012 18:04:21 +0800, Liu Yuan wrote: > > From: Liu Yuan <tailai.ly@taobao.com> > > Yes, I think so. Here is the patch. > > Subject: [PATCH] sheepdog: fix send req helpers > From: Liu Yuan <tailai.ly@taobao.com> > > We should return if reading of the header fails. > > Cc: Kevin Wolf <kwolf@redhat.com> > Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> > Signed-off-by: Liu Yuan <tailai.ly@taobao.com> > --- > block/sheepdog.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) Acked-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] RE. [PATCH] sheepdog: fix send req helpers 2012-04-03 11:13 ` MORITA Kazutaka @ 2012-04-03 11:34 ` Kevin Wolf 0 siblings, 0 replies; 19+ messages in thread From: Kevin Wolf @ 2012-04-03 11:34 UTC (permalink / raw) To: MORITA Kazutaka; +Cc: Liu Yuan, qemu-devel Am 03.04.2012 13:13, schrieb MORITA Kazutaka: > At Tue, 3 Apr 2012 18:04:21 +0800, > Liu Yuan wrote: >> >> From: Liu Yuan <tailai.ly@taobao.com> >> >> Yes, I think so. Here is the patch. >> >> Subject: [PATCH] sheepdog: fix send req helpers >> From: Liu Yuan <tailai.ly@taobao.com> >> >> We should return if reading of the header fails. >> >> Cc: Kevin Wolf <kwolf@redhat.com> >> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> >> Signed-off-by: Liu Yuan <tailai.ly@taobao.com> >> --- >> block/sheepdog.c | 2 ++ >> 1 files changed, 2 insertions(+), 0 deletions(-) > > Acked-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> Thanks, applied this one as well. Kevin ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v3] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-03 8:35 ` [Qemu-devel] [PATCH v3] " Liu Yuan 2012-04-03 9:12 ` Kevin Wolf @ 2012-04-03 9:52 ` MORITA Kazutaka 2012-04-03 9:58 ` [Qemu-devel] [PATCH v4] " Liu Yuan 1 sibling, 1 reply; 19+ messages in thread From: MORITA Kazutaka @ 2012-04-03 9:52 UTC (permalink / raw) To: Liu Yuan Cc: Kevin Wolf, Stefan Hajnoczi, Michael Tokarev, qemu-devel, MORITA Kazutaka At Tue, 3 Apr 2012 16:35:00 +0800, Liu Yuan wrote: > > +static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs) > +{ > + BDRVSheepdogState *s = bs->opaque; > + SheepdogObjReq hdr = { 0 }; > + SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; > + SheepdogInode *inode = &s->inode; > + int ret; > + unsigned int wlen = 0, rlen = 0; > + > + if (!s->cache_enabled) { > + return 0; > + } > + > + hdr.opcode = SD_OP_FLUSH_VDI; > + hdr.oid = vid_to_vdi_oid(inode->vdi_id); > + > + ret = do_co_req(s->fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen); This will mix the flush request with I/O requests on s->fd. We can read s->fd only in aio_read_response() and write only in add_aio_request(), so please create another connection for flush operations. Thanks, Kazutaka ^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v4] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-03 9:52 ` [Qemu-devel] [PATCH v3] sheepdog: implement SD_OP_FLUSH_VDI operation MORITA Kazutaka @ 2012-04-03 9:58 ` Liu Yuan 2012-04-03 10:08 ` [Qemu-devel] [PATCH v4 UPDATE] " Liu Yuan 0 siblings, 1 reply; 19+ messages in thread From: Liu Yuan @ 2012-04-03 9:58 UTC (permalink / raw) To: qemu-devel, sheepdog Cc: Kevin Wolf, Stefan Hajnoczi, Michael Tokarev, MORITA Kazutaka From: Liu Yuan <tailai.ly@taobao.com> Flush operation is supposed to flush the write-back cache of sheepdog cluster. By issuing flush operation, we can assure the Guest of data reaching the sheepdog cluster storage. Cc: Kevin Wolf <kwolf@redhat.com> Cc: Michael Tokarev <mjt@tls.msk.ru> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> Cc: Stefan Hajnoczi <stefanha@gmail.com> Signed-off-by: Liu Yuan <tailai.ly@taobao.com> --- v3 -> v4: address Kazutaka's comments. Thanks ! - use a dedicated fd for flush operation v2 -> v3: address Stefan Hajnoczi comments. Thanks ! - use qemu_co_send/recv to send/revc msg v1 -> v2: address Michael Tokarev comments. Thanks ! - use per-device flag - use bs->fd instead of 'connect_to_sdog()' block/sheepdog.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 128 insertions(+), 14 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 00276f6f..62dfa48 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -32,9 +32,11 @@ #define SD_OP_RELEASE_VDI 0x13 #define SD_OP_GET_VDI_INFO 0x14 #define SD_OP_READ_VDIS 0x15 +#define SD_OP_FLUSH_VDI 0x16 #define SD_FLAG_CMD_WRITE 0x01 #define SD_FLAG_CMD_COW 0x02 +#define SD_FLAG_CMD_CACHE 0x04 #define SD_RES_SUCCESS 0x00 /* Success */ #define SD_RES_UNKNOWN 0x01 /* Unknown error */ @@ -293,10 +295,12 @@ typedef struct BDRVSheepdogState { char name[SD_MAX_VDI_LEN]; int is_snapshot; + uint8_t cache_enabled; char *addr; char *port; int fd; + int flush_fd; CoMutex lock; Coroutine *co_send; @@ -516,6 +520,23 @@ static int send_req(int sockfd, SheepdogReq *hdr, void *data, return ret; } +static int send_co_req(int sockfd, SheepdogReq *hdr, void *data, + unsigned int *wlen) +{ + int ret; + + ret = qemu_co_send(sockfd, hdr, sizeof(*hdr)); + if (ret < sizeof(*hdr)) { + error_report("failed to send a req, %s", strerror(errno)); + } + + ret = qemu_co_send(sockfd, data, *wlen); + if (ret < *wlen) { + error_report("failed to send a req, %s", strerror(errno)); + } + + return ret; +} static int do_req(int sockfd, SheepdogReq *hdr, void *data, unsigned int *wlen, unsigned int *rlen) { @@ -550,6 +571,40 @@ out: return ret; } +static int do_co_req(int sockfd, SheepdogReq *hdr, void *data, + unsigned int *wlen, unsigned int *rlen) +{ + int ret; + + socket_set_block(sockfd); + ret = send_co_req(sockfd, hdr, data, wlen); + if (ret < 0) { + goto out; + } + + ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr)); + if (ret < sizeof(*hdr)) { + error_report("failed to get a rsp, %s", strerror(errno)); + goto out; + } + + if (*rlen > hdr->data_length) { + *rlen = hdr->data_length; + } + + if (*rlen) { + ret = qemu_co_recv(sockfd, data, *rlen); + if (ret < *rlen) { + error_report("failed to get the data, %s", strerror(errno)); + goto out; + } + } + ret = 0; +out: + socket_set_nonblock(sockfd); + return ret; +} + static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, struct iovec *iov, int niov, int create, enum AIOCBState aiocb_type); @@ -900,6 +955,10 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, hdr.flags = SD_FLAG_CMD_WRITE | flags; } + if (s->cache_enabled) { + hdr.flags |= SD_FLAG_CMD_CACHE; + } + hdr.oid = oid; hdr.cow_oid = old_oid; hdr.copies = s->inode.nr_copies; @@ -942,7 +1001,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, static int read_write_object(int fd, char *buf, uint64_t oid, int copies, unsigned int datalen, uint64_t offset, - int write, int create) + int write, int create, uint8_t cache) { SheepdogObjReq hdr; SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; @@ -965,6 +1024,11 @@ static int read_write_object(int fd, char *buf, uint64_t oid, int copies, rlen = datalen; hdr.opcode = SD_OP_READ_OBJ; } + + if (cache) { + hdr.flags |= SD_FLAG_CMD_CACHE; + } + hdr.oid = oid; hdr.data_length = datalen; hdr.offset = offset; @@ -986,15 +1050,18 @@ static int read_write_object(int fd, char *buf, uint64_t oid, int copies, } static int read_object(int fd, char *buf, uint64_t oid, int copies, - unsigned int datalen, uint64_t offset) + unsigned int datalen, uint64_t offset, uint8_t cache) { - return read_write_object(fd, buf, oid, copies, datalen, offset, 0, 0); + return read_write_object(fd, buf, oid, copies, datalen, offset, 0, 0, + cache); } static int write_object(int fd, char *buf, uint64_t oid, int copies, - unsigned int datalen, uint64_t offset, int create) + unsigned int datalen, uint64_t offset, int create, + uint8_t cache) { - return read_write_object(fd, buf, oid, copies, datalen, offset, 1, create); + return read_write_object(fd, buf, oid, copies, datalen, offset, 1, create, + cache); } static int sd_open(BlockDriverState *bs, const char *filename, int flags) @@ -1011,6 +1078,15 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) QLIST_INIT(&s->outstanding_aio_head); s->fd = -1; + if (flags & BDRV_O_CACHE_WB) { + s->cache_enabled = 1; + s->flush_fd = connect_to_sdog(s->addr, s->port); + if (s->flush_fd < 0) { + error_report("failed to connect"); + goto out; + } + } + memset(vdi, 0, sizeof(vdi)); memset(tag, 0, sizeof(tag)); if (parse_vdiname(s, filename, vdi, &snapid, tag) < 0) { @@ -1038,7 +1114,8 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) } buf = g_malloc(SD_INODE_SIZE); - ret = read_object(fd, buf, vid_to_vdi_oid(vid), 0, SD_INODE_SIZE, 0); + ret = read_object(fd, buf, vid_to_vdi_oid(vid), 0, SD_INODE_SIZE, 0, + s->cache_enabled); closesocket(fd); @@ -1272,6 +1349,9 @@ static void sd_close(BlockDriverState *bs) qemu_aio_set_fd_handler(s->fd, NULL, NULL, NULL, NULL, NULL); closesocket(s->fd); + if (s->cache_enabled) { + closesocket(s->flush_fd); + } g_free(s->addr); } @@ -1305,7 +1385,7 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset) datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id); s->inode.vdi_size = offset; ret = write_object(fd, (char *)&s->inode, vid_to_vdi_oid(s->inode.vdi_id), - s->inode.nr_copies, datalen, 0, 0); + s->inode.nr_copies, datalen, 0, 0, s->cache_enabled); close(fd); if (ret < 0) { @@ -1387,7 +1467,7 @@ static int sd_create_branch(BDRVSheepdogState *s) } ret = read_object(fd, buf, vid_to_vdi_oid(vid), s->inode.nr_copies, - SD_INODE_SIZE, 0); + SD_INODE_SIZE, 0, s->cache_enabled); closesocket(fd); @@ -1575,6 +1655,36 @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num, return acb->ret; } +static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs) +{ + BDRVSheepdogState *s = bs->opaque; + SheepdogObjReq hdr = { 0 }; + SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; + SheepdogInode *inode = &s->inode; + int ret; + unsigned int wlen = 0, rlen = 0; + + if (!s->cache_enabled) { + return 0; + } + + hdr.opcode = SD_OP_FLUSH_VDI; + hdr.oid = vid_to_vdi_oid(inode->vdi_id); + + ret = do_co_req(s->fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen); + if (ret) { + error_report("failed to send a request to the sheep"); + return ret; + } + + if (rsp->result != SD_RES_SUCCESS) { + error_report("%s", sd_strerror(rsp->result)); + return -EIO; + } + + return 0; +} + static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) { BDRVSheepdogState *s = bs->opaque; @@ -1610,7 +1720,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) } ret = write_object(fd, (char *)&s->inode, vid_to_vdi_oid(s->inode.vdi_id), - s->inode.nr_copies, datalen, 0, 0); + s->inode.nr_copies, datalen, 0, 0, s->cache_enabled); if (ret < 0) { error_report("failed to write snapshot's inode."); ret = -EIO; @@ -1629,7 +1739,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) inode = (SheepdogInode *)g_malloc(datalen); ret = read_object(fd, (char *)inode, vid_to_vdi_oid(new_vid), - s->inode.nr_copies, datalen, 0); + s->inode.nr_copies, datalen, 0, s->cache_enabled); if (ret < 0) { error_report("failed to read new inode info. %s", strerror(errno)); @@ -1684,7 +1794,7 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) buf = g_malloc(SD_INODE_SIZE); ret = read_object(fd, buf, vid_to_vdi_oid(vid), s->inode.nr_copies, - SD_INODE_SIZE, 0); + SD_INODE_SIZE, 0, s->cache_enabled); closesocket(fd); @@ -1779,7 +1889,8 @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) /* we don't need to read entire object */ ret = read_object(fd, (char *)&inode, vid_to_vdi_oid(vid), - 0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0); + 0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0, + s->cache_enabled); if (ret) { continue; @@ -1835,10 +1946,12 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data, create = (offset == 0); if (load) { ret = read_object(fd, (char *)data, vmstate_oid, - s->inode.nr_copies, data_len, offset); + s->inode.nr_copies, data_len, offset, + s->cache_enabled); } else { ret = write_object(fd, (char *)data, vmstate_oid, - s->inode.nr_copies, data_len, offset, create); + s->inode.nr_copies, data_len, offset, create, + s->cache_enabled); } if (ret < 0) { @@ -1904,6 +2017,7 @@ BlockDriver bdrv_sheepdog = { .bdrv_co_readv = sd_co_readv, .bdrv_co_writev = sd_co_writev, + .bdrv_co_flush_to_disk = sd_co_flush_to_disk, .bdrv_snapshot_create = sd_snapshot_create, .bdrv_snapshot_goto = sd_snapshot_goto, -- 1.7.8.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v4 UPDATE] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-03 9:58 ` [Qemu-devel] [PATCH v4] " Liu Yuan @ 2012-04-03 10:08 ` Liu Yuan 2012-04-03 10:32 ` MORITA Kazutaka 0 siblings, 1 reply; 19+ messages in thread From: Liu Yuan @ 2012-04-03 10:08 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Michael Tokarev, MORITA Kazutaka From: Liu Yuan <tailai.ly@taobao.com> Flush operation is supposed to flush the write-back cache of sheepdog cluster. By issuing flush operation, we can assure the Guest of data reaching the sheepdog cluster storage. Cc: Kevin Wolf <kwolf@redhat.com> Cc: Michael Tokarev <mjt@tls.msk.ru> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> Cc: Stefan Hajnoczi <stefanha@gmail.com> Signed-off-by: Liu Yuan <tailai.ly@taobao.com> --- UPDATE: use flush_fd instead of s->fd. block/sheepdog.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 128 insertions(+), 14 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 00276f6f..da3afbf 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -32,9 +32,11 @@ #define SD_OP_RELEASE_VDI 0x13 #define SD_OP_GET_VDI_INFO 0x14 #define SD_OP_READ_VDIS 0x15 +#define SD_OP_FLUSH_VDI 0x16 #define SD_FLAG_CMD_WRITE 0x01 #define SD_FLAG_CMD_COW 0x02 +#define SD_FLAG_CMD_CACHE 0x04 #define SD_RES_SUCCESS 0x00 /* Success */ #define SD_RES_UNKNOWN 0x01 /* Unknown error */ @@ -293,10 +295,12 @@ typedef struct BDRVSheepdogState { char name[SD_MAX_VDI_LEN]; int is_snapshot; + uint8_t cache_enabled; char *addr; char *port; int fd; + int flush_fd; CoMutex lock; Coroutine *co_send; @@ -516,6 +520,23 @@ static int send_req(int sockfd, SheepdogReq *hdr, void *data, return ret; } +static int send_co_req(int sockfd, SheepdogReq *hdr, void *data, + unsigned int *wlen) +{ + int ret; + + ret = qemu_co_send(sockfd, hdr, sizeof(*hdr)); + if (ret < sizeof(*hdr)) { + error_report("failed to send a req, %s", strerror(errno)); + } + + ret = qemu_co_send(sockfd, data, *wlen); + if (ret < *wlen) { + error_report("failed to send a req, %s", strerror(errno)); + } + + return ret; +} static int do_req(int sockfd, SheepdogReq *hdr, void *data, unsigned int *wlen, unsigned int *rlen) { @@ -550,6 +571,40 @@ out: return ret; } +static int do_co_req(int sockfd, SheepdogReq *hdr, void *data, + unsigned int *wlen, unsigned int *rlen) +{ + int ret; + + socket_set_block(sockfd); + ret = send_co_req(sockfd, hdr, data, wlen); + if (ret < 0) { + goto out; + } + + ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr)); + if (ret < sizeof(*hdr)) { + error_report("failed to get a rsp, %s", strerror(errno)); + goto out; + } + + if (*rlen > hdr->data_length) { + *rlen = hdr->data_length; + } + + if (*rlen) { + ret = qemu_co_recv(sockfd, data, *rlen); + if (ret < *rlen) { + error_report("failed to get the data, %s", strerror(errno)); + goto out; + } + } + ret = 0; +out: + socket_set_nonblock(sockfd); + return ret; +} + static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, struct iovec *iov, int niov, int create, enum AIOCBState aiocb_type); @@ -900,6 +955,10 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, hdr.flags = SD_FLAG_CMD_WRITE | flags; } + if (s->cache_enabled) { + hdr.flags |= SD_FLAG_CMD_CACHE; + } + hdr.oid = oid; hdr.cow_oid = old_oid; hdr.copies = s->inode.nr_copies; @@ -942,7 +1001,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, static int read_write_object(int fd, char *buf, uint64_t oid, int copies, unsigned int datalen, uint64_t offset, - int write, int create) + int write, int create, uint8_t cache) { SheepdogObjReq hdr; SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; @@ -965,6 +1024,11 @@ static int read_write_object(int fd, char *buf, uint64_t oid, int copies, rlen = datalen; hdr.opcode = SD_OP_READ_OBJ; } + + if (cache) { + hdr.flags |= SD_FLAG_CMD_CACHE; + } + hdr.oid = oid; hdr.data_length = datalen; hdr.offset = offset; @@ -986,15 +1050,18 @@ static int read_write_object(int fd, char *buf, uint64_t oid, int copies, } static int read_object(int fd, char *buf, uint64_t oid, int copies, - unsigned int datalen, uint64_t offset) + unsigned int datalen, uint64_t offset, uint8_t cache) { - return read_write_object(fd, buf, oid, copies, datalen, offset, 0, 0); + return read_write_object(fd, buf, oid, copies, datalen, offset, 0, 0, + cache); } static int write_object(int fd, char *buf, uint64_t oid, int copies, - unsigned int datalen, uint64_t offset, int create) + unsigned int datalen, uint64_t offset, int create, + uint8_t cache) { - return read_write_object(fd, buf, oid, copies, datalen, offset, 1, create); + return read_write_object(fd, buf, oid, copies, datalen, offset, 1, create, + cache); } static int sd_open(BlockDriverState *bs, const char *filename, int flags) @@ -1011,6 +1078,15 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) QLIST_INIT(&s->outstanding_aio_head); s->fd = -1; + if (flags & BDRV_O_CACHE_WB) { + s->cache_enabled = 1; + s->flush_fd = connect_to_sdog(s->addr, s->port); + if (s->flush_fd < 0) { + error_report("failed to connect"); + goto out; + } + } + memset(vdi, 0, sizeof(vdi)); memset(tag, 0, sizeof(tag)); if (parse_vdiname(s, filename, vdi, &snapid, tag) < 0) { @@ -1038,7 +1114,8 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) } buf = g_malloc(SD_INODE_SIZE); - ret = read_object(fd, buf, vid_to_vdi_oid(vid), 0, SD_INODE_SIZE, 0); + ret = read_object(fd, buf, vid_to_vdi_oid(vid), 0, SD_INODE_SIZE, 0, + s->cache_enabled); closesocket(fd); @@ -1272,6 +1349,9 @@ static void sd_close(BlockDriverState *bs) qemu_aio_set_fd_handler(s->fd, NULL, NULL, NULL, NULL, NULL); closesocket(s->fd); + if (s->cache_enabled) { + closesocket(s->flush_fd); + } g_free(s->addr); } @@ -1305,7 +1385,7 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset) datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id); s->inode.vdi_size = offset; ret = write_object(fd, (char *)&s->inode, vid_to_vdi_oid(s->inode.vdi_id), - s->inode.nr_copies, datalen, 0, 0); + s->inode.nr_copies, datalen, 0, 0, s->cache_enabled); close(fd); if (ret < 0) { @@ -1387,7 +1467,7 @@ static int sd_create_branch(BDRVSheepdogState *s) } ret = read_object(fd, buf, vid_to_vdi_oid(vid), s->inode.nr_copies, - SD_INODE_SIZE, 0); + SD_INODE_SIZE, 0, s->cache_enabled); closesocket(fd); @@ -1575,6 +1655,36 @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num, return acb->ret; } +static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs) +{ + BDRVSheepdogState *s = bs->opaque; + SheepdogObjReq hdr = { 0 }; + SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; + SheepdogInode *inode = &s->inode; + int ret; + unsigned int wlen = 0, rlen = 0; + + if (!s->cache_enabled) { + return 0; + } + + hdr.opcode = SD_OP_FLUSH_VDI; + hdr.oid = vid_to_vdi_oid(inode->vdi_id); + + ret = do_co_req(s->flush_fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen); + if (ret) { + error_report("failed to send a request to the sheep"); + return ret; + } + + if (rsp->result != SD_RES_SUCCESS) { + error_report("%s", sd_strerror(rsp->result)); + return -EIO; + } + + return 0; +} + static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) { BDRVSheepdogState *s = bs->opaque; @@ -1610,7 +1720,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) } ret = write_object(fd, (char *)&s->inode, vid_to_vdi_oid(s->inode.vdi_id), - s->inode.nr_copies, datalen, 0, 0); + s->inode.nr_copies, datalen, 0, 0, s->cache_enabled); if (ret < 0) { error_report("failed to write snapshot's inode."); ret = -EIO; @@ -1629,7 +1739,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) inode = (SheepdogInode *)g_malloc(datalen); ret = read_object(fd, (char *)inode, vid_to_vdi_oid(new_vid), - s->inode.nr_copies, datalen, 0); + s->inode.nr_copies, datalen, 0, s->cache_enabled); if (ret < 0) { error_report("failed to read new inode info. %s", strerror(errno)); @@ -1684,7 +1794,7 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) buf = g_malloc(SD_INODE_SIZE); ret = read_object(fd, buf, vid_to_vdi_oid(vid), s->inode.nr_copies, - SD_INODE_SIZE, 0); + SD_INODE_SIZE, 0, s->cache_enabled); closesocket(fd); @@ -1779,7 +1889,8 @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) /* we don't need to read entire object */ ret = read_object(fd, (char *)&inode, vid_to_vdi_oid(vid), - 0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0); + 0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0, + s->cache_enabled); if (ret) { continue; @@ -1835,10 +1946,12 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data, create = (offset == 0); if (load) { ret = read_object(fd, (char *)data, vmstate_oid, - s->inode.nr_copies, data_len, offset); + s->inode.nr_copies, data_len, offset, + s->cache_enabled); } else { ret = write_object(fd, (char *)data, vmstate_oid, - s->inode.nr_copies, data_len, offset, create); + s->inode.nr_copies, data_len, offset, create, + s->cache_enabled); } if (ret < 0) { @@ -1904,6 +2017,7 @@ BlockDriver bdrv_sheepdog = { .bdrv_co_readv = sd_co_readv, .bdrv_co_writev = sd_co_writev, + .bdrv_co_flush_to_disk = sd_co_flush_to_disk, .bdrv_snapshot_create = sd_snapshot_create, .bdrv_snapshot_goto = sd_snapshot_goto, -- 1.7.8.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 UPDATE] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-03 10:08 ` [Qemu-devel] [PATCH v4 UPDATE] " Liu Yuan @ 2012-04-03 10:32 ` MORITA Kazutaka 2012-04-03 10:30 ` Kevin Wolf 0 siblings, 1 reply; 19+ messages in thread From: MORITA Kazutaka @ 2012-04-03 10:32 UTC (permalink / raw) To: Liu Yuan Cc: Kevin Wolf, Stefan Hajnoczi, Michael Tokarev, qemu-devel, MORITA Kazutaka At Tue, 3 Apr 2012 18:08:10 +0800, Liu Yuan wrote: > > From: Liu Yuan <tailai.ly@taobao.com> > > Flush operation is supposed to flush the write-back cache of > sheepdog cluster. > > By issuing flush operation, we can assure the Guest of data > reaching the sheepdog cluster storage. > > Cc: Kevin Wolf <kwolf@redhat.com> > Cc: Michael Tokarev <mjt@tls.msk.ru> > Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> > Cc: Stefan Hajnoczi <stefanha@gmail.com> > Signed-off-by: Liu Yuan <tailai.ly@taobao.com> > --- > UPDATE: use flush_fd instead of s->fd. > > block/sheepdog.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++----- > 1 files changed, 128 insertions(+), 14 deletions(-) Thanks, looks good to me. Acked-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v4 UPDATE] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-03 10:32 ` MORITA Kazutaka @ 2012-04-03 10:30 ` Kevin Wolf 2012-04-03 20:03 ` [Qemu-devel] [PATCH v5] " Liu Yuan 0 siblings, 1 reply; 19+ messages in thread From: Kevin Wolf @ 2012-04-03 10:30 UTC (permalink / raw) To: MORITA Kazutaka; +Cc: Liu Yuan, Michael Tokarev, qemu-devel, Stefan Hajnoczi Am 03.04.2012 12:32, schrieb MORITA Kazutaka: > At Tue, 3 Apr 2012 18:08:10 +0800, > Liu Yuan wrote: >> >> From: Liu Yuan <tailai.ly@taobao.com> >> >> Flush operation is supposed to flush the write-back cache of >> sheepdog cluster. >> >> By issuing flush operation, we can assure the Guest of data >> reaching the sheepdog cluster storage. >> >> Cc: Kevin Wolf <kwolf@redhat.com> >> Cc: Michael Tokarev <mjt@tls.msk.ru> >> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> >> Cc: Stefan Hajnoczi <stefanha@gmail.com> >> Signed-off-by: Liu Yuan <tailai.ly@taobao.com> >> --- >> UPDATE: use flush_fd instead of s->fd. >> >> block/sheepdog.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++----- >> 1 files changed, 128 insertions(+), 14 deletions(-) > > Thanks, looks good to me. > > Acked-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> Thanks, applied to the block branch. Kevin ^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v5] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-03 10:30 ` Kevin Wolf @ 2012-04-03 20:03 ` Liu Yuan 2012-04-04 8:53 ` Kevin Wolf 0 siblings, 1 reply; 19+ messages in thread From: Liu Yuan @ 2012-04-03 20:03 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, MORITA Kazutaka From: Liu Yuan <tailai.ly@taobao.com> Flush operation is supposed to flush the write-back cache of sheepdog cluster. By issuing flush operation, we can assure the Guest of data reaching the sheepdog cluster storage. Cc: Kevin Wolf <kwolf@redhat.com> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> Signed-off-by: Liu Yuan <tailai.ly@taobao.com> --- Hi Kevin, Sorry, I found a problem in v4 patch, that s->{addr, port} isn't properly initilized. If you think it is inconvenient for you to update this patch, please let me know and I'll submit a new patch. block/sheepdog.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 128 insertions(+), 14 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index 00276f6f..1248534 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -32,9 +32,11 @@ #define SD_OP_RELEASE_VDI 0x13 #define SD_OP_GET_VDI_INFO 0x14 #define SD_OP_READ_VDIS 0x15 +#define SD_OP_FLUSH_VDI 0x16 #define SD_FLAG_CMD_WRITE 0x01 #define SD_FLAG_CMD_COW 0x02 +#define SD_FLAG_CMD_CACHE 0x04 #define SD_RES_SUCCESS 0x00 /* Success */ #define SD_RES_UNKNOWN 0x01 /* Unknown error */ @@ -293,10 +295,12 @@ typedef struct BDRVSheepdogState { char name[SD_MAX_VDI_LEN]; int is_snapshot; + uint8_t cache_enabled; char *addr; char *port; int fd; + int flush_fd; CoMutex lock; Coroutine *co_send; @@ -516,6 +520,23 @@ static int send_req(int sockfd, SheepdogReq *hdr, void *data, return ret; } +static int send_co_req(int sockfd, SheepdogReq *hdr, void *data, + unsigned int *wlen) +{ + int ret; + + ret = qemu_co_send(sockfd, hdr, sizeof(*hdr)); + if (ret < sizeof(*hdr)) { + error_report("failed to send a req, %s", strerror(errno)); + } + + ret = qemu_co_send(sockfd, data, *wlen); + if (ret < *wlen) { + error_report("failed to send a req, %s", strerror(errno)); + } + + return ret; +} static int do_req(int sockfd, SheepdogReq *hdr, void *data, unsigned int *wlen, unsigned int *rlen) { @@ -550,6 +571,40 @@ out: return ret; } +static int do_co_req(int sockfd, SheepdogReq *hdr, void *data, + unsigned int *wlen, unsigned int *rlen) +{ + int ret; + + socket_set_block(sockfd); + ret = send_co_req(sockfd, hdr, data, wlen); + if (ret < 0) { + goto out; + } + + ret = qemu_co_recv(sockfd, hdr, sizeof(*hdr)); + if (ret < sizeof(*hdr)) { + error_report("failed to get a rsp, %s", strerror(errno)); + goto out; + } + + if (*rlen > hdr->data_length) { + *rlen = hdr->data_length; + } + + if (*rlen) { + ret = qemu_co_recv(sockfd, data, *rlen); + if (ret < *rlen) { + error_report("failed to get the data, %s", strerror(errno)); + goto out; + } + } + ret = 0; +out: + socket_set_nonblock(sockfd); + return ret; +} + static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, struct iovec *iov, int niov, int create, enum AIOCBState aiocb_type); @@ -900,6 +955,10 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, hdr.flags = SD_FLAG_CMD_WRITE | flags; } + if (s->cache_enabled) { + hdr.flags |= SD_FLAG_CMD_CACHE; + } + hdr.oid = oid; hdr.cow_oid = old_oid; hdr.copies = s->inode.nr_copies; @@ -942,7 +1001,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req, static int read_write_object(int fd, char *buf, uint64_t oid, int copies, unsigned int datalen, uint64_t offset, - int write, int create) + int write, int create, uint8_t cache) { SheepdogObjReq hdr; SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; @@ -965,6 +1024,11 @@ static int read_write_object(int fd, char *buf, uint64_t oid, int copies, rlen = datalen; hdr.opcode = SD_OP_READ_OBJ; } + + if (cache) { + hdr.flags |= SD_FLAG_CMD_CACHE; + } + hdr.oid = oid; hdr.data_length = datalen; hdr.offset = offset; @@ -986,15 +1050,18 @@ static int read_write_object(int fd, char *buf, uint64_t oid, int copies, } static int read_object(int fd, char *buf, uint64_t oid, int copies, - unsigned int datalen, uint64_t offset) + unsigned int datalen, uint64_t offset, uint8_t cache) { - return read_write_object(fd, buf, oid, copies, datalen, offset, 0, 0); + return read_write_object(fd, buf, oid, copies, datalen, offset, 0, 0, + cache); } static int write_object(int fd, char *buf, uint64_t oid, int copies, - unsigned int datalen, uint64_t offset, int create) + unsigned int datalen, uint64_t offset, int create, + uint8_t cache) { - return read_write_object(fd, buf, oid, copies, datalen, offset, 1, create); + return read_write_object(fd, buf, oid, copies, datalen, offset, 1, create, + cache); } static int sd_open(BlockDriverState *bs, const char *filename, int flags) @@ -1026,6 +1093,15 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) goto out; } + if (flags & BDRV_O_CACHE_WB) { + s->cache_enabled = 1; + s->flush_fd = connect_to_sdog(s->addr, s->port); + if (s->flush_fd < 0) { + error_report("failed to connect"); + goto out; + } + } + if (snapid) { dprintf("%" PRIx32 " snapshot inode was open.\n", vid); s->is_snapshot = 1; @@ -1038,7 +1114,8 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) } buf = g_malloc(SD_INODE_SIZE); - ret = read_object(fd, buf, vid_to_vdi_oid(vid), 0, SD_INODE_SIZE, 0); + ret = read_object(fd, buf, vid_to_vdi_oid(vid), 0, SD_INODE_SIZE, 0, + s->cache_enabled); closesocket(fd); @@ -1272,6 +1349,9 @@ static void sd_close(BlockDriverState *bs) qemu_aio_set_fd_handler(s->fd, NULL, NULL, NULL, NULL, NULL); closesocket(s->fd); + if (s->cache_enabled) { + closesocket(s->flush_fd); + } g_free(s->addr); } @@ -1305,7 +1385,7 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset) datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id); s->inode.vdi_size = offset; ret = write_object(fd, (char *)&s->inode, vid_to_vdi_oid(s->inode.vdi_id), - s->inode.nr_copies, datalen, 0, 0); + s->inode.nr_copies, datalen, 0, 0, s->cache_enabled); close(fd); if (ret < 0) { @@ -1387,7 +1467,7 @@ static int sd_create_branch(BDRVSheepdogState *s) } ret = read_object(fd, buf, vid_to_vdi_oid(vid), s->inode.nr_copies, - SD_INODE_SIZE, 0); + SD_INODE_SIZE, 0, s->cache_enabled); closesocket(fd); @@ -1575,6 +1655,36 @@ static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num, return acb->ret; } +static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs) +{ + BDRVSheepdogState *s = bs->opaque; + SheepdogObjReq hdr = { 0 }; + SheepdogObjRsp *rsp = (SheepdogObjRsp *)&hdr; + SheepdogInode *inode = &s->inode; + int ret; + unsigned int wlen = 0, rlen = 0; + + if (!s->cache_enabled) { + return 0; + } + + hdr.opcode = SD_OP_FLUSH_VDI; + hdr.oid = vid_to_vdi_oid(inode->vdi_id); + + ret = do_co_req(s->flush_fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen); + if (ret) { + error_report("failed to send a request to the sheep"); + return ret; + } + + if (rsp->result != SD_RES_SUCCESS) { + error_report("%s", sd_strerror(rsp->result)); + return -EIO; + } + + return 0; +} + static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) { BDRVSheepdogState *s = bs->opaque; @@ -1610,7 +1720,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) } ret = write_object(fd, (char *)&s->inode, vid_to_vdi_oid(s->inode.vdi_id), - s->inode.nr_copies, datalen, 0, 0); + s->inode.nr_copies, datalen, 0, 0, s->cache_enabled); if (ret < 0) { error_report("failed to write snapshot's inode."); ret = -EIO; @@ -1629,7 +1739,7 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) inode = (SheepdogInode *)g_malloc(datalen); ret = read_object(fd, (char *)inode, vid_to_vdi_oid(new_vid), - s->inode.nr_copies, datalen, 0); + s->inode.nr_copies, datalen, 0, s->cache_enabled); if (ret < 0) { error_report("failed to read new inode info. %s", strerror(errno)); @@ -1684,7 +1794,7 @@ static int sd_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) buf = g_malloc(SD_INODE_SIZE); ret = read_object(fd, buf, vid_to_vdi_oid(vid), s->inode.nr_copies, - SD_INODE_SIZE, 0); + SD_INODE_SIZE, 0, s->cache_enabled); closesocket(fd); @@ -1779,7 +1889,8 @@ static int sd_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab) /* we don't need to read entire object */ ret = read_object(fd, (char *)&inode, vid_to_vdi_oid(vid), - 0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0); + 0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0, + s->cache_enabled); if (ret) { continue; @@ -1835,10 +1946,12 @@ static int do_load_save_vmstate(BDRVSheepdogState *s, uint8_t *data, create = (offset == 0); if (load) { ret = read_object(fd, (char *)data, vmstate_oid, - s->inode.nr_copies, data_len, offset); + s->inode.nr_copies, data_len, offset, + s->cache_enabled); } else { ret = write_object(fd, (char *)data, vmstate_oid, - s->inode.nr_copies, data_len, offset, create); + s->inode.nr_copies, data_len, offset, create, + s->cache_enabled); } if (ret < 0) { @@ -1904,6 +2017,7 @@ BlockDriver bdrv_sheepdog = { .bdrv_co_readv = sd_co_readv, .bdrv_co_writev = sd_co_writev, + .bdrv_co_flush_to_disk = sd_co_flush_to_disk, .bdrv_snapshot_create = sd_snapshot_create, .bdrv_snapshot_goto = sd_snapshot_goto, -- 1.7.8.2 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-03 20:03 ` [Qemu-devel] [PATCH v5] " Liu Yuan @ 2012-04-04 8:53 ` Kevin Wolf 2012-04-04 8:58 ` Liu Yuan 0 siblings, 1 reply; 19+ messages in thread From: Kevin Wolf @ 2012-04-04 8:53 UTC (permalink / raw) To: Liu Yuan; +Cc: qemu-devel, MORITA Kazutaka Am 03.04.2012 22:03, schrieb Liu Yuan: > From: Liu Yuan <tailai.ly@taobao.com> > > Flush operation is supposed to flush the write-back cache of > sheepdog cluster. > > By issuing flush operation, we can assure the Guest of data > reaching the sheepdog cluster storage. > > Cc: Kevin Wolf <kwolf@redhat.com> > Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp> > Signed-off-by: Liu Yuan <tailai.ly@taobao.com> > --- > Hi Kevin, > Sorry, I found a problem in v4 patch, that s->{addr, port} isn't properly > initilized. > If you think it is inconvenient for you to update this patch, please let > me know and I'll submit a new patch. I can update it in my queue, no problem there. But I diffed this against v4 and it doesn't seem to have any changes, except in the context of two hunks. Are you sure that you didn't do the fixes in a separate commit or send the wrong patch or something like that? Kevin ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-04 8:53 ` Kevin Wolf @ 2012-04-04 8:58 ` Liu Yuan 2012-04-04 9:20 ` Kevin Wolf 0 siblings, 1 reply; 19+ messages in thread From: Liu Yuan @ 2012-04-04 8:58 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-devel, MORITA Kazutaka Hi kevin, On 04/04/2012 04:53 PM, Kevin Wolf wrote: > I can update it in my queue, no problem there. > > But I diffed this against v4 and it doesn't seem to have any changes, > except in the context of two hunks. Are you sure that you didn't do the > fixes in a separate commit or send the wrong patch or something like that? V5 patch just reordered some of lines in sd_open() see below: static int sd_open(BlockDriverState *bs, const char *filename, int flags) @@ -1026,6 +1093,15 @@ static int sd_open(BlockDriverState *bs, const char *filename, int flags) goto out; } + if (flags & BDRV_O_CACHE_WB) { + s->cache_enabled = 1; + s->flush_fd = connect_to_sdog(s->addr, s->port); + if (s->flush_fd < 0) { + error_report("failed to connect"); + goto out; + } + } + if (snapid) { dprintf("%" PRIx32 " snapshot inode was open.\n", vid); s->is_snapshot = 1; s->{addr,port} is initilized in find_vdi_name(), so flush_fd should be initilized after it. I checked the patch again, V5 patch is no problem. Thanks, Yuan ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v5] sheepdog: implement SD_OP_FLUSH_VDI operation 2012-04-04 8:58 ` Liu Yuan @ 2012-04-04 9:20 ` Kevin Wolf 0 siblings, 0 replies; 19+ messages in thread From: Kevin Wolf @ 2012-04-04 9:20 UTC (permalink / raw) To: Liu Yuan; +Cc: qemu-devel, MORITA Kazutaka Am 04.04.2012 10:58, schrieb Liu Yuan: > Hi kevin, > > On 04/04/2012 04:53 PM, Kevin Wolf wrote: > >> I can update it in my queue, no problem there. >> >> But I diffed this against v4 and it doesn't seem to have any changes, >> except in the context of two hunks. Are you sure that you didn't do the >> fixes in a separate commit or send the wrong patch or something like that? > > > V5 patch just reordered some of lines in sd_open() see below: > [...] > > s->{addr,port} is initilized in find_vdi_name(), so flush_fd should be > initilized after it. Yes, of course. It seems to be still too early in the morning... Thanks, I updated the patch in my queue. Kevin ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-04-04 9:16 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-04-02 17:35 [Qemu-devel] [PATCH] sheepdog: implement SD_OP_FLUSH_VDI operation Liu Yuan 2012-04-02 20:01 ` Michael Tokarev 2012-04-03 5:14 ` Liu Yuan 2012-04-03 5:15 ` [Qemu-devel] [PATCH v2] " Liu Yuan 2012-04-03 7:23 ` Stefan Hajnoczi 2012-04-03 8:35 ` [Qemu-devel] [PATCH v3] " Liu Yuan 2012-04-03 9:12 ` Kevin Wolf 2012-04-03 10:04 ` [Qemu-devel] RE. [PATCH] sheepdog: fix send req helpers Liu Yuan 2012-04-03 11:13 ` MORITA Kazutaka 2012-04-03 11:34 ` Kevin Wolf 2012-04-03 9:52 ` [Qemu-devel] [PATCH v3] sheepdog: implement SD_OP_FLUSH_VDI operation MORITA Kazutaka 2012-04-03 9:58 ` [Qemu-devel] [PATCH v4] " Liu Yuan 2012-04-03 10:08 ` [Qemu-devel] [PATCH v4 UPDATE] " Liu Yuan 2012-04-03 10:32 ` MORITA Kazutaka 2012-04-03 10:30 ` Kevin Wolf 2012-04-03 20:03 ` [Qemu-devel] [PATCH v5] " Liu Yuan 2012-04-04 8:53 ` Kevin Wolf 2012-04-04 8:58 ` Liu Yuan 2012-04-04 9:20 ` Kevin Wolf
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).