From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50481) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEw5B-0000ip-Fg for qemu-devel@nongnu.org; Tue, 03 Apr 2012 01:15:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SEw58-0003t2-JQ for qemu-devel@nongnu.org; Tue, 03 Apr 2012 01:15:05 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:47107) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEw58-0003pt-AG for qemu-devel@nongnu.org; Tue, 03 Apr 2012 01:15:02 -0400 Received: by pbcuo5 with SMTP id uo5so5855594pbc.4 for ; Mon, 02 Apr 2012 22:15:00 -0700 (PDT) Message-ID: <4F7A874E.5050404@gmail.com> Date: Tue, 03 Apr 2012 13:14:54 +0800 From: Liu Yuan MIME-Version: 1.0 References: <1333388150-12491-1-git-send-email-namei.unix@gmail.com> <4F7A05A2.4010301@msgid.tls.msk.ru> In-Reply-To: <4F7A05A2.4010301@msgid.tls.msk.ru> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] sheepdog: implement SD_OP_FLUSH_VDI operation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev Cc: Kevin Wolf , qemu-devel@nongnu.org, MORITA Kazutaka On 04/03/2012 04:01 AM, Michael Tokarev wrote: > On 02.04.2012 21:35, Liu Yuan wrote: >> From: Liu Yuan >> >> 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 >> Reviewd-by: MORITA Kazutaka >> Signed-off-by: Liu Yuan >> --- >> 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