qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/3] nbd: make nbd_read* return -EIO on error


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH 3/3] nbd: make nbd_read* return -EIO on error
Date: Fri, 29 Jan 2021 08:48:59 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1

28.01.2021 23:14, Roman Kagan wrote:
NBD reconnect logic considers the error code from the functions that
read NBD messages to tell if reconnect should be attempted or not: it is
attempted on -EIO, otherwise the client transitions to NBD_CLIENT_QUIT
state (see nbd_channel_error).  This error code is propagated from the
primitives like nbd_read.

The problem, however, is that nbd_read itself turns every error into -1
rather than -EIO.  As a result, if the NBD server happens to die while
sending the message, the client in QEMU receives less data than it
expects, considers it as a fatal error, and wouldn't attempt
reestablishing the connection.

Fix it by turning every negative return from qio_channel_read_all into
-EIO returned from nbd_read.  Apparently that was the original behavior,
but got broken later.  Also adjust nbd_readXX to follow.

Fixes: e6798f06a6 ("nbd: generalize usage of nbd_read")
Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>

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

Really looks like a bug in e6798f06a6: it changes error code from -EIO to -1 
without any reasoning.

---
  include/block/nbd.h | 7 ++++---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 4a52a43ef5..5f34d23bb0 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -364,7 +364,7 @@ static inline int nbd_read(QIOChannel *ioc, void *buffer, 
size_t size,
          if (desc) {
              error_prepend(errp, "Failed to read %s: ", desc);
          }
-        return -1;
+        return ret;
      }
return 0;
@@ -375,8 +375,9 @@ static inline int nbd_read##bits(QIOChannel *ioc,           
            \
                                   uint##bits##_t *val,                   \
                                   const char *desc, Error **errp)        \
  {                                                                       \
-    if (nbd_read(ioc, val, sizeof(*val), desc, errp) < 0) {             \
-        return -1;                                                      \
+    int ret = nbd_read(ioc, val, sizeof(*val), desc, errp);             \
+    if (ret < 0) {                                                      \
+        return ret;                                                     \
      }                                                                   \
      *val = be##bits##_to_cpu(*val);                                     \
      return 0;                                                           \



--
Best regards,
Vladimir



reply via email to

[Prev in Thread] Current Thread [Next in Thread]