All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/9] sheepdog: reconnect server after connection failure
@ 2013-07-24  7:56 MORITA Kazutaka
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 1/9] ignore SIGPIPE in qemu-img and qemu-io MORITA Kazutaka
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: MORITA Kazutaka @ 2013-07-24  7:56 UTC (permalink / raw)
  To: Kevin Wolf, Stefan Hajnoczi, Paolo Bonzini, qemu-devel; +Cc: sheepdog

Currently, if a sheepdog server exits, all the connecting VMs need to
be restarted.  This series implements a feature to reconnect the
server, and enables us to do online sheepdog upgrade and avoid
restarting VMs when sheepdog servers crash unexpectedly.

v2:
 - Dropped nonblocking connect patches

MORITA Kazutaka (9):
  ignore SIGPIPE in qemu-img and qemu-io
  iov: handle EOF in iov_send_recv
  sheepdog: check return values of qemu_co_recv/send correctly
  sheepdog: handle vdi objects in resend_aio_req
  sheepdog: reload inode outside of resend_aioreq
  coroutine: add co_aio_sleep_ns() to allow sleep in block drivers
  sheepdog: try to reconnect to sheepdog after network error
  sheepdog: make add_aio_request and send_aioreq void functions
  sheepdog: cancel aio requests if possible

 block/sheepdog.c          | 244 ++++++++++++++++++++++++++++++----------------
 include/block/coroutine.h |   8 ++
 qemu-coroutine-sleep.c    |  47 +++++++++
 qemu-img.c                |   4 +
 qemu-io.c                 |   4 +
 util/iov.c                |   6 ++
 6 files changed, 228 insertions(+), 85 deletions(-)

-- 
1.8.1.3.566.gaa39828

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v2 1/9] ignore SIGPIPE in qemu-img and qemu-io
  2013-07-24  7:56 [Qemu-devel] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
@ 2013-07-24  7:56 ` MORITA Kazutaka
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 2/9] iov: handle EOF in iov_send_recv MORITA Kazutaka
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: MORITA Kazutaka @ 2013-07-24  7:56 UTC (permalink / raw)
  To: Kevin Wolf, Stefan Hajnoczi, Paolo Bonzini, qemu-devel; +Cc: sheepdog

This prevents the tools from being stopped when they write data to a
closed connection in the other side.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 qemu-img.c | 4 ++++
 qemu-io.c  | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index c55ca5c..919d464 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2319,6 +2319,10 @@ int main(int argc, char **argv)
     const img_cmd_t *cmd;
     const char *cmdname;
 
+#ifdef CONFIG_POSIX
+    signal(SIGPIPE, SIG_IGN);
+#endif
+
     error_set_progname(argv[0]);
 
     qemu_init_main_loop();
diff --git a/qemu-io.c b/qemu-io.c
index cb9def5..d54dc86 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -335,6 +335,10 @@ int main(int argc, char **argv)
     int opt_index = 0;
     int flags = BDRV_O_UNMAP;
 
+#ifdef CONFIG_POSIX
+    signal(SIGPIPE, SIG_IGN);
+#endif
+
     progname = basename(argv[0]);
 
     while ((c = getopt_long(argc, argv, sopt, lopt, &opt_index)) != -1) {
-- 
1.8.1.3.566.gaa39828

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v2 2/9] iov: handle EOF in iov_send_recv
  2013-07-24  7:56 [Qemu-devel] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 1/9] ignore SIGPIPE in qemu-img and qemu-io MORITA Kazutaka
@ 2013-07-24  7:56 ` MORITA Kazutaka
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 3/9] sheepdog: check return values of qemu_co_recv/send correctly MORITA Kazutaka
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: MORITA Kazutaka @ 2013-07-24  7:56 UTC (permalink / raw)
  To: Kevin Wolf, Stefan Hajnoczi, Paolo Bonzini, qemu-devel; +Cc: sheepdog

Without this patch, iov_send_recv() never returns when do_send_recv()
returns zero.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 util/iov.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/util/iov.c b/util/iov.c
index cc6e837..f705586 100644
--- a/util/iov.c
+++ b/util/iov.c
@@ -202,6 +202,12 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt,
             return -1;
         }
 
+        if (ret == 0 && !do_send) {
+            /* recv returns 0 when the peer has performed an orderly
+             * shutdown. */
+            break;
+        }
+
         /* Prepare for the next iteration */
         offset += ret;
         total += ret;
-- 
1.8.1.3.566.gaa39828

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v2 3/9] sheepdog: check return values of qemu_co_recv/send correctly
  2013-07-24  7:56 [Qemu-devel] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 1/9] ignore SIGPIPE in qemu-img and qemu-io MORITA Kazutaka
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 2/9] iov: handle EOF in iov_send_recv MORITA Kazutaka
@ 2013-07-24  7:56 ` MORITA Kazutaka
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 4/9] sheepdog: handle vdi objects in resend_aio_req MORITA Kazutaka
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: MORITA Kazutaka @ 2013-07-24  7:56 UTC (permalink / raw)
  To: Kevin Wolf, Stefan Hajnoczi, Paolo Bonzini, qemu-devel; +Cc: sheepdog

qemu_co_recv/send return shorter length on error.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block/sheepdog.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 6a41ad9..bca5730 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -671,7 +671,7 @@ static void coroutine_fn aio_read_response(void *opaque)
 
     /* read a header */
     ret = qemu_co_recv(fd, &rsp, sizeof(rsp));
-    if (ret < 0) {
+    if (ret < sizeof(rsp)) {
         error_report("failed to get the header, %s", strerror(errno));
         goto out;
     }
@@ -722,7 +722,7 @@ static void coroutine_fn aio_read_response(void *opaque)
     case AIOCB_READ_UDATA:
         ret = qemu_co_recvv(fd, acb->qiov->iov, acb->qiov->niov,
                             aio_req->iov_offset, rsp.data_length);
-        if (ret < 0) {
+        if (ret < rsp.data_length) {
             error_report("failed to get the data, %s", strerror(errno));
             goto out;
         }
@@ -1075,7 +1075,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
 
     /* send a header */
     ret = qemu_co_send(s->fd, &hdr, sizeof(hdr));
-    if (ret < 0) {
+    if (ret < sizeof(hdr)) {
         qemu_co_mutex_unlock(&s->lock);
         error_report("failed to send a req, %s", strerror(errno));
         return -errno;
@@ -1083,7 +1083,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
 
     if (wlen) {
         ret = qemu_co_sendv(s->fd, iov, niov, aio_req->iov_offset, wlen);
-        if (ret < 0) {
+        if (ret < wlen) {
             qemu_co_mutex_unlock(&s->lock);
             error_report("failed to send a data, %s", strerror(errno));
             return -errno;
-- 
1.8.1.3.566.gaa39828

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v2 4/9] sheepdog: handle vdi objects in resend_aio_req
  2013-07-24  7:56 [Qemu-devel] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
                   ` (2 preceding siblings ...)
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 3/9] sheepdog: check return values of qemu_co_recv/send correctly MORITA Kazutaka
@ 2013-07-24  7:56 ` MORITA Kazutaka
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 5/9] sheepdog: reload inode outside of resend_aioreq MORITA Kazutaka
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: MORITA Kazutaka @ 2013-07-24  7:56 UTC (permalink / raw)
  To: Kevin Wolf, Stefan Hajnoczi, Paolo Bonzini, qemu-devel; +Cc: sheepdog

The current resend_aio_req() doesn't work when the request is against
vdi objects.  This fixes the problem.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block/sheepdog.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index bca5730..f25c7df 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1209,11 +1209,15 @@ static int coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
         return ret;
     }
 
-    aio_req->oid = vid_to_data_oid(s->inode.vdi_id,
-                                   data_oid_to_idx(aio_req->oid));
+    if (is_data_obj(aio_req->oid)) {
+        aio_req->oid = vid_to_data_oid(s->inode.vdi_id,
+                                       data_oid_to_idx(aio_req->oid));
+    } else {
+        aio_req->oid = vid_to_vdi_oid(s->inode.vdi_id);
+    }
 
     /* check whether this request becomes a CoW one */
-    if (acb->aiocb_type == AIOCB_WRITE_UDATA) {
+    if (acb->aiocb_type == AIOCB_WRITE_UDATA && is_data_obj(aio_req->oid)) {
         int idx = data_oid_to_idx(aio_req->oid);
         AIOReq *areq;
 
@@ -1241,8 +1245,15 @@ static int coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
         create = true;
     }
 out:
-    return add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
-                           create, acb->aiocb_type);
+    if (is_data_obj(aio_req->oid)) {
+        return add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
+                               create, acb->aiocb_type);
+    } else {
+        struct iovec iov;
+        iov.iov_base = &s->inode;
+        iov.iov_len = sizeof(s->inode);
+        return add_aio_request(s, aio_req, &iov, 1, false, AIOCB_WRITE_UDATA);
+    }
 }
 
 /* TODO Convert to fine grained options */
-- 
1.8.1.3.566.gaa39828

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v2 5/9] sheepdog: reload inode outside of resend_aioreq
  2013-07-24  7:56 [Qemu-devel] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
                   ` (3 preceding siblings ...)
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 4/9] sheepdog: handle vdi objects in resend_aio_req MORITA Kazutaka
@ 2013-07-24  7:56 ` MORITA Kazutaka
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 6/9] coroutine: add co_aio_sleep_ns() to allow sleep in block drivers MORITA Kazutaka
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: MORITA Kazutaka @ 2013-07-24  7:56 UTC (permalink / raw)
  To: Kevin Wolf, Stefan Hajnoczi, Paolo Bonzini, qemu-devel; +Cc: sheepdog

This prepares for using resend_aioreq() after reconnecting to the
sheepdog server.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block/sheepdog.c | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index f25c7df..cde887b 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -222,6 +222,11 @@ static inline uint64_t data_oid_to_idx(uint64_t oid)
     return oid & (MAX_DATA_OBJS - 1);
 }
 
+static inline uint32_t oid_to_vid(uint64_t oid)
+{
+    return (oid & ~VDI_BIT) >> VDI_SPACE_SHIFT;
+}
+
 static inline uint64_t vid_to_vdi_oid(uint32_t vid)
 {
     return VDI_BIT | ((uint64_t)vid << VDI_SPACE_SHIFT);
@@ -607,7 +612,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
                            struct iovec *iov, int niov, bool create,
                            enum AIOCBState aiocb_type);
 static int coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req);
-
+static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag);
 
 static AIOReq *find_pending_req(BDRVSheepdogState *s, uint64_t oid)
 {
@@ -755,6 +760,19 @@ static void coroutine_fn aio_read_response(void *opaque)
     case SD_RES_SUCCESS:
         break;
     case SD_RES_READONLY:
+        if (s->inode.vdi_id == oid_to_vid(aio_req->oid)) {
+            ret = reload_inode(s, 0, "");
+            if (ret < 0) {
+                goto out;
+            }
+        }
+
+        if (is_data_obj(aio_req->oid)) {
+            aio_req->oid = vid_to_data_oid(s->inode.vdi_id,
+                                           data_oid_to_idx(aio_req->oid));
+        } else {
+            aio_req->oid = vid_to_vdi_oid(s->inode.vdi_id);
+        }
         ret = resend_aioreq(s, aio_req);
         if (ret == SD_RES_SUCCESS) {
             goto out;
@@ -1202,19 +1220,6 @@ static int coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
 {
     SheepdogAIOCB *acb = aio_req->aiocb;
     bool create = false;
-    int ret;
-
-    ret = reload_inode(s, 0, "");
-    if (ret < 0) {
-        return ret;
-    }
-
-    if (is_data_obj(aio_req->oid)) {
-        aio_req->oid = vid_to_data_oid(s->inode.vdi_id,
-                                       data_oid_to_idx(aio_req->oid));
-    } else {
-        aio_req->oid = vid_to_vdi_oid(s->inode.vdi_id);
-    }
 
     /* check whether this request becomes a CoW one */
     if (acb->aiocb_type == AIOCB_WRITE_UDATA && is_data_obj(aio_req->oid)) {
-- 
1.8.1.3.566.gaa39828

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v2 6/9] coroutine: add co_aio_sleep_ns() to allow sleep in block drivers
  2013-07-24  7:56 [Qemu-devel] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
                   ` (4 preceding siblings ...)
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 5/9] sheepdog: reload inode outside of resend_aioreq MORITA Kazutaka
@ 2013-07-24  7:56 ` MORITA Kazutaka
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 7/9] sheepdog: try to reconnect to sheepdog after network error MORITA Kazutaka
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: MORITA Kazutaka @ 2013-07-24  7:56 UTC (permalink / raw)
  To: Kevin Wolf, Stefan Hajnoczi, Paolo Bonzini, qemu-devel; +Cc: sheepdog

This helper function behaves similarly to co_sleep_ns(), but the
sleeping coroutine will be resumed when using qemu_aio_wait().

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 include/block/coroutine.h |  8 ++++++++
 qemu-coroutine-sleep.c    | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/include/block/coroutine.h b/include/block/coroutine.h
index 377805a..23ea6e9 100644
--- a/include/block/coroutine.h
+++ b/include/block/coroutine.h
@@ -210,6 +210,14 @@ void qemu_co_rwlock_unlock(CoRwlock *lock);
 void coroutine_fn co_sleep_ns(QEMUClock *clock, int64_t ns);
 
 /**
+ * Yield the coroutine for a given duration
+ *
+ * Behaves similarly to co_sleep_ns(), but the sleeping coroutine will be
+ * resumed when using qemu_aio_wait().
+ */
+void coroutine_fn co_aio_sleep_ns(int64_t ns);
+
+/**
  * Yield until a file descriptor becomes readable
  *
  * Note that this function clobbers the handlers for the file descriptor.
diff --git a/qemu-coroutine-sleep.c b/qemu-coroutine-sleep.c
index 169ce5c..3955347 100644
--- a/qemu-coroutine-sleep.c
+++ b/qemu-coroutine-sleep.c
@@ -13,6 +13,7 @@
 
 #include "block/coroutine.h"
 #include "qemu/timer.h"
+#include "qemu/thread.h"
 
 typedef struct CoSleepCB {
     QEMUTimer *ts;
@@ -37,3 +38,49 @@ void coroutine_fn co_sleep_ns(QEMUClock *clock, int64_t ns)
     qemu_del_timer(sleep_cb.ts);
     qemu_free_timer(sleep_cb.ts);
 }
+
+typedef struct CoAioSleepCB {
+    QEMUBH *bh;
+    int64_t ns;
+    Coroutine *co;
+} CoAioSleepCB;
+
+static void co_aio_sleep_cb(void *opaque)
+{
+    CoAioSleepCB *aio_sleep_cb = opaque;
+
+    qemu_coroutine_enter(aio_sleep_cb->co, NULL);
+}
+
+static void *sleep_thread(void *opaque)
+{
+    CoAioSleepCB *aio_sleep_cb = opaque;
+    struct timespec req = {
+        .tv_sec = aio_sleep_cb->ns / 1000000000,
+        .tv_nsec = aio_sleep_cb->ns % 1000000000,
+    };
+    struct timespec rem;
+
+    while (nanosleep(&req, &rem) < 0 && errno == EINTR) {
+        req = rem;
+    }
+
+    qemu_bh_schedule(aio_sleep_cb->bh);
+
+    return NULL;
+}
+
+void coroutine_fn co_aio_sleep_ns(int64_t ns)
+{
+    CoAioSleepCB aio_sleep_cb = {
+        .ns = ns,
+        .co = qemu_coroutine_self(),
+    };
+    QemuThread thread;
+
+    aio_sleep_cb.bh = qemu_bh_new(co_aio_sleep_cb, &aio_sleep_cb);
+    qemu_thread_create(&thread, sleep_thread, &aio_sleep_cb,
+                       QEMU_THREAD_DETACHED);
+    qemu_coroutine_yield();
+    qemu_bh_delete(aio_sleep_cb.bh);
+}
-- 
1.8.1.3.566.gaa39828

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v2 7/9] sheepdog: try to reconnect to sheepdog after network error
  2013-07-24  7:56 [Qemu-devel] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
                   ` (5 preceding siblings ...)
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 6/9] coroutine: add co_aio_sleep_ns() to allow sleep in block drivers MORITA Kazutaka
@ 2013-07-24  7:56 ` MORITA Kazutaka
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 8/9] sheepdog: make add_aio_request and send_aioreq void functions MORITA Kazutaka
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: MORITA Kazutaka @ 2013-07-24  7:56 UTC (permalink / raw)
  To: Kevin Wolf, Stefan Hajnoczi, Paolo Bonzini, qemu-devel; +Cc: sheepdog

This introduces a failed request queue and links all the inflight
requests to the list after network error happens.  After QEMU
reconnects to the sheepdog server successfully, the sheepdog block
driver will retry all the requests in the failed queue.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block/sheepdog.c | 72 ++++++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 57 insertions(+), 15 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index cde887b..303354e 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -318,8 +318,11 @@ typedef struct BDRVSheepdogState {
     Coroutine *co_recv;
 
     uint32_t aioreq_seq_num;
+
+    /* Every aio request must be linked to either of these queues. */
     QLIST_HEAD(inflight_aio_head, AIOReq) inflight_aio_head;
     QLIST_HEAD(pending_aio_head, AIOReq) pending_aio_head;
+    QLIST_HEAD(failed_aio_head, AIOReq) failed_aio_head;
 } BDRVSheepdogState;
 
 static const char * sd_strerror(int err)
@@ -613,6 +616,8 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
                            enum AIOCBState aiocb_type);
 static int coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req);
 static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag);
+static int get_sheep_fd(BDRVSheepdogState *s);
+static void co_write_request(void *opaque);
 
 static AIOReq *find_pending_req(BDRVSheepdogState *s, uint64_t oid)
 {
@@ -654,6 +659,44 @@ static void coroutine_fn send_pending_req(BDRVSheepdogState *s, uint64_t oid)
     }
 }
 
+static coroutine_fn void reconnect_to_sdog(void *opaque)
+{
+    BDRVSheepdogState *s = opaque;
+    AIOReq *aio_req, *next;
+
+    qemu_aio_set_fd_handler(s->fd, NULL, NULL, NULL, NULL);
+    close(s->fd);
+    s->fd = -1;
+
+    /* Wait for outstanding write requests to be completed. */
+    while (s->co_send != NULL) {
+        co_write_request(opaque);
+    }
+
+    /* Move all the inflight requests to the failed queue. */
+    QLIST_FOREACH_SAFE(aio_req, &s->inflight_aio_head, aio_siblings, next) {
+        QLIST_REMOVE(aio_req, aio_siblings);
+        QLIST_INSERT_HEAD(&s->failed_aio_head, aio_req, aio_siblings);
+    }
+
+    /* Try to reconnect the sheepdog server every one second. */
+    while (s->fd < 0) {
+        s->fd = get_sheep_fd(s);
+        if (s->fd < 0) {
+            dprintf("Wait for connection to be established\n");
+            co_aio_sleep_ns(1000000000ULL);
+        }
+    };
+
+    /* Resend all the failed aio requests. */
+    while (!QLIST_EMPTY(&s->failed_aio_head)) {
+        aio_req = QLIST_FIRST(&s->failed_aio_head);
+        QLIST_REMOVE(aio_req, aio_siblings);
+        QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
+        resend_aioreq(s, aio_req);
+    }
+}
+
 /*
  * Receive responses of the I/O requests.
  *
@@ -670,15 +713,11 @@ static void coroutine_fn aio_read_response(void *opaque)
     SheepdogAIOCB *acb;
     uint64_t idx;
 
-    if (QLIST_EMPTY(&s->inflight_aio_head)) {
-        goto out;
-    }
-
     /* read a header */
     ret = qemu_co_recv(fd, &rsp, sizeof(rsp));
     if (ret < sizeof(rsp)) {
         error_report("failed to get the header, %s", strerror(errno));
-        goto out;
+        goto err;
     }
 
     /* find the right aio_req from the inflight aio list */
@@ -689,7 +728,7 @@ static void coroutine_fn aio_read_response(void *opaque)
     }
     if (!aio_req) {
         error_report("cannot find aio_req %x", rsp.id);
-        goto out;
+        goto err;
     }
 
     acb = aio_req->aiocb;
@@ -729,7 +768,7 @@ static void coroutine_fn aio_read_response(void *opaque)
                             aio_req->iov_offset, rsp.data_length);
         if (ret < rsp.data_length) {
             error_report("failed to get the data, %s", strerror(errno));
-            goto out;
+            goto err;
         }
         break;
     case AIOCB_FLUSH_CACHE:
@@ -763,10 +802,9 @@ static void coroutine_fn aio_read_response(void *opaque)
         if (s->inode.vdi_id == oid_to_vid(aio_req->oid)) {
             ret = reload_inode(s, 0, "");
             if (ret < 0) {
-                goto out;
+                goto err;
             }
         }
-
         if (is_data_obj(aio_req->oid)) {
             aio_req->oid = vid_to_data_oid(s->inode.vdi_id,
                                            data_oid_to_idx(aio_req->oid));
@@ -794,6 +832,10 @@ static void coroutine_fn aio_read_response(void *opaque)
     }
 out:
     s->co_recv = NULL;
+    return;
+err:
+    s->co_recv = NULL;
+    reconnect_to_sdog(opaque);
 }
 
 static void co_read_response(void *opaque)
@@ -819,7 +861,8 @@ static int aio_flush_request(void *opaque)
     BDRVSheepdogState *s = opaque;
 
     return !QLIST_EMPTY(&s->inflight_aio_head) ||
-        !QLIST_EMPTY(&s->pending_aio_head);
+        !QLIST_EMPTY(&s->pending_aio_head) ||
+        !QLIST_EMPTY(&s->failed_aio_head);
 }
 
 /*
@@ -1094,23 +1137,21 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
     /* send a header */
     ret = qemu_co_send(s->fd, &hdr, sizeof(hdr));
     if (ret < sizeof(hdr)) {
-        qemu_co_mutex_unlock(&s->lock);
         error_report("failed to send a req, %s", strerror(errno));
-        return -errno;
+        goto out;
     }
 
     if (wlen) {
         ret = qemu_co_sendv(s->fd, iov, niov, aio_req->iov_offset, wlen);
         if (ret < wlen) {
-            qemu_co_mutex_unlock(&s->lock);
             error_report("failed to send a data, %s", strerror(errno));
-            return -errno;
         }
     }
-
+out:
     socket_set_cork(s->fd, 0);
     qemu_aio_set_fd_handler(s->fd, co_read_response, NULL,
                             aio_flush_request, s);
+    s->co_send = NULL;
     qemu_co_mutex_unlock(&s->lock);
 
     return 0;
@@ -1300,6 +1341,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags)
 
     QLIST_INIT(&s->inflight_aio_head);
     QLIST_INIT(&s->pending_aio_head);
+    QLIST_INIT(&s->failed_aio_head);
     s->fd = -1;
 
     memset(vdi, 0, sizeof(vdi));
-- 
1.8.1.3.566.gaa39828

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v2 8/9] sheepdog: make add_aio_request and send_aioreq void functions
  2013-07-24  7:56 [Qemu-devel] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
                   ` (6 preceding siblings ...)
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 7/9] sheepdog: try to reconnect to sheepdog after network error MORITA Kazutaka
@ 2013-07-24  7:56 ` MORITA Kazutaka
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 9/9] sheepdog: cancel aio requests if possible MORITA Kazutaka
  2013-07-24  8:28 ` [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure Liu Yuan
  9 siblings, 0 replies; 19+ messages in thread
From: MORITA Kazutaka @ 2013-07-24  7:56 UTC (permalink / raw)
  To: Kevin Wolf, Stefan Hajnoczi, Paolo Bonzini, qemu-devel; +Cc: sheepdog

These functions no longer return errors.  We can make them void
functions and simplify the codes.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block/sheepdog.c | 66 +++++++++++++++-----------------------------------------
 1 file changed, 17 insertions(+), 49 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 303354e..42a30f1 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -611,10 +611,10 @@ static int do_req(int sockfd, SheepdogReq *hdr, void *data,
     return srco.ret;
 }
 
-static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
+static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
                            struct iovec *iov, int niov, bool create,
                            enum AIOCBState aiocb_type);
-static int coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req);
+static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req);
 static int reload_inode(BDRVSheepdogState *s, uint32_t snapid, const char *tag);
 static int get_sheep_fd(BDRVSheepdogState *s);
 static void co_write_request(void *opaque);
@@ -640,22 +640,14 @@ static void coroutine_fn send_pending_req(BDRVSheepdogState *s, uint64_t oid)
 {
     AIOReq *aio_req;
     SheepdogAIOCB *acb;
-    int ret;
 
     while ((aio_req = find_pending_req(s, oid)) != NULL) {
         acb = aio_req->aiocb;
         /* move aio_req from pending list to inflight one */
         QLIST_REMOVE(aio_req, aio_siblings);
         QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
-        ret = add_aio_request(s, aio_req, acb->qiov->iov,
-                              acb->qiov->niov, false, acb->aiocb_type);
-        if (ret < 0) {
-            error_report("add_aio_request is failed");
-            free_aio_req(s, aio_req);
-            if (!acb->nr_pending) {
-                sd_finish_aiocb(acb);
-            }
-        }
+        add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, false,
+                        acb->aiocb_type);
     }
 }
 
@@ -811,11 +803,8 @@ static void coroutine_fn aio_read_response(void *opaque)
         } else {
             aio_req->oid = vid_to_vdi_oid(s->inode.vdi_id);
         }
-        ret = resend_aioreq(s, aio_req);
-        if (ret == SD_RES_SUCCESS) {
-            goto out;
-        }
-        /* fall through */
+        resend_aioreq(s, aio_req);
+        goto out;
     default:
         acb->ret = -EIO;
         error_report("%s", sd_strerror(rsp.result));
@@ -1073,7 +1062,7 @@ out:
     return ret;
 }
 
-static int coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
+static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
                            struct iovec *iov, int niov, bool create,
                            enum AIOCBState aiocb_type)
 {
@@ -1153,8 +1142,6 @@ out:
                             aio_flush_request, s);
     s->co_send = NULL;
     qemu_co_mutex_unlock(&s->lock);
-
-    return 0;
 }
 
 static int read_write_object(int fd, char *buf, uint64_t oid, int copies,
@@ -1257,7 +1244,7 @@ out:
     return ret;
 }
 
-static int coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
+static void coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
 {
     SheepdogAIOCB *acb = aio_req->aiocb;
     bool create = false;
@@ -1282,7 +1269,7 @@ static int coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
                 dprintf("simultaneous CoW to %" PRIx64 "\n", aio_req->oid);
                 QLIST_REMOVE(aio_req, aio_siblings);
                 QLIST_INSERT_HEAD(&s->pending_aio_head, aio_req, aio_siblings);
-                return SD_RES_SUCCESS;
+                return;
             }
         }
 
@@ -1292,13 +1279,13 @@ static int coroutine_fn resend_aioreq(BDRVSheepdogState *s, AIOReq *aio_req)
     }
 out:
     if (is_data_obj(aio_req->oid)) {
-        return add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
-                               create, acb->aiocb_type);
+        add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, create,
+                        acb->aiocb_type);
     } else {
         struct iovec iov;
         iov.iov_base = &s->inode;
         iov.iov_len = sizeof(s->inode);
-        return add_aio_request(s, aio_req, &iov, 1, false, AIOCB_WRITE_UDATA);
+        add_aio_request(s, aio_req, &iov, 1, false, AIOCB_WRITE_UDATA);
     }
 }
 
@@ -1688,7 +1675,6 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset)
  */
 static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
 {
-    int ret;
     BDRVSheepdogState *s = acb->common.bs->opaque;
     struct iovec iov;
     AIOReq *aio_req;
@@ -1710,18 +1696,13 @@ static void coroutine_fn sd_write_done(SheepdogAIOCB *acb)
         aio_req = alloc_aio_req(s, acb, vid_to_vdi_oid(s->inode.vdi_id),
                                 data_len, offset, 0, 0, offset);
         QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
-        ret = add_aio_request(s, aio_req, &iov, 1, false, AIOCB_WRITE_UDATA);
-        if (ret) {
-            free_aio_req(s, aio_req);
-            acb->ret = -EIO;
-            goto out;
-        }
+        add_aio_request(s, aio_req, &iov, 1, false, AIOCB_WRITE_UDATA);
 
         acb->aio_done_func = sd_finish_aiocb;
         acb->aiocb_type = AIOCB_WRITE_UDATA;
         return;
     }
-out:
+
     sd_finish_aiocb(acb);
 }
 
@@ -1928,14 +1909,8 @@ static int coroutine_fn sd_co_rw_vector(void *p)
         }
 
         QLIST_INSERT_HEAD(&s->inflight_aio_head, aio_req, aio_siblings);
-        ret = add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov,
-                              create, acb->aiocb_type);
-        if (ret < 0) {
-            error_report("add_aio_request is failed");
-            free_aio_req(s, aio_req);
-            acb->ret = -EIO;
-            goto out;
-        }
+        add_aio_request(s, aio_req, acb->qiov->iov, acb->qiov->niov, create,
+                        acb->aiocb_type);
     done:
         offset = 0;
         idx++;
@@ -2003,7 +1978,6 @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
     BDRVSheepdogState *s = bs->opaque;
     SheepdogAIOCB *acb;
     AIOReq *aio_req;
-    int ret;
 
     if (s->cache_flags != SD_FLAG_CMD_CACHE) {
         return 0;
@@ -2016,13 +1990,7 @@ static int coroutine_fn sd_co_flush_to_disk(BlockDriverState *bs)
     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;
-    }
+    add_aio_request(s, aio_req, NULL, 0, false, acb->aiocb_type);
 
     qemu_coroutine_yield();
     return acb->ret;
-- 
1.8.1.3.566.gaa39828

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v2 9/9] sheepdog: cancel aio requests if possible
  2013-07-24  7:56 [Qemu-devel] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
                   ` (7 preceding siblings ...)
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 8/9] sheepdog: make add_aio_request and send_aioreq void functions MORITA Kazutaka
@ 2013-07-24  7:56 ` MORITA Kazutaka
  2013-07-24  8:28 ` [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure Liu Yuan
  9 siblings, 0 replies; 19+ messages in thread
From: MORITA Kazutaka @ 2013-07-24  7:56 UTC (permalink / raw)
  To: Kevin Wolf, Stefan Hajnoczi, Paolo Bonzini, qemu-devel; +Cc: sheepdog

This patch tries to cancel aio requests in pending queue and failed
queue.  When the sheepdog driver cannot cancel the requests, it waits
for them to be completed.

Signed-off-by: MORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
---
 block/sheepdog.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 59 insertions(+), 11 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 42a30f1..58e03c8 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -294,7 +294,8 @@ struct SheepdogAIOCB {
     Coroutine *coroutine;
     void (*aio_done_func)(SheepdogAIOCB *);
 
-    bool canceled;
+    bool cancelable;
+    bool *finished;
     int nr_pending;
 };
 
@@ -411,6 +412,7 @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
 {
     SheepdogAIOCB *acb = aio_req->aiocb;
 
+    acb->cancelable = false;
     QLIST_REMOVE(aio_req, aio_siblings);
     g_free(aio_req);
 
@@ -419,23 +421,68 @@ static inline void free_aio_req(BDRVSheepdogState *s, AIOReq *aio_req)
 
 static void coroutine_fn sd_finish_aiocb(SheepdogAIOCB *acb)
 {
-    if (!acb->canceled) {
-        qemu_coroutine_enter(acb->coroutine, NULL);
+    qemu_coroutine_enter(acb->coroutine, NULL);
+    if (acb->finished) {
+        *acb->finished = true;
     }
     qemu_aio_release(acb);
 }
 
+/*
+ * Check whether the specified acb can be canceled
+ *
+ * We can cancel aio when any request belonging to the acb is:
+ *  - Not processed by the sheepdog server.
+ *  - Not linked to the inflight queue.
+ */
+static bool sd_acb_cancelable(const SheepdogAIOCB *acb)
+{
+    BDRVSheepdogState *s = acb->common.bs->opaque;
+    AIOReq *aioreq;
+
+    if (!acb->cancelable) {
+        return false;
+    }
+
+    QLIST_FOREACH(aioreq, &s->inflight_aio_head, aio_siblings) {
+        if (aioreq->aiocb == acb) {
+            return false;
+        }
+    }
+
+    return false;
+}
+
 static void sd_aio_cancel(BlockDriverAIOCB *blockacb)
 {
     SheepdogAIOCB *acb = (SheepdogAIOCB *)blockacb;
+    BDRVSheepdogState *s = acb->common.bs->opaque;
+    AIOReq *aioreq, *next;
+    bool finished = false;
+
+    acb->finished = &finished;
+    while (!finished) {
+        if (sd_acb_cancelable(acb)) {
+            /* Remove outstanding requests from pending and failed queues.  */
+            QLIST_FOREACH_SAFE(aioreq, &s->pending_aio_head, aio_siblings,
+                               next) {
+                if (aioreq->aiocb == acb) {
+                    free_aio_req(s, aioreq);
+                }
+            }
+            QLIST_FOREACH_SAFE(aioreq, &s->failed_aio_head, aio_siblings,
+                               next) {
+                if (aioreq->aiocb == acb) {
+                    free_aio_req(s, aioreq);
+                }
+            }
 
-    /*
-     * Sheepdog cannot cancel the requests which are already sent to
-     * the servers, so we just complete the request with -EIO here.
-     */
-    acb->ret = -EIO;
-    qemu_coroutine_enter(acb->coroutine, NULL);
-    acb->canceled = true;
+            assert(acb->nr_pending == 0);
+            sd_finish_aiocb(acb);
+            return;
+        }
+        qemu_aio_wait();
+    }
 }
 
 static const AIOCBInfo sd_aiocb_info = {
@@ -456,7 +503,8 @@ static SheepdogAIOCB *sd_aio_setup(BlockDriverState *bs, QEMUIOVector *qiov,
     acb->nb_sectors = nb_sectors;
 
     acb->aio_done_func = NULL;
-    acb->canceled = false;
+    acb->cancelable = true;
+    acb->finished = NULL;
     acb->coroutine = qemu_coroutine_self();
     acb->ret = 0;
     acb->nr_pending = 0;
-- 
1.8.1.3.566.gaa39828

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure
  2013-07-24  7:56 [Qemu-devel] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
                   ` (8 preceding siblings ...)
  2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 9/9] sheepdog: cancel aio requests if possible MORITA Kazutaka
@ 2013-07-24  8:28 ` Liu Yuan
  2013-07-24  9:07   ` MORITA Kazutaka
  9 siblings, 1 reply; 19+ messages in thread
From: Liu Yuan @ 2013-07-24  8:28 UTC (permalink / raw)
  To: MORITA Kazutaka
  Cc: Kevin Wolf, Paolo Bonzini, sheepdog, qemu-devel, Stefan Hajnoczi

On Wed, Jul 24, 2013 at 04:56:24PM +0900, MORITA Kazutaka wrote:
> Currently, if a sheepdog server exits, all the connecting VMs need to
> be restarted.  This series implements a feature to reconnect the
> server, and enables us to do online sheepdog upgrade and avoid
> restarting VMs when sheepdog servers crash unexpectedly.
> 

It doesn't work on my test. I tried start linux-0.2.img stored in sheepdog
cluster and then

1. did some buffered writes
2. restart sheep that this QEMU VM connected to.
3. $ sync

I got following error:

$ ../qemu/x86_64-softmmu/qemu-system-x86_64 --enable-kvm -m 1024 -hda sheepdog:test
qemu-system-x86_64: failed to get the header, Resource temporarily unavailable
qemu-system-x86_64: Failed to connect to socket: Connection refused
qemu-system-x86_64: Failed to connect to socket: Connection refused
qemu-system-x86_64: Failed to connect to socket: Connection refused
qemu-system-x86_64: Failed to connect to socket: Connection refused
qemu-system-x86_64: Failed to connect to socket: Connection refused
...repeat...

QEMU version is master tip

Thanks
Yuan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure
  2013-07-24  8:28 ` [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure Liu Yuan
@ 2013-07-24  9:07   ` MORITA Kazutaka
  2013-07-24 15:42     ` Liu Yuan
  0 siblings, 1 reply; 19+ messages in thread
From: MORITA Kazutaka @ 2013-07-24  9:07 UTC (permalink / raw)
  To: Liu Yuan
  Cc: Kevin Wolf, sheepdog, qemu-devel, Stefan Hajnoczi, Paolo Bonzini,
	MORITA Kazutaka

At Wed, 24 Jul 2013 16:28:30 +0800,
Liu Yuan wrote:
> 
> On Wed, Jul 24, 2013 at 04:56:24PM +0900, MORITA Kazutaka wrote:
> > Currently, if a sheepdog server exits, all the connecting VMs need to
> > be restarted.  This series implements a feature to reconnect the
> > server, and enables us to do online sheepdog upgrade and avoid
> > restarting VMs when sheepdog servers crash unexpectedly.
> > 
> 
> It doesn't work on my test. I tried start linux-0.2.img stored in sheepdog
> cluster and then
> 
> 1. did some buffered writes
> 2. restart sheep that this QEMU VM connected to.
> 3. $ sync
> 
> I got following error:
> 
> $ ../qemu/x86_64-softmmu/qemu-system-x86_64 --enable-kvm -m 1024 -hda sheepdog:test
> qemu-system-x86_64: failed to get the header, Resource temporarily unavailable
> qemu-system-x86_64: Failed to connect to socket: Connection refused
> qemu-system-x86_64: Failed to connect to socket: Connection refused
> qemu-system-x86_64: Failed to connect to socket: Connection refused
> qemu-system-x86_64: Failed to connect to socket: Connection refused
> qemu-system-x86_64: Failed to connect to socket: Connection refused
> ...repeat...
> 
> QEMU version is master tip

Your sheep daemon looks like unreachable from qemu.  I tried the same
procedure, but couldn't reproduce it.

Is the problem reproducible?  Can you make sure that you can connect
to the sheep daemon from collie while the error message shows up?

Thanks,

Kazutaka

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure
  2013-07-24  9:07   ` MORITA Kazutaka
@ 2013-07-24 15:42     ` Liu Yuan
  2013-07-25  3:36       ` Liu Yuan
  0 siblings, 1 reply; 19+ messages in thread
From: Liu Yuan @ 2013-07-24 15:42 UTC (permalink / raw)
  To: MORITA Kazutaka
  Cc: Kevin Wolf, Paolo Bonzini, sheepdog, qemu-devel, Stefan Hajnoczi

On Wed, Jul 24, 2013 at 06:07:21PM +0900, MORITA Kazutaka wrote:
> At Wed, 24 Jul 2013 16:28:30 +0800,
> Liu Yuan wrote:
> > 
> > On Wed, Jul 24, 2013 at 04:56:24PM +0900, MORITA Kazutaka wrote:
> > > Currently, if a sheepdog server exits, all the connecting VMs need to
> > > be restarted.  This series implements a feature to reconnect the
> > > server, and enables us to do online sheepdog upgrade and avoid
> > > restarting VMs when sheepdog servers crash unexpectedly.
> > > 
> > 
> > It doesn't work on my test. I tried start linux-0.2.img stored in sheepdog
> > cluster and then
> > 
> > 1. did some buffered writes
> > 2. restart sheep that this QEMU VM connected to.
> > 3. $ sync
> > 
> > I got following error:
> > 
> > $ ../qemu/x86_64-softmmu/qemu-system-x86_64 --enable-kvm -m 1024 -hda sheepdog:test
> > qemu-system-x86_64: failed to get the header, Resource temporarily unavailable
> > qemu-system-x86_64: Failed to connect to socket: Connection refused
> > qemu-system-x86_64: Failed to connect to socket: Connection refused
> > qemu-system-x86_64: Failed to connect to socket: Connection refused
> > qemu-system-x86_64: Failed to connect to socket: Connection refused
> > qemu-system-x86_64: Failed to connect to socket: Connection refused
> > ...repeat...
> > 
> > QEMU version is master tip
> 
> Your sheep daemon looks like unreachable from qemu.  I tried the same
> procedure, but couldn't reproduce it.
> 
> Is the problem reproducible?  Can you make sure that you can connect
> to the sheep daemon from collie while the error message shows up?
> 

Yesh. Well I try to repeat it with following process:

1. did some buffered write
2. kill the sheep
3. $ sync # at guest, now 'sync' hang for response
4. restart sheep

After 4 'sync' still hangs until timeout with a message
"hda:dma_timer_expiry: dma status == 0x21"

Guest end up freeze.

QEMU output is the same:
qemu-system-x86_64: failed to get the header, Resource temporarily unavailable
qemu-system-x86_64: Failed to connect to socket: Connection refused
qemu-system-x86_64: Failed to connect to socket: Connection refused
qemu-system-x86_64: Failed to connect to socket: Connection refused
qemu-system-x86_64: Failed to connect to socket: Connection refused

But notice, if I did restart sheep with guest doing nothing, your patch set work
like a charm.

Thanks
Yuan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure
  2013-07-24 15:42     ` Liu Yuan
@ 2013-07-25  3:36       ` Liu Yuan
  2013-07-25  5:25         ` [Qemu-devel] " Liu Yuan
  0 siblings, 1 reply; 19+ messages in thread
From: Liu Yuan @ 2013-07-25  3:36 UTC (permalink / raw)
  To: MORITA Kazutaka
  Cc: Kevin Wolf, Paolo Bonzini, sheepdog, qemu-devel, Stefan Hajnoczi

On Wed, Jul 24, 2013 at 11:42:49PM +0800, Liu Yuan wrote:
> On Wed, Jul 24, 2013 at 06:07:21PM +0900, MORITA Kazutaka wrote:
> > At Wed, 24 Jul 2013 16:28:30 +0800,
> > Liu Yuan wrote:
> > > 
> > > On Wed, Jul 24, 2013 at 04:56:24PM +0900, MORITA Kazutaka wrote:
> > > > Currently, if a sheepdog server exits, all the connecting VMs need to
> > > > be restarted.  This series implements a feature to reconnect the
> > > > server, and enables us to do online sheepdog upgrade and avoid
> > > > restarting VMs when sheepdog servers crash unexpectedly.
> > > > 
> > > 
> > > It doesn't work on my test. I tried start linux-0.2.img stored in sheepdog
> > > cluster and then
> > > 
> > > 1. did some buffered writes
> > > 2. restart sheep that this QEMU VM connected to.
> > > 3. $ sync
> > > 
> > > I got following error:
> > > 
> > > $ ../qemu/x86_64-softmmu/qemu-system-x86_64 --enable-kvm -m 1024 -hda sheepdog:test
> > > qemu-system-x86_64: failed to get the header, Resource temporarily unavailable
> > > qemu-system-x86_64: Failed to connect to socket: Connection refused
> > > qemu-system-x86_64: Failed to connect to socket: Connection refused
> > > qemu-system-x86_64: Failed to connect to socket: Connection refused
> > > qemu-system-x86_64: Failed to connect to socket: Connection refused
> > > qemu-system-x86_64: Failed to connect to socket: Connection refused
> > > ...repeat...
> > > 
> > > QEMU version is master tip
> > 
> > Your sheep daemon looks like unreachable from qemu.  I tried the same
> > procedure, but couldn't reproduce it.
> > 
> > Is the problem reproducible?  Can you make sure that you can connect
> > to the sheep daemon from collie while the error message shows up?
> > 
> 
> Yesh. Well I try to repeat it with following process:
> 
> 1. did some buffered write
> 2. kill the sheep
> 3. $ sync # at guest, now 'sync' hang for response
> 4. restart sheep
> 
> After 4 'sync' still hangs until timeout with a message
> "hda:dma_timer_expiry: dma status == 0x21"
> 
> Guest end up freeze.
> 
> QEMU output is the same:
> qemu-system-x86_64: failed to get the header, Resource temporarily unavailable
> qemu-system-x86_64: Failed to connect to socket: Connection refused
> qemu-system-x86_64: Failed to connect to socket: Connection refused
> qemu-system-x86_64: Failed to connect to socket: Connection refused
> qemu-system-x86_64: Failed to connect to socket: Connection refused
> 
> But notice, if I did restart sheep with guest doing nothing, your patch set work
> like a charm.

I have debug it a bit. The problem is that at stage 3, 'sync' invoke
add_aio_request() in the sheepdog driver and add_aio_request *succeed* with aio
put on the inflight_aio_head list, *not* on the failed_aio_head list. So in the
reconnect_to_sdog(), we have no way to resend the targeted aio and 'sync' wait
for ever.

Thanks
Yuan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/9] sheepdog: reconnect server after connection failure
  2013-07-25  3:36       ` Liu Yuan
@ 2013-07-25  5:25         ` Liu Yuan
  2013-07-25  5:25           ` [Qemu-devel] [PATCH 1/2] sheepdog: correct signedness of comparison Liu Yuan
                             ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Liu Yuan @ 2013-07-25  5:25 UTC (permalink / raw)
  To: sheepdog, morita.kazutaka; +Cc: kwolf, pbonzini, qemu-devel, stefanha

Hello Kazutaka,

   I have two patches fixing the problems I found on my testing and they are
complementary patches. Please consider sending them on top of your patch set.

Thanks
Yuan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH 1/2] sheepdog: correct signedness of comparison
  2013-07-25  5:25         ` [Qemu-devel] " Liu Yuan
@ 2013-07-25  5:25           ` Liu Yuan
  2013-07-25  5:25           ` [Qemu-devel] [PATCH 2/2] sheepdog: put aio request into failed list when failing to send request Liu Yuan
  2013-07-25  5:53           ` [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
  2 siblings, 0 replies; 19+ messages in thread
From: Liu Yuan @ 2013-07-25  5:25 UTC (permalink / raw)
  To: sheepdog, morita.kazutaka; +Cc: kwolf, pbonzini, qemu-devel, stefanha

When signed int compared to unsigned int, signed int will be converted to
unsigned int.

For example, (-1 < sizeof(structure)) always true because -1 in the left is
converted into unsigned int, thus this restule in unexpected true.

Signed-off-by: Liu Yuan <namei.unix@gmail.com>
---
 block/sheepdog.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 58e03c8..8c6c8f1 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -616,7 +616,7 @@ static coroutine_fn void do_co_req(void *opaque)
 
     if (*rlen) {
         ret = qemu_co_recv(sockfd, data, *rlen);
-        if (ret < *rlen) {
+        if (ret < (int)*rlen) {
             error_report("failed to get the data, %s", strerror(errno));
             ret = -errno;
             goto out;
@@ -755,7 +755,7 @@ static void coroutine_fn aio_read_response(void *opaque)
 
     /* read a header */
     ret = qemu_co_recv(fd, &rsp, sizeof(rsp));
-    if (ret < sizeof(rsp)) {
+    if (ret < (int)sizeof(rsp)) {
         error_report("failed to get the header, %s", strerror(errno));
         goto err;
     }
@@ -806,7 +806,7 @@ static void coroutine_fn aio_read_response(void *opaque)
     case AIOCB_READ_UDATA:
         ret = qemu_co_recvv(fd, acb->qiov->iov, acb->qiov->niov,
                             aio_req->iov_offset, rsp.data_length);
-        if (ret < rsp.data_length) {
+        if (ret < (int)rsp.data_length) {
             error_report("failed to get the data, %s", strerror(errno));
             goto err;
         }
@@ -1116,7 +1116,7 @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
 {
     int nr_copies = s->inode.nr_copies;
     SheepdogObjReq hdr;
-    unsigned int wlen = 0;
+    int wlen = 0;
     int ret;
     uint64_t oid = aio_req->oid;
     unsigned int datalen = aio_req->data_len;
@@ -1173,7 +1173,7 @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
 
     /* send a header */
     ret = qemu_co_send(s->fd, &hdr, sizeof(hdr));
-    if (ret < sizeof(hdr)) {
+    if (ret < (int)sizeof(hdr)) {
         error_report("failed to send a req, %s", strerror(errno));
         goto out;
     }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH 2/2] sheepdog: put aio request into failed list when failing to send request
  2013-07-25  5:25         ` [Qemu-devel] " Liu Yuan
  2013-07-25  5:25           ` [Qemu-devel] [PATCH 1/2] sheepdog: correct signedness of comparison Liu Yuan
@ 2013-07-25  5:25           ` Liu Yuan
  2013-07-25  5:53           ` [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
  2 siblings, 0 replies; 19+ messages in thread
From: Liu Yuan @ 2013-07-25  5:25 UTC (permalink / raw)
  To: sheepdog, morita.kazutaka; +Cc: kwolf, pbonzini, qemu-devel, stefanha

qemu_co_send() in the add_aio_request might fail if connection is closed. In
this case we should it requests into failed list to be resended later when
connection is repaired.

Signed-off-by: Liu Yuan <namei.unix@gmail.com>
---
 block/sheepdog.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 8c6c8f1..5bf78d0 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1174,14 +1174,18 @@ static void coroutine_fn add_aio_request(BDRVSheepdogState *s, AIOReq *aio_req,
     /* send a header */
     ret = qemu_co_send(s->fd, &hdr, sizeof(hdr));
     if (ret < (int)sizeof(hdr)) {
-        error_report("failed to send a req, %s", strerror(errno));
+        dprintf("failed to send a req, %s", strerror(errno));
+        QLIST_REMOVE(aio_req, aio_siblings);
+        QLIST_INSERT_HEAD(&s->pending_aio_head, aio_req, aio_siblings);
         goto out;
     }
 
     if (wlen) {
         ret = qemu_co_sendv(s->fd, iov, niov, aio_req->iov_offset, wlen);
         if (ret < wlen) {
-            error_report("failed to send a data, %s", strerror(errno));
+            dprintf("failed to send a data, %s", strerror(errno));
+            QLIST_REMOVE(aio_req, aio_siblings);
+            QLIST_INSERT_HEAD(&s->pending_aio_head, aio_req, aio_siblings);
         }
     }
 out:
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure
  2013-07-25  5:25         ` [Qemu-devel] " Liu Yuan
  2013-07-25  5:25           ` [Qemu-devel] [PATCH 1/2] sheepdog: correct signedness of comparison Liu Yuan
  2013-07-25  5:25           ` [Qemu-devel] [PATCH 2/2] sheepdog: put aio request into failed list when failing to send request Liu Yuan
@ 2013-07-25  5:53           ` MORITA Kazutaka
  2013-07-25  6:02             ` Liu Yuan
  2 siblings, 1 reply; 19+ messages in thread
From: MORITA Kazutaka @ 2013-07-25  5:53 UTC (permalink / raw)
  To: Liu Yuan; +Cc: kwolf, sheepdog, qemu-devel, stefanha, pbonzini, morita.kazutaka

At Thu, 25 Jul 2013 13:25:33 +0800,
Liu Yuan wrote:
> 
> Hello Kazutaka,
> 
>    I have two patches fixing the problems I found on my testing and they are
> complementary patches. Please consider sending them on top of your patch set.

Thanks a lot for your comments and patches, but I've already prepared
patches, which would be probably better fixes.  I'll send the v3
series soon.  It'd be appreciated if you would give a review for it.

Thanks,

Kazutaka

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure
  2013-07-25  5:53           ` [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
@ 2013-07-25  6:02             ` Liu Yuan
  0 siblings, 0 replies; 19+ messages in thread
From: Liu Yuan @ 2013-07-25  6:02 UTC (permalink / raw)
  To: MORITA Kazutaka; +Cc: kwolf, pbonzini, sheepdog, qemu-devel, stefanha

On Thu, Jul 25, 2013 at 02:53:57PM +0900, MORITA Kazutaka wrote:
> At Thu, 25 Jul 2013 13:25:33 +0800,
> Liu Yuan wrote:
> > 
> > Hello Kazutaka,
> > 
> >    I have two patches fixing the problems I found on my testing and they are
> > complementary patches. Please consider sending them on top of your patch set.
> 
> Thanks a lot for your comments and patches, but I've already prepared
> patches, which would be probably better fixes.  I'll send the v3
> series soon.  It'd be appreciated if you would give a review for it.
> 

Okay, no problem. Well, in my previous patches, patch 2/2 isn't correct, I did a
wrong manual rebase by hasty copy. Just FYI.

Thanks
Yuan

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2013-07-25  6:02 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-24  7:56 [Qemu-devel] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 1/9] ignore SIGPIPE in qemu-img and qemu-io MORITA Kazutaka
2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 2/9] iov: handle EOF in iov_send_recv MORITA Kazutaka
2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 3/9] sheepdog: check return values of qemu_co_recv/send correctly MORITA Kazutaka
2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 4/9] sheepdog: handle vdi objects in resend_aio_req MORITA Kazutaka
2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 5/9] sheepdog: reload inode outside of resend_aioreq MORITA Kazutaka
2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 6/9] coroutine: add co_aio_sleep_ns() to allow sleep in block drivers MORITA Kazutaka
2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 7/9] sheepdog: try to reconnect to sheepdog after network error MORITA Kazutaka
2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 8/9] sheepdog: make add_aio_request and send_aioreq void functions MORITA Kazutaka
2013-07-24  7:56 ` [Qemu-devel] [PATCH v2 9/9] sheepdog: cancel aio requests if possible MORITA Kazutaka
2013-07-24  8:28 ` [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure Liu Yuan
2013-07-24  9:07   ` MORITA Kazutaka
2013-07-24 15:42     ` Liu Yuan
2013-07-25  3:36       ` Liu Yuan
2013-07-25  5:25         ` [Qemu-devel] " Liu Yuan
2013-07-25  5:25           ` [Qemu-devel] [PATCH 1/2] sheepdog: correct signedness of comparison Liu Yuan
2013-07-25  5:25           ` [Qemu-devel] [PATCH 2/2] sheepdog: put aio request into failed list when failing to send request Liu Yuan
2013-07-25  5:53           ` [Qemu-devel] [sheepdog] [PATCH v2 0/9] sheepdog: reconnect server after connection failure MORITA Kazutaka
2013-07-25  6:02             ` Liu Yuan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.