Re: [Qemu-devel] [PATCH 05/25] nbd: Avoid generic -EINVAL

Max Reitz Mon, 16 Mar 2015 06:55:18 -0700

On 2015-03-11 at 07:22, Paolo Bonzini wrote:


On 25/02/2015 19:08, Max Reitz wrote:

Just returning -EINVAL for everything is bad. -EIO is often better, and
sometimes there is an even more fitting value.

Propagating the return value from write_sync is uglier, but it is even
better in terms of returned value.

We can only return -errno values, but write_sync() may do partial writesso it may return non-negative values which still indicate an error. Sowe'd have to check whether the return value is negative, if it is,return that, if it isn't but if it's still below what we wanted towrite, return a fixed error (such as -EIO). I'd rather just return -EIOand be done with it, but if you really want me to, I can of course do itdifferently.

Max

Paolo

Signed-off-by: Max Reitz <mre...@redhat.com>
---
  nbd.c | 49 +++++++++++++++++++++++++------------------------
  1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/nbd.c b/nbd.c
index b4776ce..d2eb1c5 100644
--- a/nbd.c
+++ b/nbd.c
@@ -238,22 +238,22 @@ static int nbd_send_rep(int csock, uint32_t type, 
uint32_t opt)
      magic = cpu_to_be64(NBD_REP_MAGIC);
      if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
          LOG("write failed (rep magic)");
-        return -EINVAL;
+        return -EIO;
      }
      opt = cpu_to_be32(opt);
      if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
          LOG("write failed (rep opt)");
-        return -EINVAL;
+        return -EIO;
      }
      type = cpu_to_be32(type);
      if (write_sync(csock, &type, sizeof(type)) != sizeof(type)) {
          LOG("write failed (rep type)");
-        return -EINVAL;
+        return -EIO;
      }
      len = cpu_to_be32(0);
      if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
          LOG("write failed (rep data length)");
-        return -EINVAL;
+        return -EIO;
      }
      return 0;
  }
@@ -267,31 +267,31 @@ static int nbd_send_rep_list(int csock, NBDExport *exp)
      magic = cpu_to_be64(NBD_REP_MAGIC);
      if (write_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
          LOG("write failed (magic)");
-        return -EINVAL;
+        return -EIO;
       }
      opt = cpu_to_be32(NBD_OPT_LIST);
      if (write_sync(csock, &opt, sizeof(opt)) != sizeof(opt)) {
          LOG("write failed (opt)");
-        return -EINVAL;
+        return -EIO;
      }
      type = cpu_to_be32(NBD_REP_SERVER);
      if (write_sync(csock, &type, sizeof(type)) != sizeof(type)) {
          LOG("write failed (reply type)");
-        return -EINVAL;
+        return -EIO;
      }
      len = cpu_to_be32(name_len + sizeof(len));
      if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
          LOG("write failed (length)");
-        return -EINVAL;
+        return -EIO;
      }
      len = cpu_to_be32(name_len);
      if (write_sync(csock, &len, sizeof(len)) != sizeof(len)) {
          LOG("write failed (length)");
-        return -EINVAL;
+        return -EIO;
      }
      if (write_sync(csock, exp->name, name_len) != name_len) {
          LOG("write failed (buffer)");
-        return -EINVAL;
+        return -EIO;
      }
      return 0;
  }
@@ -309,7 +309,7 @@ static int nbd_handle_list(NBDClient *client, uint32_t 
length)
      /* For each export, send a NBD_REP_SERVER reply. */
      QTAILQ_FOREACH(exp, &exports, next) {
          if (nbd_send_rep_list(csock, exp)) {
-            return -EINVAL;
+            return -EIO;
          }
      }
      /* Finish with a NBD_REP_ACK. */
@@ -331,6 +331,7 @@ static int nbd_handle_export_name(NBDClient *client, 
uint32_t length)
      }
      if (read_sync(csock, name, length) != length) {
          LOG("read failed");
+        rc = -EIO;
          goto fail;
      }
      name[length] = '\0';
@@ -376,22 +377,22 @@ static int nbd_receive_options(NBDClient *client)

if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {

              LOG("read failed");
-            return -EINVAL;
+            return -EIO;
          }
          TRACE("Checking opts magic");
          if (magic != be64_to_cpu(NBD_OPTS_MAGIC)) {
              LOG("Bad magic received");
-            return -EINVAL;
+            return -EIO;
          }

if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {

              LOG("read failed");
-            return -EINVAL;
+            return -EIO;
          }

if (read_sync(csock, &length, sizeof(length)) != sizeof(length)) {

              LOG("read failed");
-            return -EINVAL;
+            return -EIO;
          }
          length = be32_to_cpu(length);

@@ -404,7 +405,7 @@ static int nbd_receive_options(NBDClient *client)

              break;

case NBD_OPT_ABORT:

-            return -EINVAL;
+            return -ECONNABORTED;

case NBD_OPT_EXPORT_NAME:

              return nbd_handle_export_name(client, length);
@@ -446,7 +447,7 @@ static int nbd_send_negotiate(NBDClient *client)
       */

qemu_set_block(csock);

-    rc = -EINVAL;
+    rc = -EIO;

TRACE("Beginning negotiation.");

      memset(buf, 0, sizeof(buf));
@@ -503,7 +504,7 @@ int nbd_receive_negotiate(int csock, const char *name, 
uint32_t *flags,

TRACE("Receiving negotiation.");- rc = -EINVAL;

+    rc = -EIO;

if (read_sync(csock, buf, 8) != 8) {

          error_setg(errp, "Failed to read data");
@@ -752,7 +753,7 @@ ssize_t nbd_send_request(int csock, struct nbd_request 
*request)

if (ret != sizeof(buf)) {

          LOG("writing to socket failed");
-        return -EINVAL;
+        return -EIO;
      }
      return 0;
  }
@@ -770,7 +771,7 @@ static ssize_t nbd_receive_request(int csock, struct 
nbd_request *request)

if (ret != sizeof(buf)) {

          LOG("read failed");
-        return -EINVAL;
+        return -EIO;
      }

/* Request

@@ -793,7 +794,7 @@ static ssize_t nbd_receive_request(int csock, struct 
nbd_request *request)

if (magic != NBD_REQUEST_MAGIC) {

          LOG("invalid magic (got 0x%x)", magic);
-        return -EINVAL;
+        return -EIO;
      }
      return 0;
  }
@@ -811,7 +812,7 @@ ssize_t nbd_receive_reply(int csock, struct nbd_reply 
*reply)

if (ret != sizeof(buf)) {

          LOG("read failed");
-        return -EINVAL;
+        return -EIO;
      }

/* Reply

@@ -830,7 +831,7 @@ ssize_t nbd_receive_reply(int csock, struct nbd_reply 
*reply)

if (magic != NBD_REPLY_MAGIC) {

          LOG("invalid magic (got 0x%x)", magic);
-        return -EINVAL;
+        return -EIO;
      }
      return 0;
  }
@@ -858,7 +859,7 @@ static ssize_t nbd_send_reply(int csock, struct nbd_reply 
*reply)

if (ret != sizeof(buf)) {

          LOG("writing to socket failed");
-        return -EINVAL;
+        return -EIO;
      }
      return 0;
  }

Re: [Qemu-devel] [PATCH 05/25] nbd: Avoid generic -EINVAL

Reply via email to