From nobody Thu May 2 22:23:37 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1500329971867713.4383372633436; Mon, 17 Jul 2017 15:19:31 -0700 (PDT) Received: from localhost ([::1]:52809 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dXEMT-0002V3-MH for importer@patchew.org; Mon, 17 Jul 2017 18:19:29 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59865) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dXEIc-0007XR-P3 for qemu-devel@nongnu.org; Mon, 17 Jul 2017 18:15:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dXEIb-00022p-L1 for qemu-devel@nongnu.org; Mon, 17 Jul 2017 18:15:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52052) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dXEIW-0001wY-3Z; Mon, 17 Jul 2017 18:15:24 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1EDDD7CE0B; Mon, 17 Jul 2017 22:15:23 +0000 (UTC) Received: from red.redhat.com (ovpn-120-160.rdu2.redhat.com [10.10.120.160]) by smtp.corp.redhat.com (Postfix) with ESMTP id 47EBD60BF1; Mon, 17 Jul 2017 22:15:22 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 1EDDD7CE0B Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 1EDDD7CE0B From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 17 Jul 2017 17:15:14 -0500 Message-Id: <20170717221516.7692-2-eblake@redhat.com> In-Reply-To: <20170717221516.7692-1-eblake@redhat.com> References: <20170717221516.7692-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Mon, 17 Jul 2017 22:15:23 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 1/3] nbd: Fix iotests failure due to changed client error message X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Paolo Bonzini , "open list:Network Block Dev..." , Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Commit 8ecaeae8 changed the way the client requests an NBD export, and in the process also changed the resulting error message when the export is not present, breaking a couple of iotests. The error message is now directly given by the server (a failed NBD_OPT_GO) instead of implied by the client (after exhausting NBD_OPT_LIST), but looking at the testsuite changes, it proves worthwhile to reword the error message to be slightly less verbose (as this is one particular error message likely to be hit by a user). Note that the error message is now sensitive to which binary is running the server as well as the client (since the expected output is replaying a message received from the server - for that matter, it depends on a server new enough to understand NBD_OPT_GO); in general iotests are run on client and server from the same source code base so the default setup will pass; but if it proves problematic for people overriding QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, and QEMU_NBD_PROG to point across multiple builds for cross-version integration testing, we may have to later tweak or sanitize the output somehow. Reported-by: Kevin Wolf Signed-off-by: Eric Blake Message-Id: <20170717142310.17048-1-eblake@redhat.com> Tested-by: John Snow Reviewed-by: John Snow --- nbd/client.c | 5 ++--- tests/qemu-iotests/140.out | 3 ++- tests/qemu-iotests/143.out | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index c3ee9f36b1..846bdc394c 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -232,8 +232,7 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_op= t_reply *reply, break; case NBD_REP_ERR_UNKNOWN: - error_setg(errp, "Requested export not available for option %" PRI= x32 - " (%s)", reply->option, nbd_opt_lookup(reply->option)); + error_setg(errp, "Requested export not available"); break; case NBD_REP_ERR_SHUTDOWN: @@ -253,7 +252,7 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_op= t_reply *reply, } if (msg) { - error_append_hint(errp, "%s\n", msg); + error_append_hint(errp, "server reported: %s\n", msg); } cleanup: diff --git a/tests/qemu-iotests/140.out b/tests/qemu-iotests/140.out index 0689b2b41c..7295b3d975 100644 --- a/tests/qemu-iotests/140.out +++ b/tests/qemu-iotests/140.out @@ -8,7 +8,8 @@ wrote 65536/65536 bytes at offset 0 read 65536/65536 bytes at offset 0 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) {"return": {}} -can't open device nbd+unix:///drv?socket=3DTEST_DIR/nbd: No export with na= me 'drv' available +can't open device nbd+unix:///drv?socket=3DTEST_DIR/nbd: Requested export = not available +server reported: export 'drv' not present {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event"= : "SHUTDOWN", "data": {"guest": false}} *** done diff --git a/tests/qemu-iotests/143.out b/tests/qemu-iotests/143.out index 0978b8985a..1c7fb45543 100644 --- a/tests/qemu-iotests/143.out +++ b/tests/qemu-iotests/143.out @@ -1,7 +1,8 @@ QA output created by 143 {"return": {}} {"return": {}} -can't open device nbd+unix:///no_such_export?socket=3DTEST_DIR/nbd: No exp= ort with name 'no_such_export' available +can't open device nbd+unix:///no_such_export?socket=3DTEST_DIR/nbd: Reques= ted export not available +server reported: export 'no_such_export' not present {"return": {}} {"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event"= : "SHUTDOWN", "data": {"guest": false}} *** done --=20 2.13.3 From nobody Thu May 2 22:23:37 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1500329891279355.6211817171362; Mon, 17 Jul 2017 15:18:11 -0700 (PDT) Received: from localhost ([::1]:52806 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dXELB-000198-3y for importer@patchew.org; Mon, 17 Jul 2017 18:18:09 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59856) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dXEIc-0007XE-GP for qemu-devel@nongnu.org; Mon, 17 Jul 2017 18:15:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dXEIb-00022g-Jw for qemu-devel@nongnu.org; Mon, 17 Jul 2017 18:15:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51256) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dXEIX-0001xm-4f; Mon, 17 Jul 2017 18:15:25 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1635F85376; Mon, 17 Jul 2017 22:15:24 +0000 (UTC) Received: from red.redhat.com (ovpn-120-160.rdu2.redhat.com [10.10.120.160]) by smtp.corp.redhat.com (Postfix) with ESMTP id 56C6160BF1; Mon, 17 Jul 2017 22:15:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 1635F85376 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 1635F85376 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 17 Jul 2017 17:15:15 -0500 Message-Id: <20170717221516.7692-3-eblake@redhat.com> In-Reply-To: <20170717221516.7692-1-eblake@redhat.com> References: <20170717221516.7692-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Mon, 17 Jul 2017 22:15:24 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 2/3] nbd: Trace client command being sent X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini , "open list:Network Block Dev..." Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Make the client trace slightly more legible by including the name of the command being sent. Signed-off-by: Eric Blake Reviewed-by: John Snow Message-Id: <20170717192635.17880-2-eblake@redhat.com> --- nbd/client.c | 3 ++- nbd/trace-events | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 846bdc394c..509ed5e4ba 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -901,7 +901,8 @@ ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *r= equest) uint8_t buf[NBD_REQUEST_SIZE]; trace_nbd_send_request(request->from, request->len, request->handle, - request->flags, request->type); + request->flags, request->type, + nbd_cmd_lookup(request->type)); stl_be_p(buf, NBD_REQUEST_MAGIC); stw_be_p(buf + 4, request->flags); diff --git a/nbd/trace-events b/nbd/trace-events index f5024d85a1..d7c7746ea8 100644 --- a/nbd/trace-events +++ b/nbd/trace-events @@ -28,7 +28,7 @@ nbd_client_loop(void) "Doing NBD loop" nbd_client_loop_ret(int ret, const char *error) "NBD loop returned %d: %s" nbd_client_clear_queue(void) "Clearing NBD queue" nbd_client_clear_socket(void) "Clearing NBD socket" -nbd_send_request(uint64_t from, uint32_t len, uint64_t handle, uint16_t fl= ags, uint16_t type) "Sending request to server: { .from =3D %" PRIu64", .le= n =3D %" PRIu32 ", .handle =3D %" PRIu64 ", .flags =3D %" PRIx16 ", .type = =3D %" PRIu16 " }" +nbd_send_request(uint64_t from, uint32_t len, uint64_t handle, uint16_t fl= ags, uint16_t type, const char *name) "Sending request to server: { .from = =3D %" PRIu64", .len =3D %" PRIu32 ", .handle =3D %" PRIu64 ", .flags =3D %= " PRIx16 ", .type =3D %" PRIu16 " (%s) }" nbd_receive_reply(uint32_t magic, int32_t error, uint64_t handle) "Got rep= ly: { magic =3D 0x%" PRIx32 ", .error =3D % " PRId32 ", handle =3D %" PRIu6= 4" }" # nbd/server.c --=20 2.13.3 From nobody Thu May 2 22:23:37 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1500329825135602.9275959801222; Mon, 17 Jul 2017 15:17:05 -0700 (PDT) Received: from localhost ([::1]:52800 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dXEK7-0000Iy-OB for importer@patchew.org; Mon, 17 Jul 2017 18:17:03 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59873) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dXEId-0007Xl-1Y for qemu-devel@nongnu.org; Mon, 17 Jul 2017 18:15:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dXEIb-00022y-Ph for qemu-devel@nongnu.org; Mon, 17 Jul 2017 18:15:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47416) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dXEIX-0001yj-TI; Mon, 17 Jul 2017 18:15:26 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E2D8661B83; Mon, 17 Jul 2017 22:15:24 +0000 (UTC) Received: from red.redhat.com (ovpn-120-160.rdu2.redhat.com [10.10.120.160]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4DB6277C00; Mon, 17 Jul 2017 22:15:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com E2D8661B83 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com E2D8661B83 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 17 Jul 2017 17:15:16 -0500 Message-Id: <20170717221516.7692-4-eblake@redhat.com> In-Reply-To: <20170717221516.7692-1-eblake@redhat.com> References: <20170717221516.7692-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 17 Jul 2017 22:15:25 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 3/3] nbd: Fix server reply to NBD_OPT_EXPORT_NAME of older clients X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini , "open list:Network Block Dev..." Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" A typo in commit 23e099c set the size of buf[] used in response to NBD_OPT_EXPORT_NAME according to the length needed for old-style negotiation (4 bytes of flag information) instead of the intended 2 bytes used in new style. If the client doesn't enable NBD_FLAG_C_NO_ZEROES, then the server sends two bytes too many, and is then out of sync in response to the client's next command (the bug is masked when modern qemu is the client, since we enable the no zeroes flag). While touching this code, add some more defines to nbd_internal.h rather than having quite so many magic numbers in the .c; also, use "" initialization rather than memset(), and tweak the oldstyle negotiation to better match the spec description of the layout (since the spec is big-endian, skipping two bytes as 0 followed by writing a 2-byte flag is the same as writing a zero-extended 4-byte flag), to make it a bit easier to follow compared to the spec. [checkpatch.pl has some false positives in the comments] Signed-off-by: Eric Blake Message-Id: <20170717192635.17880-3-eblake@redhat.com> Reviewed-by: John Snow --- nbd/nbd-internal.h | 8 ++++++-- nbd/server.c | 18 ++++++++++-------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h index 4065bc68ac..396ddb4d3e 100644 --- a/nbd/nbd-internal.h +++ b/nbd/nbd-internal.h @@ -38,9 +38,13 @@ */ /* Size of all NBD_OPT_*, without payload */ -#define NBD_REQUEST_SIZE (4 + 2 + 2 + 8 + 8 + 4) +#define NBD_REQUEST_SIZE (4 + 2 + 2 + 8 + 8 + 4) /* Size of all NBD_REP_* sent in answer to most NBD_OPT_*, without payload= */ -#define NBD_REPLY_SIZE (4 + 4 + 8) +#define NBD_REPLY_SIZE (4 + 4 + 8) +/* Size of reply to NBD_OPT_EXPORT_NAME */ +#define NBD_REPLY_EXPORT_NAME_SIZE (8 + 2 + 124) +/* Size of oldstyle negotiation */ +#define NBD_OLDSTYLE_NEGOTIATE_SIZE (8 + 8 + 8 + 4 + 124) #define NBD_REQUEST_MAGIC 0x25609513 #define NBD_REPLY_MAGIC 0x67446698 diff --git a/nbd/server.c b/nbd/server.c index 49ed57455c..82a78bf439 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -283,12 +283,16 @@ static int nbd_negotiate_handle_export_name(NBDClient= *client, uint32_t length, Error **errp) { char name[NBD_MAX_NAME_SIZE + 1]; - char buf[8 + 4 + 124] =3D ""; + char buf[NBD_REPLY_EXPORT_NAME_SIZE] =3D ""; size_t len; int ret; /* Client sends: [20 .. xx] export name (length bytes) + Server replies: + [ 0 .. 7] size + [ 8 .. 9] export flags + [10 .. 133] reserved (0) [unless no_zeroes] */ trace_nbd_negotiate_handle_export_name(); if (length >=3D sizeof(name)) { @@ -800,22 +804,21 @@ static int nbd_negotiate_options(NBDClient *client, u= int16_t myflags, */ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp) { - char buf[8 + 8 + 8 + 128]; + char buf[NBD_OLDSTYLE_NEGOTIATE_SIZE] =3D ""; int ret; const uint16_t myflags =3D (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM | NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA | NBD_FLAG_SEND_WRITE_ZEROES); bool oldStyle; - /* Old style negotiation header without options + /* Old style negotiation header, no room for options [ 0 .. 7] passwd ("NBDMAGIC") [ 8 .. 15] magic (NBD_CLIENT_MAGIC) [16 .. 23] size - [24 .. 25] server flags (0) - [26 .. 27] export flags + [24 .. 27] export flags (zero-extended) [28 .. 151] reserved (0) - New style negotiation header with options + New style negotiation header, client can send options [ 0 .. 7] passwd ("NBDMAGIC") [ 8 .. 15] magic (NBD_OPTS_MAGIC) [16 .. 17] server flags (0) @@ -825,7 +828,6 @@ static coroutine_fn int nbd_negotiate(NBDClient *client= , Error **errp) qio_channel_set_blocking(client->ioc, false, NULL); trace_nbd_negotiate_begin(); - memset(buf, 0, sizeof(buf)); memcpy(buf, "NBDMAGIC", 8); oldStyle =3D client->exp !=3D NULL && !client->tlscreds; @@ -834,7 +836,7 @@ static coroutine_fn int nbd_negotiate(NBDClient *client= , Error **errp) client->exp->nbdflags | myflags); stq_be_p(buf + 8, NBD_CLIENT_MAGIC); stq_be_p(buf + 16, client->exp->size); - stw_be_p(buf + 26, client->exp->nbdflags | myflags); + stl_be_p(buf + 24, client->exp->nbdflags | myflags); if (nbd_write(client->ioc, buf, sizeof(buf), errp) < 0) { error_prepend(errp, "write failed: "); --=20 2.13.3