diff mbox

[v3,31/44] nbd: Share common reply-sending code in server

Message ID 1461368452-10389-32-git-send-email-eblake@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Blake April 22, 2016, 11:40 p.m. UTC 
Rather than open-coding NBD_REP_SERVER, reuse the code we
already have by adding a length parameter.  Additionally,
the refactoring will make adding NBD_OPT_GO in a later patch
easier.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v3: rebase to changes earlier in series
---
 nbd/server.c | 48 +++++++++++++++++++++++-------------------------
 1 file changed, 23 insertions(+), 25 deletions(-)

Comments

Alex Bligh April 25, 2016, 9:34 a.m. UTC | #1 
On 23 Apr 2016, at 00:40, Eric Blake <eblake@redhat.com> wrote:

> Rather than open-coding NBD_REP_SERVER, reuse the code we
> already have by adding a length parameter.  Additionally,
> the refactoring will make adding NBD_OPT_GO in a later patch
> easier.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alex Bligh <alex@alex.org.uk>
> 
> ---
> v3: rebase to changes earlier in series
> ---
> nbd/server.c | 48 +++++++++++++++++++++++-------------------------
> 1 file changed, 23 insertions(+), 25 deletions(-)
> 
> diff --git a/nbd/server.c b/nbd/server.c
> index 1d30b6d..4435d37 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -195,12 +195,15 @@ static ssize_t nbd_negotiate_drop_sync(QIOChannel *ioc, size_t size)
> 
> */
> 
> -static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
> +/* Send a reply header, including length, but no payload.
> + * Return -errno to kill connection, 0 to continue negotiation. */
> +static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,
> +                                      uint32_t opt, uint32_t len)
> {
>     uint64_t magic;
> -    uint32_t len;
> 
> -    TRACE("Reply opt=%" PRIx32 " type=%" PRIx32, type, opt);
> +    TRACE("Reply opt=%" PRIx32 " type=%" PRIx32 " len=%" PRIu32,
> +          type, opt, len);
> 
>     magic = cpu_to_be64(NBD_REP_MAGIC);
>     if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
> @@ -217,7 +220,7 @@ static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
>         LOG("write failed (rep type)");
>         return -EINVAL;
>     }
> -    len = cpu_to_be32(0);
> +    len = cpu_to_be32(len);
>     if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
>         LOG("write failed (rep data length)");
>         return -EINVAL;
> @@ -225,37 +228,32 @@ static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
>     return 0;
> }
> 
> +/* Send a reply header with default 0 length.
> + * Return -errno to kill connection, 0 to continue negotiation. */
> +static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
> +{
> +    return nbd_negotiate_send_rep_len(ioc, type, opt, 0);
> +}
> +
> +/* Send an NBD_REP_SERVER reply to NBD_OPT_LIST, including payload.
> + * Return -errno to kill connection, 0 to continue negotiation. */
> static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
> {
> -    uint64_t magic;
>     size_t name_len, desc_len;
> -    uint32_t opt, type, len;
> +    uint32_t len;
>     const char *name = exp->name ? exp->name : "";
>     const char *desc = exp->description ? exp->description : "";
> +    int rc;
> 
>     TRACE("Advertising export name '%s' description '%s'", name, desc);
>     name_len = strlen(name);
>     desc_len = strlen(desc);
> -    magic = cpu_to_be64(NBD_REP_MAGIC);
> -    if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
> -        LOG("write failed (magic)");
> -        return -EINVAL;
> -     }
> -    opt = cpu_to_be32(NBD_OPT_LIST);
> -    if (nbd_negotiate_write(ioc, &opt, sizeof(opt)) != sizeof(opt)) {
> -        LOG("write failed (opt)");
> -        return -EINVAL;
> -    }
> -    type = cpu_to_be32(NBD_REP_SERVER);
> -    if (nbd_negotiate_write(ioc, &type, sizeof(type)) != sizeof(type)) {
> -        LOG("write failed (reply type)");
> -        return -EINVAL;
> -    }
> -    len = cpu_to_be32(name_len + desc_len + sizeof(len));
> -    if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
> -        LOG("write failed (length)");
> -        return -EINVAL;
> +    len = name_len + desc_len + sizeof(len);
> +    rc = nbd_negotiate_send_rep_len(ioc, NBD_REP_SERVER, NBD_OPT_LIST, len);
> +    if (rc < 0) {
> +        return rc;
>     }
> +
>     len = cpu_to_be32(name_len);
>     if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
>         LOG("write failed (name length)");
> -- 
> 2.5.5
> 
>
diff mbox

Patch

diff --git a/nbd/server.c b/nbd/server.c
index 1d30b6d..4435d37 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -195,12 +195,15 @@  static ssize_t nbd_negotiate_drop_sync(QIOChannel *ioc, size_t size)

 */

-static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
+/* Send a reply header, including length, but no payload.
+ * Return -errno to kill connection, 0 to continue negotiation. */
+static int nbd_negotiate_send_rep_len(QIOChannel *ioc, uint32_t type,
+                                      uint32_t opt, uint32_t len)
 {
     uint64_t magic;
-    uint32_t len;

-    TRACE("Reply opt=%" PRIx32 " type=%" PRIx32, type, opt);
+    TRACE("Reply opt=%" PRIx32 " type=%" PRIx32 " len=%" PRIu32,
+          type, opt, len);

     magic = cpu_to_be64(NBD_REP_MAGIC);
     if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
@@ -217,7 +220,7 @@  static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
         LOG("write failed (rep type)");
         return -EINVAL;
     }
-    len = cpu_to_be32(0);
+    len = cpu_to_be32(len);
     if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
         LOG("write failed (rep data length)");
         return -EINVAL;
@@ -225,37 +228,32 @@  static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
     return 0;
 }

+/* Send a reply header with default 0 length.
+ * Return -errno to kill connection, 0 to continue negotiation. */
+static int nbd_negotiate_send_rep(QIOChannel *ioc, uint32_t type, uint32_t opt)
+{
+    return nbd_negotiate_send_rep_len(ioc, type, opt, 0);
+}
+
+/* Send an NBD_REP_SERVER reply to NBD_OPT_LIST, including payload.
+ * Return -errno to kill connection, 0 to continue negotiation. */
 static int nbd_negotiate_send_rep_list(QIOChannel *ioc, NBDExport *exp)
 {
-    uint64_t magic;
     size_t name_len, desc_len;
-    uint32_t opt, type, len;
+    uint32_t len;
     const char *name = exp->name ? exp->name : "";
     const char *desc = exp->description ? exp->description : "";
+    int rc;

     TRACE("Advertising export name '%s' description '%s'", name, desc);
     name_len = strlen(name);
     desc_len = strlen(desc);
-    magic = cpu_to_be64(NBD_REP_MAGIC);
-    if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) != sizeof(magic)) {
-        LOG("write failed (magic)");
-        return -EINVAL;
-     }
-    opt = cpu_to_be32(NBD_OPT_LIST);
-    if (nbd_negotiate_write(ioc, &opt, sizeof(opt)) != sizeof(opt)) {
-        LOG("write failed (opt)");
-        return -EINVAL;
-    }
-    type = cpu_to_be32(NBD_REP_SERVER);
-    if (nbd_negotiate_write(ioc, &type, sizeof(type)) != sizeof(type)) {
-        LOG("write failed (reply type)");
-        return -EINVAL;
-    }
-    len = cpu_to_be32(name_len + desc_len + sizeof(len));
-    if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
-        LOG("write failed (length)");
-        return -EINVAL;
+    len = name_len + desc_len + sizeof(len);
+    rc = nbd_negotiate_send_rep_len(ioc, NBD_REP_SERVER, NBD_OPT_LIST, len);
+    if (rc < 0) {
+        return rc;
     }
+
     len = cpu_to_be32(name_len);
     if (nbd_negotiate_write(ioc, &len, sizeof(len)) != sizeof(len)) {
         LOG("write failed (name length)");