Re: [Qemu-devel] [PATCH v3 07/10] sheepdog: try to reconnect to sheepdog after network error

[Liu Yuan <namei.unix@gmail.com>]

On Thu, Jul 25, 2013 at 05:32:02PM +0900, MORITA Kazutaka wrote:
> 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 7b22816..43a6feb 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);
> +    }
> +
> +    /* 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);
> +        }
> +    };
> +
> +    /* 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);
> +    }
> +
> +    /* 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);
> +    }
> +}
> +

Is failed queue necessary? Here you just move requests from inflight queue to
failed queue, then interate the failed queue to send them all.

Isn't it simpler we just resend the requests in the inflight queue like

> +    QLIST_FOREACH(aio_req, &s->inflight_aio_head, aio_siblings, next) {
> +        resend_aioreq(s, aio_req);
> +    }

Thanks
Yuan