diff mbox

[1/2] sheepdog: multiplex the rw FD to flush cache

Message ID 1358143263-14565-1-git-send-email-namei.unix@gmail.com
State New
Headers show

Commit Message

Liu Yuan Jan. 14, 2013, 6:01 a.m. UTC 
From: Liu Yuan <tailai.ly@taobao.com>

This will reduce sockfds connected to the sheep server to one, which simply the
future hacks.

Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Liu Yuan <tailai.ly@taobao.com>
---
 block/sheepdog.c |   64 +++++++++++++++++++++++-------------------------------
 1 file changed, 27 insertions(+), 37 deletions(-)

Comments

Stefan Hajnoczi Jan. 15, 2013, 7:21 a.m. UTC | #1 
On Mon, Jan 14, 2013 at 02:01:02PM +0800, Liu Yuan wrote:
> From: Liu Yuan <tailai.ly@taobao.com>
> 
> This will reduce sockfds connected to the sheep server to one, which simply the
> future hacks.
> 
> Cc: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Liu Yuan <tailai.ly@taobao.com>
> ---
>  block/sheepdog.c |   64 +++++++++++++++++++++++-------------------------------
>  1 file changed, 27 insertions(+), 37 deletions(-)

This series looks fine to me but I'd like to give Kazutaka a chance to
review.

Stefan
MORITA Kazutaka Jan. 15, 2013, 8:15 a.m. UTC | #2 
At Mon, 14 Jan 2013 14:01:02 +0800,
Liu Yuan wrote:
> 
> @@ -964,7 +971,10 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
>  
>      memset(&hdr, 0, sizeof(hdr));
>  
> -    if (aiocb_type == AIOCB_READ_UDATA) {
> +    if (aiocb_type == AIOCB_FLUSH_CACHE) {
> +        wlen = 0;
> +        hdr.opcode = SD_OP_FLUSH_VDI;
> +    } else if (aiocb_type == AIOCB_READ_UDATA) {
>          wlen = 0;
>          hdr.opcode = SD_OP_READ_OBJ;
>          hdr.flags = flags;

I think it's better to use a switch statement here.

The other parts look good to me.

Thanks,

Kazutaka
diff mbox

Patch

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 462c4b2..4e38670 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -266,6 +266,7 @@  typedef struct AIOReq {
 enum AIOCBState {
     AIOCB_WRITE_UDATA,
     AIOCB_READ_UDATA,
+    AIOCB_FLUSH_CACHE,
 };
 
 struct SheepdogAIOCB {
@@ -299,7 +300,6 @@  typedef struct BDRVSheepdogState {
     char *addr;
     char *port;
     int fd;
-    int flush_fd;
 
     CoMutex lock;
     Coroutine *co_send;
@@ -736,6 +736,13 @@  static void coroutine_fn aio_read_response(void *opaque)
             goto out;
         }
         break;
+    case AIOCB_FLUSH_CACHE:
+        if (rsp.result == SD_RES_INVALID_PARMS) {
+            dprintf("disable cache since the server doesn't support it\n");
+            s->cache_flags = SD_FLAG_CMD_DIRECT;
+            rsp.result = SD_RES_SUCCESS;
+        }
+        break;
     }
 
     if (rsp.result != SD_RES_SUCCESS) {
@@ -964,7 +971,10 @@  static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
 
     memset(&hdr, 0, sizeof(hdr));
 
-    if (aiocb_type == AIOCB_READ_UDATA) {
+    if (aiocb_type == AIOCB_FLUSH_CACHE) {
+        wlen = 0;
+        hdr.opcode = SD_OP_FLUSH_VDI;
+    } else if (aiocb_type == AIOCB_READ_UDATA) {
         wlen = 0;
         hdr.opcode = SD_OP_READ_OBJ;
         hdr.flags = flags;
@@ -1127,15 +1137,6 @@  static int sd_open(BlockDriverState *bs, const char *filename, int flags)
         s->cache_flags = SD_FLAG_CMD_DIRECT;
     }
 
-    if (s->cache_flags == SD_FLAG_CMD_CACHE) {
-        s->flush_fd = connect_to_sdog(s->addr, s->port);
-        if (s->flush_fd < 0) {
-            error_report("failed to connect");
-            ret = s->flush_fd;
-            goto out;
-        }
-    }
-
     if (snapid || tag[0] != '\0') {
         dprintf("%" PRIx32 " snapshot inode was open.\n", vid);
         s->is_snapshot = true;
@@ -1397,9 +1398,6 @@  static void sd_close(BlockDriverState *bs)
 
     qemu_aio_set_fd_handler(s->fd, NULL, NULL, NULL, NULL);
     closesocket(s->fd);
-    if (s->cache_flags) {
-        closesocket(s->flush_fd);
-    }
     g_free(s->addr);
 }
 
@@ -1711,39 +1709,31 @@  static coroutine_fn int sd_co_readv(BlockDriverState *bs, int64_t sector_num,
 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;
+    SheepdogAIOCB *acb;
+    AIOReq *aio_req;
     int ret;
-    unsigned int wlen = 0, rlen = 0;
 
     if (s->cache_flags != SD_FLAG_CMD_CACHE) {
         return 0;
     }
 
-    hdr.opcode = SD_OP_FLUSH_VDI;
-    hdr.oid = vid_to_vdi_oid(inode->vdi_id);
+    acb = sd_aio_setup(bs, NULL, 0, 0, NULL, NULL);
+    acb->aiocb_type = AIOCB_FLUSH_CACHE;
+    acb->aio_done_func = sd_finish_aiocb;
 
-    ret = do_req(s->flush_fd, (SheepdogReq *)&hdr, NULL, &wlen, &rlen);
-    if (ret) {
-        error_report("failed to send a request to the sheep");
+    aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
+                            0, 0, 0, 0, 0);
+    QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
+    ret = add_aio_request(s, aio_req, NULL, 0, false, acb->aiocb_type);
+    if (ret < 0) {
+        error_report("add_aio_request is failed");
+        free_aio_req(s, aio_req);
+        qemu_aio_release(acb);
         return ret;
     }
 
-    if (rsp->result == SD_RES_INVALID_PARMS) {
-        dprintf("disable write cache since the server doesn't support it\n");
-
-        s->cache_flags = SD_FLAG_CMD_DIRECT;
-        closesocket(s->flush_fd);
-        return 0;
-    }
-
-    if (rsp->result != SD_RES_SUCCESS) {
-        error_report("%s", sd_strerror(rsp->result));
-        return -EIO;
-    }
-
-    return 0;
+    qemu_coroutine_yield();
+    return acb->ret;
 }
 
 static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)