From 4f4e329628b6d530cf74d792f0297f651e18e6ba Mon Sep 17 00:00:00 2001 From: John Snow Date: Wed, 27 Mar 2019 17:22:24 +0100 Subject: [PATCH 085/163] nbd/client: Drop pointless buf variable RH-Author: John Snow Message-id: <20190327172308.31077-12-jsnow@redhat.com> Patchwork-id: 85182 O-Subject: [RHEL-7.7 qemu-kvm-rhev PATCH 11/55] nbd/client: Drop pointless buf variable Bugzilla: 1691009 RH-Acked-by: Stefan Hajnoczi RH-Acked-by: Max Reitz RH-Acked-by: Miroslav Rezanina From: Eric Blake There's no need to read into a temporary buffer (oversized since commit 7d3123e1) followed by a byteswap into a uint64_t to check for a magic number via memcmp(), when the code immediately below demonstrates reading into the uint64_t then byteswapping in place and checking for a magic number via integer math. What's more, having a different error message when the server's first reply byte is 0 is unusual - it's no different from any other wrong magic number, and we already detected short reads. That whole strlen() issue has been present and useless since commit 1d45f8b5 in 2010; perhaps it was leftover debugging (since the correct magic number happens to be ASCII)? Make the error messages more consistent and detailed while touching things. Signed-off-by: Eric Blake Reviewed-by: Richard W.M. Jones Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20181215135324.152629-9-eblake@redhat.com> (cherry picked from commit ef2e35fcc8e14bcc9366df5fdf53f65d679f8dca) Signed-off-by: John Snow Signed-off-by: Miroslav Rezanina --- nbd/client.c | 22 +++++++--------------- nbd/nbd-internal.h | 3 ++- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 5a03a84..f625c20 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -733,7 +733,6 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, QIOChannel **outioc, NBDExportInfo *info, Error **errp) { - char buf[256]; uint64_t magic; int rc; bool zeroes = true; @@ -754,27 +753,20 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, goto fail; } - if (nbd_read(ioc, buf, 8, errp) < 0) { - error_prepend(errp, "Failed to read data: "); - goto fail; - } - - buf[8] = '\0'; - if (strlen(buf) == 0) { - error_setg(errp, "Server connection closed unexpectedly"); + if (nbd_read(ioc, &magic, sizeof(magic), errp) < 0) { + error_prepend(errp, "Failed to read initial magic: "); goto fail; } - - magic = ldq_be_p(buf); + magic = be64_to_cpu(magic); trace_nbd_receive_negotiate_magic(magic); - if (memcmp(buf, "NBDMAGIC", 8) != 0) { - error_setg(errp, "Invalid magic received"); + if (magic != NBD_INIT_MAGIC) { + error_setg(errp, "Bad initial magic received: 0x%" PRIx64, magic); goto fail; } if (nbd_read(ioc, &magic, sizeof(magic), errp) < 0) { - error_prepend(errp, "Failed to read magic: "); + error_prepend(errp, "Failed to read server magic: "); goto fail; } magic = be64_to_cpu(magic); @@ -913,7 +905,7 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name, } info->flags = oldflags; } else { - error_setg(errp, "Bad magic received"); + error_setg(errp, "Bad server magic received: 0x%" PRIx64, magic); goto fail; } diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h index f38be9e..82aa221 100644 --- a/nbd/nbd-internal.h +++ b/nbd/nbd-internal.h @@ -46,8 +46,9 @@ /* Size of oldstyle negotiation */ #define NBD_OLDSTYLE_NEGOTIATE_SIZE (8 + 8 + 8 + 4 + 124) +#define NBD_INIT_MAGIC 0x4e42444d41474943LL /* ASCII "NBDMAGIC" */ #define NBD_REQUEST_MAGIC 0x25609513 -#define NBD_OPTS_MAGIC 0x49484156454F5054LL +#define NBD_OPTS_MAGIC 0x49484156454F5054LL /* ASCII "IHAVEOPT" */ #define NBD_CLIENT_MAGIC 0x0000420281861253LL #define NBD_REP_MAGIC 0x0003e889045565a9LL -- 1.8.3.1