On 04/03/2012 04:01 AM, Michael Tokarev wrote: > On 02.04.2012 21:35, Liu Yuan wrote: >> From: Liu Yuan <tailai...@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 <kw...@redhat.com> >> Reviewd-by: MORITA Kazutaka <morita.kazut...@lab.ntt.co.jp> >> Signed-off-by: Liu Yuan <tailai...@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