[Qemu-devel] [PATCH 0/3 for-2.6] more nbd fixups

[Qemu-devel] [PATCH 0/3 for-2.6] more nbd fixups

[Eric Blake]

Patch 1 fixes a minor bug (wrong error) on read-only exports,
patch 2 and 3 fix up the debugging traces I used to find the
root cause of patch 1.  Maintainer's discretion on whether
to take all three or just patch 1 in qemu 2.6 (but debug
output improvements can't cause regressions...)

Eric Blake (3):
  nbd: Return correct error for write to read-only export
  nbd: Avoid bitrot in TRACE() usage
  nbd: Improve debug traces on little-endian

 nbd/nbd-internal.h | 15 +++++++++------
 nbd/client.c       |  8 ++++----
 nbd/server.c       |  6 ++++--
 3 files changed, 17 insertions(+), 12 deletions(-)

-- 
2.5.5

[Qemu-devel] [PATCH 1/3] nbd: Return correct error for write to read-only export

[Eric Blake]

The NBD Protocol requires that servers should send EPERM for
attempts to write (or trim) a read-only export.  We were
correct for TRIM (blk_co_discard() gave EPERM); but were
manually setting EROFS which then got mapped to EINVAL over
the wire on writes.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 nbd/server.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/nbd/server.c b/nbd/server.c
index b95571b..98e3957 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -26,6 +26,7 @@ static int system_errno_to_nbd_errno(int err)
     case 0:
         return NBD_SUCCESS;
     case EPERM:
+    case EROFS:
         return NBD_EPERM;
     case EIO:
         return NBD_EIO;
-- 
2.5.5

[Qemu-devel] [PATCH 2/3] nbd: Avoid bitrot in TRACE() usage

[Eric Blake]

The compiler is smart enough to optimize out 'if (0)', but won't
type-check our printfs if they are hidden behind #if.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 nbd/nbd-internal.h | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index 127c48d..f4dce06 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -33,18 +33,21 @@
 #define DEBUG_NBD

 #ifdef DEBUG_NBD
-#define TRACE(msg, ...) do { \
-    LOG(msg, ## __VA_ARGS__); \
-} while(0)
+#define DEBUG_NBD_PRINT 1
 #else
-#define TRACE(msg, ...) \
-    do { } while (0)
+#define DEBUG_NBD_PRINT 0
 #endif

+#define TRACE(msg, ...) do { \
+    if (DEBUG_NBD_PRINT) { \
+        LOG(msg, ## __VA_ARGS__); \
+    } \
+} while (0)
+
 #define LOG(msg, ...) do { \
     fprintf(stderr, "%s:%s():L%d: " msg "\n", \
             __FILE__, __FUNCTION__, __LINE__, ## __VA_ARGS__); \
-} while(0)
+} while (0)

 /* This is all part of the "official" NBD API.
  *
-- 
2.5.5

[Qemu-devel] [PATCH 3/3] nbd: Improve debug traces on little-endian

[Eric Blake]

Print debug tracing messages while data is still in native
ordering, rather than after we've potentially swapped it into
network order for transmission.  Also, it's nice if the server
mentions what it is replying, to correlate it to with what the
client says it is receiving.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 nbd/client.c | 8 ++++----
 nbd/server.c | 5 +++--
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/nbd/client.c b/nbd/client.c
index 6f0541d..602768e 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -629,16 +629,16 @@ ssize_t nbd_send_request(QIOChannel *ioc, struct nbd_request *request)
     uint8_t buf[NBD_REQUEST_SIZE];
     ssize_t ret;

+    TRACE("Sending request to server: "
+          "{ .from = %" PRIu64", .len = %u, .handle = %" PRIu64", .type=%i}",
+          request->from, request->len, request->handle, request->type);
+
     cpu_to_be32w((uint32_t*)buf, NBD_REQUEST_MAGIC);
     cpu_to_be32w((uint32_t*)(buf + 4), request->type);
     cpu_to_be64w((uint64_t*)(buf + 8), request->handle);
     cpu_to_be64w((uint64_t*)(buf + 16), request->from);
     cpu_to_be32w((uint32_t*)(buf + 24), request->len);

-    TRACE("Sending request to server: "
-          "{ .from = %" PRIu64", .len = %u, .handle = %" PRIu64", .type=%i}",
-          request->from, request->len, request->handle, request->type);
-
     ret = write_sync(ioc, buf, sizeof(buf));
     if (ret < 0) {
         return ret;
diff --git a/nbd/server.c b/nbd/server.c
index 98e3957..6d9c15a 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -656,6 +656,9 @@ static ssize_t nbd_send_reply(QIOChannel *ioc, struct nbd_reply *reply)

     reply->error = system_errno_to_nbd_errno(reply->error);

+    TRACE("Sending response to client: { .error = %d, handle = %" PRIu64 " }",
+          reply->error, reply->handle);
+
     /* Reply
        [ 0 ..  3]    magic   (NBD_REPLY_MAGIC)
        [ 4 ..  7]    error   (0 == no error)
@@ -665,8 +668,6 @@ static ssize_t nbd_send_reply(QIOChannel *ioc, struct nbd_reply *reply)
     stl_be_p(buf + 4, reply->error);
     stq_be_p(buf + 8, reply->handle);

-    TRACE("Sending response to client");
-
     ret = write_sync(ioc, buf, sizeof(buf));
     if (ret < 0) {
         return ret;
-- 
2.5.5