[Qemu-devel] [PULL 0/4] NBD patches for 2017-09-25

[Qemu-devel] [PULL 0/4] NBD patches for 2017-09-25

[Eric Blake]

The following changes since commit 460b6c8e581aa06b86f59eebd9e52edfe7adf417:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2017-09-23 12:55:40 +0100)

are available in the git repository at:

  git://repo.or.cz/qemu/ericb.git tags/pull-nbd-2017-09-25

for you to fetch changes up to a693437037328a95d815ad5aec37ac2f8e130e58:

  block/nbd-client: nbd_co_send_request: fix return code (2017-09-25 08:21:26 -0500)

----------------------------------------------------------------
nbd patches for 2017-09-25

- Eric Blake: nbd-client: Use correct macro parenthesization
- Vladimir Sementsov-Ogievskiy: 0/3 nbd client refactoring and fixing

----------------------------------------------------------------
Eric Blake (1):
      nbd-client: Use correct macro parenthesization

Vladimir Sementsov-Ogievskiy (3):
      block/nbd-client: refactor nbd_co_receive_reply
      block/nbd-client: simplify check in nbd_co_receive_reply
      block/nbd-client: nbd_co_send_request: fix return code

 block/nbd-client.c | 37 ++++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

-- 
2.13.5

[Qemu-devel] [PULL 1/4] nbd-client: Use correct macro parenthesization

[Eric Blake]

If 'bs' is a complex expression, we were only casting the front half
rather than the full expression.  Luckily, none of the callers were
passing bad arguments, but it's better to be robust up front.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170918214649.17550-1-eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nbd-client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index ee7f758e68..cc05e73c2d 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -31,8 +31,8 @@
 #include "qapi/error.h"
 #include "nbd-client.h"

-#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ ((uint64_t)(intptr_t)bs))
-#define INDEX_TO_HANDLE(bs, index)  ((index)  ^ ((uint64_t)(intptr_t)bs))
+#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ (uint64_t)(intptr_t)(bs))
+#define INDEX_TO_HANDLE(bs, index)  ((index)  ^ (uint64_t)(intptr_t)(bs))

 static void nbd_recv_coroutines_wake_all(NBDClientSession *s)
 {
-- 
2.13.5

[Qemu-devel] [PULL 2/4] block/nbd-client: refactor nbd_co_receive_reply

[Eric Blake]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

"NBDReply *reply" parameter of nbd_co_receive_reply is used only
to pass return value for nbd_co_request (reply.error). Remove it
and use function return value instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170920124507.18841-2-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/nbd-client.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index cc05e73c2d..5168a2cab6 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -178,26 +178,26 @@ err:
     return rc;
 }

-static void nbd_co_receive_reply(NBDClientSession *s,
-                                 NBDRequest *request,
-                                 NBDReply *reply,
-                                 QEMUIOVector *qiov)
+static int nbd_co_receive_reply(NBDClientSession *s,
+                                NBDRequest *request,
+                                QEMUIOVector *qiov)
 {
+    int ret;
     int i = HANDLE_TO_INDEX(s, request->handle);

     /* Wait until we're woken up by nbd_read_reply_entry.  */
     s->requests[i].receiving = true;
     qemu_coroutine_yield();
     s->requests[i].receiving = false;
-    *reply = s->reply;
-    if (reply->handle != request->handle || !s->ioc || s->quit) {
-        reply->error = EIO;
+    if (s->reply.handle != request->handle || !s->ioc || s->quit) {
+        ret = -EIO;
     } else {
-        if (qiov && reply->error == 0) {
+        ret = -s->reply.error;
+        if (qiov && s->reply.error == 0) {
             assert(request->len == iov_size(qiov->iov, qiov->niov));
             if (qio_channel_readv_all(s->ioc, qiov->iov, qiov->niov,
                                       NULL) < 0) {
-                reply->error = EIO;
+                ret = -EIO;
                 s->quit = true;
             }
         }
@@ -217,6 +217,8 @@ static void nbd_co_receive_reply(NBDClientSession *s,
     s->in_flight--;
     qemu_co_queue_next(&s->free_sema);
     qemu_co_mutex_unlock(&s->send_mutex);
+
+    return ret;
 }

 static int nbd_co_request(BlockDriverState *bs,
@@ -224,7 +226,6 @@ static int nbd_co_request(BlockDriverState *bs,
                           QEMUIOVector *qiov)
 {
     NBDClientSession *client = nbd_get_client_session(bs);
-    NBDReply reply;
     int ret;

     assert(!qiov || request->type == NBD_CMD_WRITE ||
@@ -232,12 +233,11 @@ static int nbd_co_request(BlockDriverState *bs,
     ret = nbd_co_send_request(bs, request,
                               request->type == NBD_CMD_WRITE ? qiov : NULL);
     if (ret < 0) {
-        reply.error = -ret;
-    } else {
-        nbd_co_receive_reply(client, request, &reply,
-                             request->type == NBD_CMD_READ ? qiov : NULL);
+        return ret;
     }
-    return -reply.error;
+
+    return nbd_co_receive_reply(client, request,
+                                request->type == NBD_CMD_READ ? qiov : NULL);
 }

 int nbd_client_co_preadv(BlockDriverState *bs, uint64_t offset,
-- 
2.13.5

[Qemu-devel] [PULL 3/4] block/nbd-client: simplify check in nbd_co_receive_reply

[Eric Blake]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

If we are woken up from while() loop in nbd_read_reply_entry
handles must be equal. If we are woken up from
nbd_recv_coroutines_wake_all s->quit must be true, so we do
not need checking handles equality.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170920124507.18841-3-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/nbd-client.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 5168a2cab6..ac93c4c0d0 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -189,9 +189,10 @@ static int nbd_co_receive_reply(NBDClientSession *s,
     s->requests[i].receiving = true;
     qemu_coroutine_yield();
     s->requests[i].receiving = false;
-    if (s->reply.handle != request->handle || !s->ioc || s->quit) {
+    if (!s->ioc || s->quit) {
         ret = -EIO;
     } else {
+        assert(s->reply.handle == request->handle);
         ret = -s->reply.error;
         if (qiov && s->reply.error == 0) {
             assert(request->len == iov_size(qiov->iov, qiov->niov));
-- 
2.13.5

[Qemu-devel] [PULL 4/4] block/nbd-client: nbd_co_send_request: fix return code

[Eric Blake]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

It's incorrect to return success rc >= 0 if we skip qio_channel_writev_all()
call due to s->quit.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170920124507.18841-4-vsementsov@virtuozzo.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 block/nbd-client.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index ac93c4c0d0..72651dcdb1 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -161,6 +161,8 @@ static int nbd_co_send_request(BlockDriverState *bs,
                                        NULL) < 0) {
                 rc = -EIO;
             }
+        } else if (rc >= 0) {
+            rc = -EIO;
         }
         qio_channel_set_cork(s->ioc, false);
     } else {
-- 
2.13.5

Re: [Qemu-devel] [PULL 0/4] NBD patches for 2017-09-25

[Peter Maydell]

On 25 September 2017 at 14:41, Eric Blake <eblake@redhat.com> wrote:
> The following changes since commit 460b6c8e581aa06b86f59eebd9e52edfe7adf417:
>
>   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2017-09-23 12:55:40 +0100)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/ericb.git tags/pull-nbd-2017-09-25
>
> for you to fetch changes up to a693437037328a95d815ad5aec37ac2f8e130e58:
>
>   block/nbd-client: nbd_co_send_request: fix return code (2017-09-25 08:21:26 -0500)
>
> ----------------------------------------------------------------
> nbd patches for 2017-09-25
>
> - Eric Blake: nbd-client: Use correct macro parenthesization
> - Vladimir Sementsov-Ogievskiy: 0/3 nbd client refactoring and fixing
>
> ----------------------------------------------------------------
> Eric Blake (1):
>       nbd-client: Use correct macro parenthesization
>
> Vladimir Sementsov-Ogievskiy (3):
>       block/nbd-client: refactor nbd_co_receive_reply
>       block/nbd-client: simplify check in nbd_co_receive_reply
>       block/nbd-client: nbd_co_send_request: fix return code
>
>  block/nbd-client.c | 37 ++++++++++++++++++++-----------------
>  1 file changed, 20 insertions(+), 17 deletions(-)

Applied, thanks.

-- PMM