[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v2 5/6] nbd/server: Add helper functions for par
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-block] [PATCH v2 5/6] nbd/server: Add helper functions for parsing option payload |
Date: |
Fri, 12 Jan 2018 13:20:07 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
11.01.2018 02:08, Eric Blake wrote:
Rather than making every callsite perform length sanity checks
and error reporting, add the helper functions nbd_opt_read()
and nbd_opt_drop() that use the length stored in the client
struct; also add an assertion that optlen is reduced to zero
after each option is handled.
Note that the call in nbd_negotiate_handle_export_name() does
not use the new helper (in part because the server cannot
reply to NBD_OPT_EXPORT_NAME - it either succeeds or the
connection drops).
Based on patches by Vladimir Sementsov-Ogievskiy.
Signed-off-by: Eric Blake <address@hidden>
---
nbd/server.c | 123 ++++++++++++++++++++++++++++++-----------------------------
1 file changed, 63 insertions(+), 60 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index d23bc2918a..ec8c3be019 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -229,6 +229,41 @@ nbd_negotiate_send_rep_err(NBDClient *client, uint32_t
type,
return ret;
}
+/* Drop remainder of the current option, after sending a reply with
looks a bit weird: actually you drop the remainder _before_ sending a reply)
+ * the given error type and message. Return -errno on read or write
also, unrelated note, -errno is always forced to -EIO, because of
nbd_read realization.
and this note applies to many other places here. It is correct (EIO is
errno, why not?),
but it may be not bad to note it somewhere..
+ * failure; or 0 if connection is still live. */
+static int GCC_FMT_ATTR(4, 5)
+nbd_opt_drop(NBDClient *client, uint32_t type, Error **errp,
+ const char *fmt, ...)
+{
+ int ret = nbd_drop(client->ioc, client->optlen, errp);
+
+ client->optlen = 0;
+ if (!ret) {
+ va_list va;
+
+ va_start(va, fmt);
+ ret = nbd_negotiate_send_rep_verr(client, type, errp, fmt, va);
+ va_end(va);
+ }
+ return ret;
+}
[..]
@@ -812,14 +819,9 @@ static int nbd_negotiate_options(NBDClient *client,
uint16_t myflags,
break;
default:
- if (nbd_drop(client->ioc, length, errp) < 0) {
- return -EIO;
- }
- ret = nbd_negotiate_send_rep_err(client,
- NBD_REP_ERR_UNSUP, errp,
- "Unsupported option 0x%"
- PRIx32 " (%s)", option,
- nbd_opt_lookup(option));
+ ret = nbd_opt_drop(client, NBD_REP_ERR_UNSUP, errp,
+ "Unsupported option 0x%" PRIx32 " (%s)",
+ option, nbd_opt_lookup(option));
break;
}
} else {
@@ -842,6 +844,7 @@ static int nbd_negotiate_options(NBDClient *client,
uint16_t myflags,
if (ret < 0) {
return ret;
}
+ assert(!client->optlen);
isn't it from 2/6?
}
}
anyway,
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
--
Best regards,
Vladimir
[Qemu-block] [PATCH v2 5/6] nbd/server: Add helper functions for parsing option payload, Eric Blake, 2018/01/10
- Re: [Qemu-block] [PATCH v2 5/6] nbd/server: Add helper functions for parsing option payload,
Vladimir Sementsov-Ogievskiy <=
[Qemu-block] [PATCH v2 3/6] nbd/server: Better error for NBD_OPT_EXPORT_NAME failure, Eric Blake, 2018/01/10
[Qemu-block] [PATCH v2 6/6] nbd/server: structurize option reply sending, Eric Blake, 2018/01/10
[Qemu-block] [PATCH v2 4/6] nbd/server: Add va_list form of nbd_negotiate_send_rep_err(), Eric Blake, 2018/01/10