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? > +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