From nobody Fri May 17 23:49:27 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 1499459732327755.8058781859588; Fri, 7 Jul 2017 13:35:32 -0700 (PDT) Received: from localhost ([::1]:58714 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZyN-0006dr-0O for importer@patchew.org; Fri, 07 Jul 2017 16:35:31 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46682) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZuC-0002u8-Ik for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dTZuA-0003t6-UQ for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:12 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60014) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dTZu4-0003hI-C9; Fri, 07 Jul 2017 16:31:04 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3B0CB5D68D; Fri, 7 Jul 2017 20:31:03 +0000 (UTC) Received: from red.redhat.com (ovpn-120-204.rdu2.redhat.com [10.10.120.204]) by smtp.corp.redhat.com (Postfix) with ESMTP id 07BB671C55; Fri, 7 Jul 2017 20:30:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 3B0CB5D68D 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 3B0CB5D68D From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 7 Jul 2017 15:30:41 -0500 Message-Id: <20170707203049.534-2-eblake@redhat.com> In-Reply-To: <20170707203049.534-1-eblake@redhat.com> References: <20170707203049.534-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 07 Jul 2017 20:31:03 +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] [PATCH v5 1/9] nbd: Create struct for tracking export info 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 , vsementsov@virtuozzo.com, den@virtuozzo.com, qemu-block@nongnu.org, Max Reitz , marcandre.lureau@redhat.com, pbonzini@redhat.com 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" The NBD Protocol is introducing some additional information about exports, such as minimum request size and alignment, as well as an advertised maximum request size. It will be easier to feed this information back to the block layer if we gather all the information into a struct, rather than adding yet more pointer parameters during negotiation. Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy --- v5: rebase to master, enough differences to drop R-b v4: rebase to master v3: new patch --- block/nbd-client.h | 3 +-- include/block/nbd.h | 15 +++++++++++---- block/nbd-client.c | 18 ++++++++---------- block/nbd.c | 2 +- nbd/client.c | 44 ++++++++++++++++++++++---------------------- qemu-nbd.c | 10 ++++------ 6 files changed, 47 insertions(+), 45 deletions(-) diff --git a/block/nbd-client.h b/block/nbd-client.h index 49636bc..df80771 100644 --- a/block/nbd-client.h +++ b/block/nbd-client.h @@ -20,8 +20,7 @@ typedef struct NBDClientSession { QIOChannelSocket *sioc; /* The master data channel */ QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) = */ - uint16_t nbdflags; - off_t size; + NBDExportInfo info; CoMutex send_mutex; CoQueue free_sema; diff --git a/include/block/nbd.h b/include/block/nbd.h index 6d75d5a..9092374 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -123,13 +123,20 @@ enum { * aren't overflowing some other buffer. */ #define NBD_MAX_NAME_SIZE 256 +/* Details collected by NBD_OPT_EXPORT_NAME and NBD_OPT_GO */ +struct NBDExportInfo { + uint64_t size; + uint16_t flags; +}; +typedef struct NBDExportInfo NBDExportInfo; + ssize_t nbd_rwv(QIOChannel *ioc, struct iovec *iov, size_t niov, size_t le= ngth, bool do_read, Error **errp); -int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *fla= gs, +int nbd_receive_negotiate(QIOChannel *ioc, const char *name, QCryptoTLSCreds *tlscreds, const char *hostname, - QIOChannel **outioc, - off_t *size, Error **errp); -int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t flags, off_t size, + QIOChannel **outioc, NBDExportInfo *info, + Error **errp); +int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info, Error **errp); ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request); ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp); diff --git a/block/nbd-client.c b/block/nbd-client.c index 208f907..aab1e32 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -242,7 +242,7 @@ int nbd_client_co_pwritev(BlockDriverState *bs, uint64_= t offset, ssize_t ret; if (flags & BDRV_REQ_FUA) { - assert(client->nbdflags & NBD_FLAG_SEND_FUA); + assert(client->info.flags & NBD_FLAG_SEND_FUA); request.flags |=3D NBD_CMD_FLAG_FUA; } @@ -270,12 +270,12 @@ int nbd_client_co_pwrite_zeroes(BlockDriverState *bs,= int64_t offset, }; NBDReply reply; - if (!(client->nbdflags & NBD_FLAG_SEND_WRITE_ZEROES)) { + if (!(client->info.flags & NBD_FLAG_SEND_WRITE_ZEROES)) { return -ENOTSUP; } if (flags & BDRV_REQ_FUA) { - assert(client->nbdflags & NBD_FLAG_SEND_FUA); + assert(client->info.flags & NBD_FLAG_SEND_FUA); request.flags |=3D NBD_CMD_FLAG_FUA; } if (!(flags & BDRV_REQ_MAY_UNMAP)) { @@ -299,7 +299,7 @@ int nbd_client_co_flush(BlockDriverState *bs) NBDReply reply; ssize_t ret; - if (!(client->nbdflags & NBD_FLAG_SEND_FLUSH)) { + if (!(client->info.flags & NBD_FLAG_SEND_FLUSH)) { return 0; } @@ -327,7 +327,7 @@ int nbd_client_co_pdiscard(BlockDriverState *bs, int64_= t offset, int bytes) NBDReply reply; ssize_t ret; - if (!(client->nbdflags & NBD_FLAG_SEND_TRIM)) { + if (!(client->info.flags & NBD_FLAG_SEND_TRIM)) { return 0; } @@ -385,19 +385,17 @@ int nbd_client_init(BlockDriverState *bs, qio_channel_set_blocking(QIO_CHANNEL(sioc), true, NULL); ret =3D nbd_receive_negotiate(QIO_CHANNEL(sioc), export, - &client->nbdflags, tlscreds, hostname, - &client->ioc, - &client->size, errp); + &client->ioc, &client->info, errp); if (ret < 0) { logout("Failed to negotiate with the NBD server\n"); return ret; } - if (client->nbdflags & NBD_FLAG_SEND_FUA) { + if (client->info.flags & NBD_FLAG_SEND_FUA) { bs->supported_write_flags =3D BDRV_REQ_FUA; bs->supported_zero_flags |=3D BDRV_REQ_FUA; } - if (client->nbdflags & NBD_FLAG_SEND_WRITE_ZEROES) { + if (client->info.flags & NBD_FLAG_SEND_WRITE_ZEROES) { bs->supported_zero_flags |=3D BDRV_REQ_MAY_UNMAP; } diff --git a/block/nbd.c b/block/nbd.c index d529305..4a9048c 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -492,7 +492,7 @@ static int64_t nbd_getlength(BlockDriverState *bs) { BDRVNBDState *s =3D bs->opaque; - return s->client.size; + return s->client.info.size; } static void nbd_detach_aio_context(BlockDriverState *bs) diff --git a/nbd/client.c b/nbd/client.c index 9c52b9b..607ad32 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -425,13 +425,13 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *i= oc, } -int nbd_receive_negotiate(QIOChannel *ioc, const char *name, uint16_t *fla= gs, +int nbd_receive_negotiate(QIOChannel *ioc, const char *name, QCryptoTLSCreds *tlscreds, const char *hostname, - QIOChannel **outioc, - off_t *size, Error **errp) + QIOChannel **outioc, NBDExportInfo *info, + Error **errp) { char buf[256]; - uint64_t magic, s; + uint64_t magic; int rc; bool zeroes =3D true; @@ -532,17 +532,17 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char= *name, uint16_t *flags, } /* Read the response */ - if (nbd_read(ioc, &s, sizeof(s), errp) < 0) { + if (nbd_read(ioc, &info->size, sizeof(info->size), errp) < 0) { error_prepend(errp, "Failed to read export length"); goto fail; } - *size =3D be64_to_cpu(s); + be64_to_cpus(&info->size); - if (nbd_read(ioc, flags, sizeof(*flags), errp) < 0) { + if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0) { error_prepend(errp, "Failed to read export flags"); goto fail; } - be16_to_cpus(flags); + be16_to_cpus(&info->flags); } else if (magic =3D=3D NBD_CLIENT_MAGIC) { uint32_t oldflags; @@ -555,11 +555,11 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char= *name, uint16_t *flags, goto fail; } - if (nbd_read(ioc, &s, sizeof(s), errp) < 0) { + if (nbd_read(ioc, &info->size, sizeof(info->size), errp) < 0) { error_prepend(errp, "Failed to read export length"); goto fail; } - *size =3D be64_to_cpu(s); + be64_to_cpus(&info->size); if (nbd_read(ioc, &oldflags, sizeof(oldflags), errp) < 0) { error_prepend(errp, "Failed to read export flags"); @@ -570,13 +570,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char= *name, uint16_t *flags, error_setg(errp, "Unexpected export flags %0x" PRIx32, oldflag= s); goto fail; } - *flags =3D oldflags; + info->flags =3D oldflags; } else { error_setg(errp, "Bad magic received"); goto fail; } - trace_nbd_receive_negotiate_size_flags(*size, *flags); + trace_nbd_receive_negotiate_size_flags(info->size, info->flags); if (zeroes && nbd_drop(ioc, 124, errp) < 0) { error_prepend(errp, "Failed to read reserved block"); goto fail; @@ -588,13 +588,13 @@ fail: } #ifdef __linux__ -int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t flags, off_t size, +int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info, Error **errp) { - unsigned long sectors =3D size / BDRV_SECTOR_SIZE; - if (size / BDRV_SECTOR_SIZE !=3D sectors) { - error_setg(errp, "Export size %lld too large for 32-bit kernel", - (long long) size); + unsigned long sectors =3D info->size / BDRV_SECTOR_SIZE; + if (info->size / BDRV_SECTOR_SIZE !=3D sectors) { + error_setg(errp, "Export size %" PRId64 " too large for 32-bit ker= nel", + info->size); return -E2BIG; } @@ -615,8 +615,8 @@ int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t f= lags, off_t size, } trace_nbd_init_set_size(sectors); - if (size % BDRV_SECTOR_SIZE) { - trace_nbd_init_trailing_bytes(size % BDRV_SECTOR_SIZE); + if (info->size % BDRV_SECTOR_SIZE) { + trace_nbd_init_trailing_bytes(info->size % BDRV_SECTOR_SIZE); } if (ioctl(fd, NBD_SET_SIZE_BLOCKS, sectors) < 0) { @@ -625,9 +625,9 @@ int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t f= lags, off_t size, return -serrno; } - if (ioctl(fd, NBD_SET_FLAGS, (unsigned long) flags) < 0) { + if (ioctl(fd, NBD_SET_FLAGS, (unsigned long) info->flags) < 0) { if (errno =3D=3D ENOTTY) { - int read_only =3D (flags & NBD_FLAG_READ_ONLY) !=3D 0; + int read_only =3D (info->flags & NBD_FLAG_READ_ONLY) !=3D 0; trace_nbd_init_set_readonly(); if (ioctl(fd, BLKROSET, (unsigned long) &read_only) < 0) { @@ -685,7 +685,7 @@ int nbd_disconnect(int fd) } #else -int nbd_init(int fd, QIOChannelSocket *ioc, uint16_t flags, off_t size, +int nbd_init(int fd, QIOChannelSocket *ioc, NBDExportInfo *info, Error **errp) { error_setg(errp, "nbd_init is only supported on Linux"); diff --git a/qemu-nbd.c b/qemu-nbd.c index 4dd3fd4..c8bd47f 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -255,8 +255,7 @@ static void *show_parts(void *arg) static void *nbd_client_thread(void *arg) { char *device =3D arg; - off_t size; - uint16_t nbdflags; + NBDExportInfo info; QIOChannelSocket *sioc; int fd; int ret; @@ -271,9 +270,8 @@ static void *nbd_client_thread(void *arg) goto out; } - ret =3D nbd_receive_negotiate(QIO_CHANNEL(sioc), NULL, &nbdflags, - NULL, NULL, NULL, - &size, &local_error); + ret =3D nbd_receive_negotiate(QIO_CHANNEL(sioc), NULL, + NULL, NULL, NULL, &info, &local_error); if (ret < 0) { if (local_error) { error_report_err(local_error); @@ -288,7 +286,7 @@ static void *nbd_client_thread(void *arg) goto out_socket; } - ret =3D nbd_init(fd, sioc, nbdflags, size, &local_error); + ret =3D nbd_init(fd, sioc, &info, &local_error); if (ret < 0) { error_report_err(local_error); goto out_fd; --=20 2.9.4 From nobody Fri May 17 23:49:27 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 1499459579439175.39504662772038; Fri, 7 Jul 2017 13:32:59 -0700 (PDT) Received: from localhost ([::1]:58693 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZvt-00047B-Qi for importer@patchew.org; Fri, 07 Jul 2017 16:32:57 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46664) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZuB-0002tK-QT for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dTZuB-0003tP-0V for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:54468) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dTZu5-0003iI-DI; Fri, 07 Jul 2017 16:31:05 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6322583F3F; Fri, 7 Jul 2017 20:31:04 +0000 (UTC) Received: from red.redhat.com (ovpn-120-204.rdu2.redhat.com [10.10.120.204]) by smtp.corp.redhat.com (Postfix) with ESMTP id 66DF0729C3; Fri, 7 Jul 2017 20:31:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6322583F3F Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 6322583F3F From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 7 Jul 2017 15:30:42 -0500 Message-Id: <20170707203049.534-3-eblake@redhat.com> In-Reply-To: <20170707203049.534-1-eblake@redhat.com> References: <20170707203049.534-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Fri, 07 Jul 2017 20:31:04 +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] [PATCH v5 2/9] nbd: Don't bother tracing an NBD_OPT_ABORT response failure 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: pbonzini@redhat.com, vsementsov@virtuozzo.com, den@virtuozzo.com, qemu-block@nongnu.org, marcandre.lureau@redhat.com 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" We really don't care if our spec-compliant reply to NBD_OPT_ABORT was received, so shave off some lines of code by not even tracing it. Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy --- v5: new patch --- nbd/server.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 9b0c588..e15385b 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -376,7 +376,6 @@ static int nbd_negotiate_options(NBDClient *client, Err= or **errp) { uint32_t flags; bool fixedNewstyle =3D false; - Error *local_err =3D NULL; /* Client sends: [ 0 .. 3] client flags @@ -479,7 +478,9 @@ static int nbd_negotiate_options(NBDClient *client, Err= or **errp) if (ret < 0) { return ret; } - /* Let the client keep trying, unless they asked to quit */ + /* Let the client keep trying, unless they asked to + * quit. In this mode, we've already sent an error, so + * we can't ack the abort. */ if (option =3D=3D NBD_OPT_ABORT) { return 1; } @@ -498,15 +499,7 @@ static int nbd_negotiate_options(NBDClient *client, Er= ror **errp) /* NBD spec says we must try to reply before * disconnecting, but that we must also tolerate * guests that don't wait for our reply. */ - nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, option, - &local_err); - - if (local_err !=3D NULL) { - const char *error =3D error_get_pretty(local_err); - trace_nbd_opt_abort_reply_failed(error); - error_free(local_err); - } - + nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, option, N= ULL); return 1; case NBD_OPT_EXPORT_NAME: --=20 2.9.4 From nobody Fri May 17 23:49:27 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 1499459745038904.8432430469774; Fri, 7 Jul 2017 13:35:45 -0700 (PDT) Received: from localhost ([::1]:58715 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZyY-0006s7-KL for importer@patchew.org; Fri, 07 Jul 2017 16:35:42 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46741) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZuG-0002xP-0F for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dTZuD-0003wo-OM for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60176) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dTZu8-0003oE-Nb; Fri, 07 Jul 2017 16:31:08 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B022B6267F; Fri, 7 Jul 2017 20:31:07 +0000 (UTC) Received: from red.redhat.com (ovpn-120-204.rdu2.redhat.com [10.10.120.204]) by smtp.corp.redhat.com (Postfix) with ESMTP id A08AB71C55; Fri, 7 Jul 2017 20:31:04 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com B022B6267F 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 B022B6267F From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 7 Jul 2017 15:30:43 -0500 Message-Id: <20170707203049.534-4-eblake@redhat.com> In-Reply-To: <20170707203049.534-1-eblake@redhat.com> References: <20170707203049.534-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 07 Jul 2017 20:31:07 +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] [PATCH v5 3/9] nbd: Expose and debug more NBD constants 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 , vsementsov@virtuozzo.com, den@virtuozzo.com, qemu-block@nongnu.org, Max Reitz , marcandre.lureau@redhat.com, pbonzini@redhat.com 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" The NBD protocol has several constants defined in various extensions that we are about to implement. Expose them to the code, along with an easy way to map various constants to strings during diagnostic messages. Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy --- v5: rebase to master, add command name lookup; enough changes that R-b is dropped v4: new patch --- include/block/nbd.h | 35 +++++++++++++++----- nbd/nbd-internal.h | 10 +++--- nbd/client.c | 52 +++++++++++++++++++----------- nbd/common.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++= ++++ nbd/server.c | 18 +++++++---- nbd/trace-events | 12 +++---- 6 files changed, 174 insertions(+), 45 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index 9092374..4a22eca 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016 Red Hat, Inc. + * Copyright (C) 2016-2017 Red Hat, Inc. * Copyright (C) 2005 Anthony Liguori * * Network Block Device @@ -83,18 +83,37 @@ typedef struct NBDReply NBDReply; #define NBD_FLAG_C_FIXED_NEWSTYLE (1 << 0) /* Fixed newstyle protocol. */ #define NBD_FLAG_C_NO_ZEROES (1 << 1) /* End handshake without zeroes= . */ -/* Reply types. */ +/* Option requests. */ +#define NBD_OPT_EXPORT_NAME (1) +#define NBD_OPT_ABORT (2) +#define NBD_OPT_LIST (3) +/* #define NBD_OPT_PEEK_EXPORT (4) not in use */ +#define NBD_OPT_STARTTLS (5) +#define NBD_OPT_INFO (6) +#define NBD_OPT_GO (7) +#define NBD_OPT_STRUCTURED_REPLY (8) + +/* Option reply types. */ #define NBD_REP_ERR(value) ((UINT32_C(1) << 31) | (value)) #define NBD_REP_ACK (1) /* Data sending finished. = */ #define NBD_REP_SERVER (2) /* Export description. */ +#define NBD_REP_INFO (3) /* NBD_OPT_INFO/GO. */ -#define NBD_REP_ERR_UNSUP NBD_REP_ERR(1) /* Unknown option */ -#define NBD_REP_ERR_POLICY NBD_REP_ERR(2) /* Server denied */ -#define NBD_REP_ERR_INVALID NBD_REP_ERR(3) /* Invalid length */ -#define NBD_REP_ERR_PLATFORM NBD_REP_ERR(4) /* Not compiled in */ -#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR(5) /* TLS required */ -#define NBD_REP_ERR_SHUTDOWN NBD_REP_ERR(7) /* Server shutting down */ +#define NBD_REP_ERR_UNSUP NBD_REP_ERR(1) /* Unknown option */ +#define NBD_REP_ERR_POLICY NBD_REP_ERR(2) /* Server denied */ +#define NBD_REP_ERR_INVALID NBD_REP_ERR(3) /* Invalid length */ +#define NBD_REP_ERR_PLATFORM NBD_REP_ERR(4) /* Not compiled in */ +#define NBD_REP_ERR_TLS_REQD NBD_REP_ERR(5) /* TLS required */ +#define NBD_REP_ERR_UNKNOWN NBD_REP_ERR(6) /* Export unknown */ +#define NBD_REP_ERR_SHUTDOWN NBD_REP_ERR(7) /* Server shutting dow= n */ +#define NBD_REP_ERR_BLOCK_SIZE_REQD NBD_REP_ERR(8) /* Need INFO_BLOCK_SIZ= E */ + +/* Info types, used during NBD_REP_INFO */ +#define NBD_INFO_EXPORT 0 +#define NBD_INFO_NAME 1 +#define NBD_INFO_DESCRIPTION 2 +#define NBD_INFO_BLOCK_SIZE 3 /* Request flags, sent from client to server during transmission phase */ #define NBD_CMD_FLAG_FUA (1 << 0) /* 'force unit access' during wri= te */ diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h index cf6ecbf..bf95601 100644 --- a/nbd/nbd-internal.h +++ b/nbd/nbd-internal.h @@ -57,12 +57,6 @@ #define NBD_SET_TIMEOUT _IO(0xab, 9) #define NBD_SET_FLAGS _IO(0xab, 10) -#define NBD_OPT_EXPORT_NAME (1) -#define NBD_OPT_ABORT (2) -#define NBD_OPT_LIST (3) -#define NBD_OPT_PEEK_EXPORT (4) -#define NBD_OPT_STARTTLS (5) - /* NBD errors are based on errno numbers, so there is a 1:1 mapping, * but only a limited set of errno values is specified in the protocol. * Everything else is squashed to EINVAL. @@ -133,6 +127,10 @@ struct NBDTLSHandshakeData { void nbd_tls_handshake(QIOTask *task, void *opaque); +const char *nbd_opt_lookup(uint32_t opt); +const char *nbd_rep_lookup(uint32_t rep); +const char *nbd_info_lookup(uint16_t info); +const char *nbd_cmd_lookup(uint16_t info); int nbd_drop(QIOChannel *ioc, size_t size, Error **errp); diff --git a/nbd/client.c b/nbd/client.c index 607ad32..011960b 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016 Red Hat, Inc. + * Copyright (C) 2016-2017 Red Hat, Inc. * Copyright (C) 2005 Anthony Liguori * * Network Block Device Client Side @@ -104,7 +104,7 @@ static int nbd_send_option_request(QIOChannel *ioc, uin= t32_t opt, if (len =3D=3D -1) { req.length =3D len =3D strlen(data); } - trace_nbd_send_option_request(opt, len); + trace_nbd_send_option_request(opt, nbd_opt_lookup(opt), len); stq_be_p(&req.magic, NBD_OPTS_MAGIC); stl_be_p(&req.option, opt); @@ -154,7 +154,9 @@ static int nbd_receive_option_reply(QIOChannel *ioc, ui= nt32_t opt, be32_to_cpus(&reply->type); be32_to_cpus(&reply->length); - trace_nbd_receive_option_reply(reply->option, reply->type, reply->leng= th); + trace_nbd_receive_option_reply(reply->option, nbd_opt_lookup(reply->op= tion), + reply->type, nbd_rep_lookup(reply->type= ), + reply->length); if (reply->magic !=3D NBD_REP_MAGIC) { error_setg(errp, "Unexpected option reply magic"); @@ -188,12 +190,16 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_= opt_reply *reply, if (reply->length) { if (reply->length > NBD_MAX_BUFFER_SIZE) { - error_setg(errp, "server's error message is too long"); + error_setg(errp, "server error 0x%" PRIx32 + " (%s) message is too long", + reply->type, nbd_rep_lookup(reply->type)); goto cleanup; } msg =3D g_malloc(reply->length + 1); if (nbd_read(ioc, msg, reply->length, errp) < 0) { - error_prepend(errp, "failed to read option error message"); + error_prepend(errp, "failed to read option error 0x%" PRIx32 + " (%s) message", + reply->type, nbd_rep_lookup(reply->type)); goto cleanup; } msg[reply->length] =3D '\0'; @@ -201,38 +207,48 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_= opt_reply *reply, switch (reply->type) { case NBD_REP_ERR_UNSUP: - trace_nbd_reply_err_unsup(reply->option); + trace_nbd_reply_err_unsup(reply->option, nbd_opt_lookup(reply->opt= ion)); result =3D 0; goto cleanup; case NBD_REP_ERR_POLICY: - error_setg(errp, "Denied by server for option %" PRIx32, - reply->option); + error_setg(errp, "Denied by server for option %" PRIx32 " (%s)", + reply->option, nbd_opt_lookup(reply->option)); break; case NBD_REP_ERR_INVALID: - error_setg(errp, "Invalid data length for option %" PRIx32, - reply->option); + error_setg(errp, "Invalid data length for option %" PRIx32 " (%s)", + reply->option, nbd_opt_lookup(reply->option)); break; case NBD_REP_ERR_PLATFORM: - error_setg(errp, "Server lacks support for option %" PRIx32, - reply->option); + error_setg(errp, "Server lacks support for option %" PRIx32 " (%s)= ", + reply->option, nbd_opt_lookup(reply->option)); break; case NBD_REP_ERR_TLS_REQD: - error_setg(errp, "TLS negotiation required before option %" PRIx32, - reply->option); + error_setg(errp, "TLS negotiation required before option %" PRIx32 + " (%s)", reply->option, nbd_opt_lookup(reply->option)); + 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)); break; case NBD_REP_ERR_SHUTDOWN: - error_setg(errp, "Server shutting down before option %" PRIx32, - reply->option); + error_setg(errp, "Server shutting down before option %" PRIx32 " (= %s)", + reply->option, nbd_opt_lookup(reply->option)); + break; + + case NBD_REP_ERR_BLOCK_SIZE_REQD: + error_setg(errp, "Server requires INFO_BLOCK_SIZE for option %" PR= Ix32 + " (%s)", reply->option, nbd_opt_lookup(reply->option)); break; default: - error_setg(errp, "Unknown error code when asking for option %" PRI= x32, - reply->option); + error_setg(errp, "Unknown error code when asking for option %" PRI= x32 + " (%s)", reply->option, nbd_opt_lookup(reply->option)); break; } diff --git a/nbd/common.c b/nbd/common.c index 4dab41e..a2f28f2 100644 --- a/nbd/common.c +++ b/nbd/common.c @@ -101,3 +101,95 @@ void nbd_tls_handshake(QIOTask *task, data->complete =3D true; g_main_loop_quit(data->loop); } + + +const char *nbd_opt_lookup(uint32_t opt) +{ + switch (opt) { + case NBD_OPT_EXPORT_NAME: + return "export name"; + case NBD_OPT_ABORT: + return "abort"; + case NBD_OPT_LIST: + return "list"; + case NBD_OPT_STARTTLS: + return "starttls"; + case NBD_OPT_INFO: + return "info"; + case NBD_OPT_GO: + return "go"; + case NBD_OPT_STRUCTURED_REPLY: + return "structured reply"; + default: + return ""; + } +} + + +const char *nbd_rep_lookup(uint32_t rep) +{ + switch (rep) { + case NBD_REP_ACK: + return "ack"; + case NBD_REP_SERVER: + return "server"; + case NBD_REP_INFO: + return "info"; + case NBD_REP_ERR_UNSUP: + return "unsupported"; + case NBD_REP_ERR_POLICY: + return "denied by policy"; + case NBD_REP_ERR_INVALID: + return "invalid"; + case NBD_REP_ERR_PLATFORM: + return "platform lacks support"; + case NBD_REP_ERR_TLS_REQD: + return "TLS required"; + case NBD_REP_ERR_UNKNOWN: + return "export unknown"; + case NBD_REP_ERR_SHUTDOWN: + return "server shutting down"; + case NBD_REP_ERR_BLOCK_SIZE_REQD: + return "block size required"; + default: + return ""; + } +} + + +const char *nbd_info_lookup(uint16_t info) +{ + switch (info) { + case NBD_INFO_EXPORT: + return "export"; + case NBD_INFO_NAME: + return "name"; + case NBD_INFO_DESCRIPTION: + return "description"; + case NBD_INFO_BLOCK_SIZE: + return "block size"; + default: + return ""; + } +} + + +const char *nbd_cmd_lookup(uint16_t cmd) +{ + switch (cmd) { + case NBD_CMD_READ: + return "read"; + case NBD_CMD_WRITE: + return "write"; + case NBD_CMD_DISC: + return "discard"; + case NBD_CMD_FLUSH: + return "flush"; + case NBD_CMD_TRIM: + return "trim"; + case NBD_CMD_WRITE_ZEROES: + return "write zeroes"; + default: + return ""; + } +} diff --git a/nbd/server.c b/nbd/server.c index e15385b..27a0aab 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016 Red Hat, Inc. + * Copyright (C) 2016-2017 Red Hat, Inc. * Copyright (C) 2005 Anthony Liguori * * Network Block Device Server Side @@ -139,7 +139,8 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc, = uint32_t type, { uint64_t magic; - trace_nbd_negotiate_send_rep_len(opt, type, len); + trace_nbd_negotiate_send_rep_len(opt, nbd_opt_lookup(opt), + type, nbd_rep_lookup(type), len); magic =3D cpu_to_be64(NBD_REP_MAGIC); if (nbd_write(ioc, &magic, sizeof(magic), errp) < 0) { @@ -441,7 +442,8 @@ static int nbd_negotiate_options(NBDClient *client, Err= or **errp) } length =3D be32_to_cpu(length); - trace_nbd_negotiate_options_check_option(option); + trace_nbd_negotiate_options_check_option(option, + nbd_opt_lookup(option)); if (client->tlscreds && client->ioc =3D=3D (QIOChannel *)client->sioc) { QIOChannel *tioc; @@ -532,8 +534,8 @@ static int nbd_negotiate_options(NBDClient *client, Err= or **errp) NBD_REP_ERR_UNSUP, option, errp, "Unsupported option 0x%" - PRIx32, - option); + PRIx32 " (%s)", option, + nbd_opt_lookup(option)); if (ret < 0) { return ret; } @@ -549,7 +551,8 @@ static int nbd_negotiate_options(NBDClient *client, Err= or **errp) return nbd_negotiate_handle_export_name(client, length, er= rp); default: - error_setg(errp, "Unsupported option 0x%" PRIx32, option); + error_setg(errp, "Unsupported option 0x%" PRIx32 " (%s)", + option, nbd_opt_lookup(option)); return -EINVAL; } } @@ -1033,7 +1036,8 @@ static int nbd_co_receive_request(NBDRequestData *req= , NBDRequest *request, return -EIO; } - trace_nbd_co_receive_request_decode_type(request->handle, request->typ= e); + trace_nbd_co_receive_request_decode_type(request->handle, request->typ= e, + nbd_cmd_lookup(request->type)= ); if (request->type !=3D NBD_CMD_WRITE) { /* No payload, we are ready to read the next request. */ diff --git a/nbd/trace-events b/nbd/trace-events index 4b233b8..e61feda 100644 --- a/nbd/trace-events +++ b/nbd/trace-events @@ -1,8 +1,8 @@ # nbd/client.c nbd_unknown_error(int err) "Squashing unexpected error %d to EINVAL" -nbd_send_option_request(uint32_t opt, uint32_t len) "Sending option reques= t %" PRIu32", len %" PRIu32 -nbd_receive_option_reply(uint32_t option, uint32_t type, uint32_t length) = "Received option reply %" PRIx32", type %" PRIx32", len %" PRIu32 -nbd_reply_err_unsup(uint32_t option) "server doesn't understand request %"= PRIx32 ", attempting fallback" +nbd_send_option_request(uint32_t opt, const char *name, uint32_t len) "Sen= ding option request %" PRIu32" (%s), len %" PRIu32 +nbd_receive_option_reply(uint32_t option, const char *optname, uint32_t ty= pe, const char *typename, uint32_t length) "Received option reply %" PRIx32= " (%s), type %" PRIx32" (%s), len %" PRIu32 +nbd_reply_err_unsup(uint32_t option, const char *name) "server doesn't und= erstand request %" PRIx32 " (%s), attempting fallback" nbd_receive_query_exports_start(const char *wantname) "Querying export lis= t for '%s'" nbd_receive_query_exports_success(const char *wantname) "Found desired exp= ort name '%s'" nbd_receive_starttls_request(void) "Requesting TLS from server" @@ -28,7 +28,7 @@ nbd_send_request(uint64_t from, uint32_t len, uint64_t ha= ndle, uint16_t flags, u 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 -nbd_negotiate_send_rep_len(uint32_t opt, uint32_t type, uint32_t len) "Rep= ly opt=3D%" PRIx32 " type=3D%" PRIx32 " len=3D%" PRIu32 +nbd_negotiate_send_rep_len(uint32_t opt, const char *optname, uint32_t typ= e, const char *typename, uint32_t len) "Reply opt=3D%" PRIx32 " (%s), type= =3D%" PRIx32 " (%s), len=3D%" PRIu32 nbd_negotiate_send_rep_err(const char *msg) "sending error message \"%s\"" nbd_negotiate_send_rep_list(const char *name, const char *desc) "Advertisi= ng export name '%s' description '%s'" nbd_negotiate_handle_export_name(void) "Checking length" @@ -39,7 +39,7 @@ nbd_negotiate_options_flags(void) "Checking client flags" nbd_negotiate_options_newstyle(void) "Client supports fixed newstyle hands= hake" nbd_negotiate_options_no_zeroes(void) "Client supports no zeroes at handsh= ake end" nbd_negotiate_options_check_magic(uint64_t magic) "Checking opts magic 0x%= " PRIx64 -nbd_negotiate_options_check_option(uint32_t option) "Checking option 0x%" = PRIx32 +nbd_negotiate_options_check_option(uint32_t option, const char *name) "Che= cking option 0x%" PRIx32 " (%s)" nbd_opt_abort_reply_failed(const char *error) "Reply to NBD_OPT_ABORT requ= est failed: %s" nbd_negotiate_begin(void) "Beginning negotiation" nbd_negotiate_old_style(uint64_t size, unsigned flags) "advertising size %= " PRIu64 " and flags %x" @@ -50,7 +50,7 @@ nbd_send_reply(int32_t error, uint64_t handle) "Sending r= esponse to client: { .e nbd_blk_aio_attached(const char *name, void *ctx) "Export %s: Attaching cl= ients to AIO context %p\n" nbd_blk_aio_detach(const char *name, void *ctx) "Export %s: Detaching clie= nts from AIO context %p\n" nbd_co_send_reply(uint64_t handle, uint32_t error, int len) "Send reply: h= andle =3D %" PRIu64 ", error =3D %" PRIu32 ", len =3D %d" -nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type) "Decodi= ng type: handle =3D %" PRIu64 ", type =3D %" PRIu16 +nbd_co_receive_request_decode_type(uint64_t handle, uint16_t type, const c= har *name) "Decoding type: handle =3D %" PRIu64 ", type =3D %" PRIu16 " (%s= )" nbd_co_receive_request_payload_received(uint64_t handle, uint32_t len) "Pa= yload received: handle =3D %" PRIu64 ", len =3D %" PRIu32 nbd_co_receive_request_cmd_write(uint32_t len) "Reading %" PRIu32 " byte(s= )" nbd_trip(void) "Reading request" --=20 2.9.4 From nobody Fri May 17 23:49:27 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 1499459587058439.30677293490317; Fri, 7 Jul 2017 13:33:07 -0700 (PDT) Received: from localhost ([::1]:58694 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZw1-0004D5-Ok for importer@patchew.org; Fri, 07 Jul 2017 16:33:05 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46699) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZuD-0002uq-ET for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dTZuC-0003vq-Ek for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60196) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dTZu9-0003pW-Sx; Fri, 07 Jul 2017 16:31:10 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DC7FF5D68D; Fri, 7 Jul 2017 20:31:08 +0000 (UTC) Received: from red.redhat.com (ovpn-120-204.rdu2.redhat.com [10.10.120.204]) by smtp.corp.redhat.com (Postfix) with ESMTP id E6DA671C55; Fri, 7 Jul 2017 20:31:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com DC7FF5D68D 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 DC7FF5D68D From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 7 Jul 2017 15:30:44 -0500 Message-Id: <20170707203049.534-5-eblake@redhat.com> In-Reply-To: <20170707203049.534-1-eblake@redhat.com> References: <20170707203049.534-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Fri, 07 Jul 2017 20:31:09 +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] [PATCH v5 4/9] nbd: Simplify trace of client flags in negotiation 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: pbonzini@redhat.com, vsementsov@virtuozzo.com, den@virtuozzo.com, qemu-block@nongnu.org, marcandre.lureau@redhat.com 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" Simplify the tracing of client flags in the server, and return -EINVAL instead of -EIO if we successfully read but don't like those flags. Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy --- v5: new patch --- nbd/server.c | 6 ++---- nbd/trace-events | 4 +--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 27a0aab..8e363d3 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -396,21 +396,19 @@ static int nbd_negotiate_options(NBDClient *client, E= rror **errp) error_prepend(errp, "read failed: "); return -EIO; } - trace_nbd_negotiate_options_flags(); be32_to_cpus(&flags); + trace_nbd_negotiate_options_flags(flags); if (flags & NBD_FLAG_C_FIXED_NEWSTYLE) { - trace_nbd_negotiate_options_newstyle(); fixedNewstyle =3D true; flags &=3D ~NBD_FLAG_C_FIXED_NEWSTYLE; } if (flags & NBD_FLAG_C_NO_ZEROES) { - trace_nbd_negotiate_options_no_zeroes(); client->no_zeroes =3D true; flags &=3D ~NBD_FLAG_C_NO_ZEROES; } if (flags !=3D 0) { error_setg(errp, "Unknown client flags 0x%" PRIx32 " received", fl= ags); - return -EIO; + return -EINVAL; } while (1) { diff --git a/nbd/trace-events b/nbd/trace-events index e61feda..765f5f4 100644 --- a/nbd/trace-events +++ b/nbd/trace-events @@ -35,9 +35,7 @@ nbd_negotiate_handle_export_name(void) "Checking length" nbd_negotiate_handle_export_name_request(const char *name) "Client request= ed export '%s'" nbd_negotiate_handle_starttls(void) "Setting up TLS" nbd_negotiate_handle_starttls_handshake(void) "Starting TLS handshake" -nbd_negotiate_options_flags(void) "Checking client flags" -nbd_negotiate_options_newstyle(void) "Client supports fixed newstyle hands= hake" -nbd_negotiate_options_no_zeroes(void) "Client supports no zeroes at handsh= ake end" +nbd_negotiate_options_flags(uint32_t flags) "Received client flags 0x%" PR= Ix32 nbd_negotiate_options_check_magic(uint64_t magic) "Checking opts magic 0x%= " PRIx64 nbd_negotiate_options_check_option(uint32_t option, const char *name) "Che= cking option 0x%" PRIx32 " (%s)" nbd_opt_abort_reply_failed(const char *error) "Reply to NBD_OPT_ABORT requ= est failed: %s" --=20 2.9.4 From nobody Fri May 17 23:49:27 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 1499459595937452.1139539735375; Fri, 7 Jul 2017 13:33:15 -0700 (PDT) Received: from localhost ([::1]:58695 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZwA-0004KM-I9 for importer@patchew.org; Fri, 07 Jul 2017 16:33:14 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46731) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZuF-0002x2-IF for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dTZuE-0003xs-D1 for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60682) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dTZuB-0003sj-AY; Fri, 07 Jul 2017 16:31:11 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2051FC0587C9; Fri, 7 Jul 2017 20:31:10 +0000 (UTC) Received: from red.redhat.com (ovpn-120-204.rdu2.redhat.com [10.10.120.204]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1D24A729C3; Fri, 7 Jul 2017 20:31:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 2051FC0587C9 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 2051FC0587C9 From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 7 Jul 2017 15:30:45 -0500 Message-Id: <20170707203049.534-6-eblake@redhat.com> In-Reply-To: <20170707203049.534-1-eblake@redhat.com> References: <20170707203049.534-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 07 Jul 2017 20:31:10 +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] [PATCH v5 5/9] nbd: Refactor reply to NBD_OPT_EXPORT_NAME 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: pbonzini@redhat.com, vsementsov@virtuozzo.com, den@virtuozzo.com, qemu-block@nongnu.org, marcandre.lureau@redhat.com 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" Reply directly in nbd_negotiate_handle_export_name(), rather than waiting until nbd_negotiate_options() completes. This will make it easier to implement NBD_OPT_GO. Pass additional parameters around, rather than stashing things inside NBDClient. Signed-off-by: Eric Blake --- v5: new patch --- nbd/server.c | 50 ++++++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 8e363d3..841986d 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -84,7 +84,6 @@ struct NBDClient { int refcount; void (*close_fn)(NBDClient *client, bool negotiated); - bool no_zeroes; NBDExport *exp; QCryptoTLSCreds *tlscreds; char *tlsaclname; @@ -277,9 +276,13 @@ static int nbd_negotiate_handle_list(NBDClient *client= , uint32_t length, } static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t le= ngth, + uint16_t myflags, bool no_zero= es, Error **errp) { char name[NBD_MAX_NAME_SIZE + 1]; + char buf[8 + 4 + 124] =3D ""; + size_t len; + int ret; /* Client sends: [20 .. xx] export name (length bytes) @@ -303,6 +306,17 @@ static int nbd_negotiate_handle_export_name(NBDClient = *client, uint32_t length, return -EINVAL; } + trace_nbd_negotiate_new_style_size_flags(client->exp->size, + client->exp->nbdflags | myfla= gs); + stq_be_p(buf, client->exp->size); + stw_be_p(buf + 8, client->exp->nbdflags | myflags); + len =3D no_zeroes ? 10 : sizeof(buf); + ret =3D nbd_write(client->ioc, buf, len, errp); + if (ret < 0) { + error_prepend(errp, "write failed: "); + return ret; + } + QTAILQ_INSERT_TAIL(&client->exp->clients, client, next); nbd_export_get(client->exp); @@ -373,14 +387,17 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDC= lient *client, * 1 if client sent NBD_OPT_ABORT, i.e. on valid disconnect, * errp is not set */ -static int nbd_negotiate_options(NBDClient *client, Error **errp) +static int nbd_negotiate_options(NBDClient *client, uint16_t myflags, + Error **errp) { uint32_t flags; bool fixedNewstyle =3D false; + bool no_zeroes =3D false; /* Client sends: [ 0 .. 3] client flags + Then we loop until NBD_OPT_EXPORT_NAME: [ 0 .. 7] NBD_OPTS_MAGIC [ 8 .. 11] NBD option [12 .. 15] Data length @@ -403,7 +420,7 @@ static int nbd_negotiate_options(NBDClient *client, Err= or **errp) flags &=3D ~NBD_FLAG_C_FIXED_NEWSTYLE; } if (flags & NBD_FLAG_C_NO_ZEROES) { - client->no_zeroes =3D true; + no_zeroes =3D true; flags &=3D ~NBD_FLAG_C_NO_ZEROES; } if (flags !=3D 0) { @@ -503,7 +520,9 @@ static int nbd_negotiate_options(NBDClient *client, Err= or **errp) return 1; case NBD_OPT_EXPORT_NAME: - return nbd_negotiate_handle_export_name(client, length, er= rp); + return nbd_negotiate_handle_export_name(client, length, + myflags, no_zeroes, + errp); case NBD_OPT_STARTTLS: if (nbd_drop(client->ioc, length, errp) < 0) { @@ -546,7 +565,9 @@ static int nbd_negotiate_options(NBDClient *client, Err= or **errp) */ switch (option) { case NBD_OPT_EXPORT_NAME: - return nbd_negotiate_handle_export_name(client, length, er= rp); + return nbd_negotiate_handle_export_name(client, length, + myflags, no_zeroes, + errp); default: error_setg(errp, "Unsupported option 0x%" PRIx32 " (%s)", @@ -572,7 +593,6 @@ static coroutine_fn int nbd_negotiate(NBDClient *client= , Error **errp) NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA | NBD_FLAG_SEND_WRITE_ZEROES); bool oldStyle; - size_t len; /* Old style negotiation header without options [ 0 .. 7] passwd ("NBDMAGIC") @@ -586,10 +606,7 @@ static coroutine_fn int nbd_negotiate(NBDClient *clien= t, Error **errp) [ 0 .. 7] passwd ("NBDMAGIC") [ 8 .. 15] magic (NBD_OPTS_MAGIC) [16 .. 17] server flags (0) - ....options sent.... - [18 .. 25] size - [26 .. 27] export flags - [28 .. 151] reserved (0, omit if no_zeroes) + ....options sent, ending in NBD_OPT_EXPORT_NAME.... */ qio_channel_set_blocking(client->ioc, false, NULL); @@ -618,24 +635,13 @@ static coroutine_fn int nbd_negotiate(NBDClient *clie= nt, Error **errp) error_prepend(errp, "write failed: "); return -EINVAL; } - ret =3D nbd_negotiate_options(client, errp); + ret =3D nbd_negotiate_options(client, myflags, errp); if (ret !=3D 0) { if (ret < 0) { error_prepend(errp, "option negotiation failed: "); } return ret; } - - trace_nbd_negotiate_new_style_size_flags( - client->exp->size, client->exp->nbdflags | myflags); - stq_be_p(buf + 18, client->exp->size); - stw_be_p(buf + 26, client->exp->nbdflags | myflags); - len =3D client->no_zeroes ? 10 : sizeof(buf) - 18; - ret =3D nbd_write(client->ioc, buf + 18, len, errp); - if (ret < 0) { - error_prepend(errp, "write failed: "); - return ret; - } } trace_nbd_negotiate_success(); --=20 2.9.4 From nobody Fri May 17 23:49:27 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 149945986535043.126232896565966; Fri, 7 Jul 2017 13:37:45 -0700 (PDT) Received: from localhost ([::1]:58727 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTa0V-0000AK-Uv for importer@patchew.org; Fri, 07 Jul 2017 16:37:44 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46831) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZuL-00034m-Vc for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dTZuK-00042Q-9l for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55980) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dTZuE-0003wh-94; Fri, 07 Jul 2017 16:31:14 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3B9A380C1A; Fri, 7 Jul 2017 20:31:13 +0000 (UTC) Received: from red.redhat.com (ovpn-120-204.rdu2.redhat.com [10.10.120.204]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5A28E71C55; Fri, 7 Jul 2017 20:31:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 3B9A380C1A Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 3B9A380C1A From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 7 Jul 2017 15:30:46 -0500 Message-Id: <20170707203049.534-7-eblake@redhat.com> In-Reply-To: <20170707203049.534-1-eblake@redhat.com> References: <20170707203049.534-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Fri, 07 Jul 2017 20:31:13 +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] [PATCH v5 6/9] nbd: Implement NBD_OPT_GO on server 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: pbonzini@redhat.com, vsementsov@virtuozzo.com, den@virtuozzo.com, qemu-block@nongnu.org, marcandre.lureau@redhat.com 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" NBD_OPT_EXPORT_NAME is lousy: per the NBD protocol, any failure requires us to close the connection rather than report an error. Therefore, upstream NBD recently added NBD_OPT_GO as the improved version of the option that does what we want [1], along with NBD_OPT_INFO that returns the same information but does not transition to transmission phase. [1] https://github.com/NetworkBlockDevice/nbd/blob/extension-info/doc/proto= .md This is a first cut at the information types, and only passes the same information already available through NBD_OPT_LIST and NBD_OPT_EXPORT_NAME; items like NBD_INFO_BLOCK_SIZE (and thus any use of NBD_REP_ERR_BLOCK_SIZE_REQD) are intentionally left for later patches. Signed-off-by: Eric Blake --- v5: update to master, R-b dropped v4: revamp to another round of NBD protocol changes v3: revamp to match latest version of NBD protocol --- nbd/server.c | 179 +++++++++++++++++++++++++++++++++++++++++++++++++++= +++- nbd/trace-events | 3 + 2 files changed, 179 insertions(+), 3 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 841986d..e84d012 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -141,6 +141,7 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc, = uint32_t type, trace_nbd_negotiate_send_rep_len(opt, nbd_opt_lookup(opt), type, nbd_rep_lookup(type), len); + assert(len < NBD_MAX_BUFFER_SIZE); magic =3D cpu_to_be64(NBD_REP_MAGIC); if (nbd_write(ioc, &magic, sizeof(magic), errp) < 0) { error_prepend(errp, "write failed (rep magic): "); @@ -275,6 +276,8 @@ static int nbd_negotiate_handle_list(NBDClient *client,= uint32_t length, return nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, NBD_OPT_LIST, = errp); } +/* Send a reply to NBD_OPT_EXPORT_NAME. + * Return -errno on error, 0 on success. */ static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t le= ngth, uint16_t myflags, bool no_zero= es, Error **errp) @@ -323,6 +326,162 @@ static int nbd_negotiate_handle_export_name(NBDClient= *client, uint32_t length, return 0; } +/* Send a single NBD_REP_INFO, with a buffer @buf of @length bytes. + * The buffer does NOT include the info type prefix. + * Return -errno on error, 0 if ready to send more. */ +static int nbd_negotiate_send_info(NBDClient *client, uint32_t opt, + uint16_t info, uint32_t length, void *b= uf, + Error **errp) +{ + int rc; + + trace_nbd_negotiate_send_info(info, nbd_info_lookup(info), length); + rc =3D nbd_negotiate_send_rep_len(client->ioc, NBD_REP_INFO, opt, + sizeof(info) + length, errp); + if (rc < 0) { + return rc; + } + cpu_to_be16s(&info); + if (nbd_write(client->ioc, &info, sizeof(info), errp) < 0) { + return -EIO; + } + if (nbd_write(client->ioc, buf, length, errp) < 0) { + return -EIO; + } + return 0; +} + +/* Handle NBD_OPT_INFO and NBD_OPT_GO. + * Return -errno on error, 0 if ready for next option, and 1 to move + * into transmission phase. */ +static int nbd_negotiate_handle_info(NBDClient *client, uint32_t length, + uint32_t opt, uint16_t myflags, + Error **errp) +{ + int rc; + char name[NBD_MAX_NAME_SIZE + 1]; + NBDExport *exp; + uint16_t requests; + uint16_t request; + uint32_t namelen; + bool sendname =3D false; + char buf[sizeof(uint64_t) + sizeof(uint16_t)]; + const char *msg; + + /* Client sends: + 4 bytes: L, name length (can be 0) + L bytes: export name + 2 bytes: N, number of requests (can be 0) + N * 2 bytes: N requests + */ + if (length < sizeof(namelen) + sizeof(requests)) { + msg =3D "overall request too short"; + goto invalid; + } + if (nbd_read(client->ioc, &namelen, sizeof(namelen), errp) < 0) { + return -EIO; + } + be32_to_cpus(&namelen); + length -=3D sizeof(namelen); + if (namelen > length - sizeof(requests) || (length - namelen) % 2) { + msg =3D "name length is incorrect"; + goto invalid; + } + if (nbd_read(client->ioc, name, namelen, errp) < 0) { + return -EIO; + } + name[namelen] =3D '\0'; + length -=3D namelen; + trace_nbd_negotiate_handle_export_name_request(name); + + if (nbd_read(client->ioc, &requests, sizeof(requests), errp) < 0) { + return -EIO; + } + be16_to_cpus(&requests); + length -=3D sizeof(requests); + trace_nbd_negotiate_handle_info_requests(requests); + if (requests !=3D length / sizeof(request)) { + msg =3D "incorrect number of requests for overall length"; + goto invalid; + } + while (requests--) { + if (nbd_read(client->ioc, &request, sizeof(request), errp) < 0) { + return -EIO; + } + be16_to_cpus(&request); + length -=3D sizeof(request); + trace_nbd_negotiate_handle_info_request(request, + nbd_info_lookup(request)); + /* For now, we only care about NBD_INFO_NAME; everything else + * is either a request we don't know or something we send + * regardless of request. */ + if (request =3D=3D NBD_INFO_NAME) { + sendname =3D true; + } + } + + exp =3D nbd_export_find(name); + if (!exp) { + return nbd_negotiate_send_rep_err(client->ioc, NBD_REP_ERR_UNKNOWN, + opt, errp, "export '%s' not pres= ent", + name); + } + + /* Don't bother sending NBD_INFO_NAME unless client requested it */ + if (sendname) { + rc =3D nbd_negotiate_send_info(client, opt, NBD_INFO_NAME, length,= name, + errp); + if (rc < 0) { + return rc; + } + } + + /* Send NBD_INFO_DESCRIPTION only if available, regardless of + * client request */ + if (exp->description) { + size_t len =3D strlen(exp->description); + + rc =3D nbd_negotiate_send_info(client, opt, NBD_INFO_DESCRIPTION, + len, exp->description, errp); + if (rc < 0) { + return rc; + } + } + + /* Send NBD_INFO_EXPORT always */ + trace_nbd_negotiate_new_style_size_flags(exp->size, + exp->nbdflags | myflags); + stq_be_p(buf, exp->size); + stw_be_p(buf + 8, exp->nbdflags | myflags); + rc =3D nbd_negotiate_send_info(client, opt, NBD_INFO_EXPORT, + sizeof(buf), buf, errp); + if (rc < 0) { + return rc; + } + + /* Final reply */ + rc =3D nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, opt, errp); + if (rc < 0) { + return rc; + } + + if (opt =3D=3D NBD_OPT_GO) { + client->exp =3D exp; + QTAILQ_INSERT_TAIL(&client->exp->clients, client, next); + nbd_export_get(client->exp); + rc =3D 1; + } + return rc; + + invalid: + if (nbd_drop(client->ioc, length, errp) < 0) { + return -EIO; + } + return nbd_negotiate_send_rep_err(client->ioc, NBD_REP_ERR_INVALID, op= t, + errp, "%s", msg); +} + + /* Handle NBD_OPT_STARTTLS. Return NULL to drop connection, or else the * new channel for all further (now-encrypted) communication. */ static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client, @@ -380,7 +539,8 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDCli= ent *client, } /* nbd_negotiate_options - * Process all NBD_OPT_* client option commands. + * Process all NBD_OPT_* client option commands, during fixed newstyle + * negotiation. * Return: * -errno on error, errp is set * 0 on successful negotiation, errp is not set @@ -397,7 +557,7 @@ static int nbd_negotiate_options(NBDClient *client, uin= t16_t myflags, /* Client sends: [ 0 .. 3] client flags - Then we loop until NBD_OPT_EXPORT_NAME: + Then we loop until NBD_OPT_EXPORT_NAME or NBD_OPT_GO: [ 0 .. 7] NBD_OPTS_MAGIC [ 8 .. 11] NBD option [12 .. 15] Data length @@ -524,6 +684,19 @@ static int nbd_negotiate_options(NBDClient *client, ui= nt16_t myflags, myflags, no_zeroes, errp); + case NBD_OPT_INFO: + case NBD_OPT_GO: + ret =3D nbd_negotiate_handle_info(client, length, option, + myflags, errp); + if (ret =3D=3D 1) { + assert(option =3D=3D NBD_OPT_GO); + return 0; + } + if (ret) { + return ret; + } + break; + case NBD_OPT_STARTTLS: if (nbd_drop(client->ioc, length, errp) < 0) { return -EIO; @@ -606,7 +779,7 @@ static coroutine_fn int nbd_negotiate(NBDClient *client= , Error **errp) [ 0 .. 7] passwd ("NBDMAGIC") [ 8 .. 15] magic (NBD_OPTS_MAGIC) [16 .. 17] server flags (0) - ....options sent, ending in NBD_OPT_EXPORT_NAME.... + ....options sent, ending in NBD_OPT_EXPORT_NAME or NBD_OPT_GO.... */ qio_channel_set_blocking(client->ioc, false, NULL); diff --git a/nbd/trace-events b/nbd/trace-events index 765f5f4..80c2447 100644 --- a/nbd/trace-events +++ b/nbd/trace-events @@ -33,6 +33,9 @@ nbd_negotiate_send_rep_err(const char *msg) "sending erro= r message \"%s\"" nbd_negotiate_send_rep_list(const char *name, const char *desc) "Advertisi= ng export name '%s' description '%s'" nbd_negotiate_handle_export_name(void) "Checking length" nbd_negotiate_handle_export_name_request(const char *name) "Client request= ed export '%s'" +nbd_negotiate_send_info(int info, const char *name, uint32_t length) "Send= ing NBD_REP_INFO type %d (%s) with remaining length %" PRIu32 +nbd_negotiate_handle_info_requests(int requests) "Client requested %d item= s of info" +nbd_negotiate_handle_info_request(int request, const char *name) "Client r= equested info %d (%s)" nbd_negotiate_handle_starttls(void) "Setting up TLS" nbd_negotiate_handle_starttls_handshake(void) "Starting TLS handshake" nbd_negotiate_options_flags(uint32_t flags) "Received client flags 0x%" PR= Ix32 --=20 2.9.4 From nobody Fri May 17 23:49:27 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 1499459729160832.4770246909609; Fri, 7 Jul 2017 13:35:29 -0700 (PDT) Received: from localhost ([::1]:58708 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZyJ-0006ZQ-Fs for importer@patchew.org; Fri, 07 Jul 2017 16:35:27 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46830) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZuL-00034l-V5 for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dTZuK-00042O-9f for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60752) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dTZuF-0003yv-Dx; Fri, 07 Jul 2017 16:31:15 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 651DAC057FA5; Fri, 7 Jul 2017 20:31:14 +0000 (UTC) Received: from red.redhat.com (ovpn-120-204.rdu2.redhat.com [10.10.120.204]) by smtp.corp.redhat.com (Postfix) with ESMTP id 72D2B729C3; Fri, 7 Jul 2017 20:31:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 651DAC057FA5 Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx08.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 651DAC057FA5 From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 7 Jul 2017 15:30:47 -0500 Message-Id: <20170707203049.534-8-eblake@redhat.com> In-Reply-To: <20170707203049.534-1-eblake@redhat.com> References: <20170707203049.534-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 07 Jul 2017 20:31:14 +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] [PATCH v5 7/9] nbd: Implement NBD_OPT_GO on client 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: pbonzini@redhat.com, vsementsov@virtuozzo.com, den@virtuozzo.com, qemu-block@nongnu.org, marcandre.lureau@redhat.com 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" NBD_OPT_EXPORT_NAME is lousy: per the NBD protocol, any failure requires the server to close the connection rather than report an error to us. Therefore, upstream NBD recently added NBD_OPT_GO as the improved version of the option that does what we want [1]: it reports sane errors on failures, and on success provides at least as much info as NBD_OPT_EXPORT_NAME. [1] https://github.com/NetworkBlockDevice/nbd/blob/extension-info/doc/proto= .md This is a first cut at use of the information types. Note that we do not need to use NBD_OPT_INFO, and that use of NBD_OPT_GO means we no longer have to use NBD_OPT_LIST to learn whether a server requires TLS (this requires servers that gracefully handle unknown NBD_OPT, many servers prior to qemu 2.5 were buggy, but I have patched qemu, upstream nbd, and nbdkit in the meantime, in part because of interoperability testing with this patch). We still fall back to NBD_OPT_LIST when NBD_OPT_GO is not supported on the server, as it is still one last chance for a nicer error message. Later patches will use further info, like NBD_INFO_BLOCK_SIZE. Signed-off-by: Eric Blake --- v5: rebase to master; enough changes to drop R-b v4: NBD protocol changes, again v3: revamp to match latest version of NBD protocol --- nbd/nbd-internal.h | 3 ++ nbd/client.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++= +++- nbd/trace-events | 3 ++ 3 files changed, 130 insertions(+), 2 deletions(-) diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h index bf95601..4065bc6 100644 --- a/nbd/nbd-internal.h +++ b/nbd/nbd-internal.h @@ -37,8 +37,11 @@ * https://github.com/yoe/nbd/blob/master/doc/proto.md */ +/* Size of all NBD_OPT_*, without payload */ #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_REQUEST_MAGIC 0x25609513 #define NBD_REPLY_MAGIC 0x67446698 #define NBD_OPTS_MAGIC 0x49484156454F5054LL diff --git a/nbd/client.c b/nbd/client.c index 011960b..cb55f3d 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -350,6 +350,114 @@ static int nbd_receive_list(QIOChannel *ioc, const ch= ar *want, bool *match, } +/* Returns -1 if NBD_OPT_GO proves the export @wantname cannot be + * used, 0 if NBD_OPT_GO is unsupported (fall back to NBD_OPT_LIST and + * NBD_OPT_EXPORT_NAME in that case), and > 0 if the export is good to + * go (with @info populated). */ +static int nbd_opt_go(QIOChannel *ioc, const char *wantname, + NBDExportInfo *info, Error **errp) +{ + nbd_opt_reply reply; + uint32_t len =3D strlen(wantname); + uint16_t type; + int error; + char *buf; + + /* The protocol requires that the server send NBD_INFO_EXPORT with + * a non-zero flags (at least NBD_FLAG_HAS_FLAGS must be set); so + * flags still 0 is a witness of a broken server. */ + info->flags =3D 0; + + trace_nbd_opt_go_start(wantname); + buf =3D g_malloc(4 + len + 2 + 1); + stl_be_p(buf, len); + memcpy(buf + 4, wantname, len); + /* No requests, live with whatever server sends */ + stw_be_p(buf + 4 + len, 0); + if (nbd_send_option_request(ioc, NBD_OPT_GO, len + 6, buf, errp) < 0) { + return -1; + } + + while (1) { + if (nbd_receive_option_reply(ioc, NBD_OPT_GO, &reply, errp) < 0) { + return -1; + } + error =3D nbd_handle_reply_err(ioc, &reply, errp); + if (error <=3D 0) { + return error; + } + len =3D reply.length; + + if (reply.type =3D=3D NBD_REP_ACK) { + /* Server is done sending info and moved into transmission + phase, but make sure it sent flags */ + if (len) { + error_setg(errp, "server sent invalid NBD_REP_ACK"); + nbd_send_opt_abort(ioc); + return -1; + } + if (!info->flags) { + error_setg(errp, "broken server omitted NBD_INFO_EXPORT"); + nbd_send_opt_abort(ioc); + return -1; + } + trace_nbd_opt_go_success(); + return 1; + } + if (reply.type !=3D NBD_REP_INFO) { + error_setg(errp, "unexpected reply type %" PRIx32 ", expected = %x", + reply.type, NBD_REP_INFO); + nbd_send_opt_abort(ioc); + return -1; + } + if (len < sizeof(type)) { + error_setg(errp, "NBD_REP_INFO length %" PRIu32 " is too short= ", + len); + nbd_send_opt_abort(ioc); + return -1; + } + if (nbd_read(ioc, &type, sizeof(type), errp) < 0) { + error_prepend(errp, "failed to read info type"); + nbd_send_opt_abort(ioc); + return -1; + } + len -=3D sizeof(type); + be16_to_cpus(&type); + switch (type) { + case NBD_INFO_EXPORT: + if (len !=3D sizeof(info->size) + sizeof(info->flags)) { + error_setg(errp, "remaining export info len %" PRIu32 + " is unexpected size", len); + nbd_send_opt_abort(ioc); + return -1; + } + if (nbd_read(ioc, &info->size, sizeof(info->size), errp) < 0) { + error_prepend(errp, "failed to read info size"); + nbd_send_opt_abort(ioc); + return -1; + } + be64_to_cpus(&info->size); + if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0= ) { + error_prepend(errp, "failed to read info flags"); + nbd_send_opt_abort(ioc); + return -1; + } + be16_to_cpus(&info->flags); + trace_nbd_receive_negotiate_size_flags(info->size, info->flags= ); + break; + + default: + trace_nbd_opt_go_info_unknown(type, nbd_info_lookup(type)); + if (nbd_drop(ioc, len, errp) < 0) { + error_prepend(errp, "Failed to read info payload"); + nbd_send_opt_abort(ioc); + return -1; + } + break; + } + } +} + /* Return -1 on failure, 0 if wantname is an available export. */ static int nbd_receive_query_exports(QIOChannel *ioc, const char *wantname, @@ -531,11 +639,25 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char= *name, name =3D ""; } if (fixedNewStyle) { + int result; + + /* Try NBD_OPT_GO first - if it works, we are done (it + * also gives us a good message if the server requires + * TLS). If it is not available, fall back to + * NBD_OPT_LIST for nicer error messages about a missing + * export, then use NBD_OPT_EXPORT_NAME. */ + result =3D nbd_opt_go(ioc, name, info, errp); + if (result < 0) { + goto fail; + } + if (result > 0) { + return 0; + } /* Check our desired export is present in the * server export list. Since NBD_OPT_EXPORT_NAME * cannot return an error message, running this - * query gives us good error reporting if the - * server required TLS + * query gives us better error reporting if the + * export name is not available. */ if (nbd_receive_query_exports(ioc, name, errp) < 0) { goto fail; diff --git a/nbd/trace-events b/nbd/trace-events index 80c2447..4969047 100644 --- a/nbd/trace-events +++ b/nbd/trace-events @@ -3,6 +3,9 @@ nbd_unknown_error(int err) "Squashing unexpected error %d t= o EINVAL" nbd_send_option_request(uint32_t opt, const char *name, uint32_t len) "Sen= ding option request %" PRIu32" (%s), len %" PRIu32 nbd_receive_option_reply(uint32_t option, const char *optname, uint32_t ty= pe, const char *typename, uint32_t length) "Received option reply %" PRIx32= " (%s), type %" PRIx32" (%s), len %" PRIu32 nbd_reply_err_unsup(uint32_t option, const char *name) "server doesn't und= erstand request %" PRIx32 " (%s), attempting fallback" +nbd_opt_go_start(const char *name) "Attempting NBD_OPT_GO for export '%s'" +nbd_opt_go_success(void) "Export is good to go" +nbd_opt_go_info_unknown(int info, const char *name) "Ignoring unknown info= %d (%s)" nbd_receive_query_exports_start(const char *wantname) "Querying export lis= t for '%s'" nbd_receive_query_exports_success(const char *wantname) "Found desired exp= ort name '%s'" nbd_receive_starttls_request(void) "Requesting TLS from server" --=20 2.9.4 From nobody Fri May 17 23:49:27 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 1499459752904107.01575196298882; Fri, 7 Jul 2017 13:35:52 -0700 (PDT) Received: from localhost ([::1]:58716 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZyg-0006xp-D3 for importer@patchew.org; Fri, 07 Jul 2017 16:35:50 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46842) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZuM-000364-Sy for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dTZuL-00043H-Ic for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40519) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dTZuG-0003zh-KD; Fri, 07 Jul 2017 16:31:16 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9176619D385; Fri, 7 Jul 2017 20:31:15 +0000 (UTC) Received: from red.redhat.com (ovpn-120-204.rdu2.redhat.com [10.10.120.204]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9F4A371C55; Fri, 7 Jul 2017 20:31:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 9176619D385 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 9176619D385 From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 7 Jul 2017 15:30:48 -0500 Message-Id: <20170707203049.534-9-eblake@redhat.com> In-Reply-To: <20170707203049.534-1-eblake@redhat.com> References: <20170707203049.534-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Fri, 07 Jul 2017 20:31:15 +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] [PATCH v5 8/9] nbd: Implement NBD_INFO_BLOCK_SIZE on server 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: pbonzini@redhat.com, vsementsov@virtuozzo.com, den@virtuozzo.com, qemu-block@nongnu.org, marcandre.lureau@redhat.com 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" The upstream NBD Protocol has defined a new extension to allow the server to advertise block sizes to the client, as well as a way for the client to inform the server that it intends to obey block sizes. Thanks to a recent fix (commit df7b97ff), our real minimum transfer size is always 1 (the block layer takes care of read-modify-write on our behalf), but we're still more efficient if we advertise 512 when the client supports it, as follows: - OPT_INFO, but no NBD_INFO_BLOCK_SIZE: advertise 512, then fail with NBD_REP_ERR_BLOCK_SIZE_REQD; client is free to try something else since we don't disconnect - OPT_INFO with NBD_INFO_BLOCK_SIZE: advertise 512 - OPT_GO, but no NBD_INFO_BLOCK_SIZE: advertise 1 - OPT_GO with NBD_INFO_BLOCK_SIZE: advertise 512 We can also advertise the optimum block size (presumably the cluster size, when exporting a qcow2 file), and our absolute maximum transfer size of 32M, to help newer clients avoid EINVAL failures or abrupt disconnects on oversize requests. We do not reject clients for using the older NBD_OPT_EXPORT_NAME; we are no worse off for those clients than we used to be. Signed-off-by: Eric Blake --- v5: rebase to master v4: new patch --- nbd/server.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- nbd/trace-events | 1 + 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index e84d012..49ed574 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -365,6 +365,8 @@ static int nbd_negotiate_handle_info(NBDClient *client,= uint32_t length, uint16_t request; uint32_t namelen; bool sendname =3D false; + bool blocksize =3D false; + uint32_t sizes[3]; char buf[sizeof(uint64_t) + sizeof(uint16_t)]; const char *msg; @@ -412,11 +414,16 @@ static int nbd_negotiate_handle_info(NBDClient *clien= t, uint32_t length, length -=3D sizeof(request); trace_nbd_negotiate_handle_info_request(request, nbd_info_lookup(request)); - /* For now, we only care about NBD_INFO_NAME; everything else - * is either a request we don't know or something we send - * regardless of request. */ - if (request =3D=3D NBD_INFO_NAME) { + /* We care about NBD_INFO_NAME and NBD_INFO_BLOCK_SIZE; + * everything else is either a request we don't know or + * something we send regardless of request */ + switch (request) { + case NBD_INFO_NAME: sendname =3D true; + break; + case NBD_INFO_BLOCK_SIZE: + blocksize =3D true; + break; } } @@ -448,6 +455,27 @@ static int nbd_negotiate_handle_info(NBDClient *client= , uint32_t length, } } + /* Send NBD_INFO_BLOCK_SIZE always, but tweak the minimum size + * according to whether the client requested it, and according to + * whether this is OPT_INFO or OPT_GO. */ + /* minimum - 1 for back-compat, or 512 if client is new enough. + * TODO: consult blk_bs(blk)->bl.request_alignment? */ + sizes[0] =3D (opt =3D=3D NBD_OPT_INFO || blocksize) ? BDRV_SECTOR_SIZE= : 1; + /* preferred - Hard-code to 4096 for now. + * TODO: is blk_bs(blk)->bl.opt_transfer appropriate? */ + sizes[1] =3D 4096; + /* maximum - At most 32M, but smaller as appropriate. */ + sizes[2] =3D MIN(blk_get_max_transfer(exp->blk), NBD_MAX_BUFFER_SIZE); + trace_nbd_negotiate_handle_info_block_size(sizes[0], sizes[1], sizes[2= ]); + cpu_to_be32s(&sizes[0]); + cpu_to_be32s(&sizes[1]); + cpu_to_be32s(&sizes[2]); + rc =3D nbd_negotiate_send_info(client, opt, NBD_INFO_BLOCK_SIZE, + sizeof(sizes), sizes, errp); + if (rc < 0) { + return rc; + } + /* Send NBD_INFO_EXPORT always */ trace_nbd_negotiate_new_style_size_flags(exp->size, exp->nbdflags | myflags); @@ -459,6 +487,18 @@ static int nbd_negotiate_handle_info(NBDClient *client= , uint32_t length, return rc; } + /* If the client is just asking for NBD_OPT_INFO, but forgot to + * request block sizes, return an error. + * TODO: consult blk_bs(blk)->request_align, and only error if it + * is not 1? */ + if (opt =3D=3D NBD_OPT_INFO && !blocksize) { + return nbd_negotiate_send_rep_err(client->ioc, + NBD_REP_ERR_BLOCK_SIZE_REQD, opt, + errp, + "request NBD_INFO_BLOCK_SIZE to " + "use this export"); + } + /* Final reply */ rc =3D nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, opt, errp); if (rc < 0) { diff --git a/nbd/trace-events b/nbd/trace-events index 4969047..a3ba4bc 100644 --- a/nbd/trace-events +++ b/nbd/trace-events @@ -39,6 +39,7 @@ nbd_negotiate_handle_export_name_request(const char *name= ) "Client requested exp nbd_negotiate_send_info(int info, const char *name, uint32_t length) "Send= ing NBD_REP_INFO type %d (%s) with remaining length %" PRIu32 nbd_negotiate_handle_info_requests(int requests) "Client requested %d item= s of info" nbd_negotiate_handle_info_request(int request, const char *name) "Client r= equested info %d (%s)" +nbd_negotiate_handle_info_block_size(uint32_t minimum, uint32_t preferred,= uint32_t maximum) "advertising minimum 0x%" PRIx32 ", preferred 0x%" PRIx3= 2 ", maximum 0x%" PRIx32 nbd_negotiate_handle_starttls(void) "Setting up TLS" nbd_negotiate_handle_starttls_handshake(void) "Starting TLS handshake" nbd_negotiate_options_flags(uint32_t flags) "Received client flags 0x%" PR= Ix32 --=20 2.9.4 From nobody Fri May 17 23:49:27 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 1499459861520965.0864146777401; Fri, 7 Jul 2017 13:37:41 -0700 (PDT) Received: from localhost ([::1]:58725 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTa0S-00007R-6P for importer@patchew.org; Fri, 07 Jul 2017 16:37:40 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46858) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dTZuN-00036c-FO for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dTZuL-00043b-Uu for qemu-devel@nongnu.org; Fri, 07 Jul 2017 16:31:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33910) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dTZuI-00040g-0d; Fri, 07 Jul 2017 16:31:18 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 068F7C049E16; Fri, 7 Jul 2017 20:31:17 +0000 (UTC) Received: from red.redhat.com (ovpn-120-204.rdu2.redhat.com [10.10.120.204]) by smtp.corp.redhat.com (Postfix) with ESMTP id CF98D71C55; Fri, 7 Jul 2017 20:31:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 068F7C049E16 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=eblake@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 068F7C049E16 From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 7 Jul 2017 15:30:49 -0500 Message-Id: <20170707203049.534-10-eblake@redhat.com> In-Reply-To: <20170707203049.534-1-eblake@redhat.com> References: <20170707203049.534-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 07 Jul 2017 20:31:17 +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] [PATCH v5 9/9] nbd: Implement NBD_INFO_BLOCK_SIZE on client 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 , vsementsov@virtuozzo.com, den@virtuozzo.com, qemu-block@nongnu.org, Max Reitz , marcandre.lureau@redhat.com, pbonzini@redhat.com 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" The upstream NBD Protocol has defined a new extension to allow the server to advertise block sizes to the client, as well as a way for the client to inform the server whether it intends to obey block sizes. When using the block layer as the client, we will obey block sizes; but when used as 'qemu-nbd -c' to hand off to the kernel nbd module as the client, we are still waiting for the kernel to implement a way for us to learn if it will honor block sizes (perhaps by an addition to sysfs, rather than an ioctl), as well as any way to tell the kernel what additional block sizes to obey (NBD_SET_BLKSIZE appears to be accurate for the minimum size, but preferred and maximum sizes would probably be new ioctl()s), so until then, we need to make our request for block sizes conditional. When using ioctl(NBD_SET_BLKSIZE) to hand off to the kernel, use the minimum block size as the sector size if it is larger than 512, which also has the nice effect of cooperating with (non-qemu) servers that don't do read-modify-write when exposing a block device with 4k sectors; it might also allow us to visit a file larger than 2T on a 32-bit kernel. Signed-off-by: Eric Blake --- v5: rebase to master v4: new patch --- include/block/nbd.h | 6 ++++ block/nbd-client.c | 4 +++ block/nbd.c | 14 +++++++-- nbd/client.c | 81 +++++++++++++++++++++++++++++++++++++++++++++----= ---- qemu-nbd.c | 2 +- nbd/trace-events | 1 + 6 files changed, 92 insertions(+), 16 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index 4a22eca..9c3d0a5 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -144,8 +144,14 @@ enum { /* Details collected by NBD_OPT_EXPORT_NAME and NBD_OPT_GO */ struct NBDExportInfo { + /* Set by client before nbd_receive_negotiate() */ + bool request_sizes; + /* Set by server results during nbd_receive_negotiate() */ uint64_t size; uint16_t flags; + uint32_t min_block; + uint32_t opt_block; + uint32_t max_block; }; typedef struct NBDExportInfo NBDExportInfo; diff --git a/block/nbd-client.c b/block/nbd-client.c index aab1e32..25dd284 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -384,6 +384,7 @@ int nbd_client_init(BlockDriverState *bs, logout("session init %s\n", export); qio_channel_set_blocking(QIO_CHANNEL(sioc), true, NULL); + client->info.request_sizes =3D true; ret =3D nbd_receive_negotiate(QIO_CHANNEL(sioc), export, tlscreds, hostname, &client->ioc, &client->info, errp); @@ -398,6 +399,9 @@ int nbd_client_init(BlockDriverState *bs, if (client->info.flags & NBD_FLAG_SEND_WRITE_ZEROES) { bs->supported_zero_flags |=3D BDRV_REQ_MAY_UNMAP; } + if (client->info.min_block > bs->bl.request_alignment) { + bs->bl.request_alignment =3D client->info.min_block; + } qemu_co_mutex_init(&client->send_mutex); qemu_co_queue_init(&client->free_sema); diff --git a/block/nbd.c b/block/nbd.c index 4a9048c..a50d24b 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -472,9 +472,17 @@ static int nbd_co_flush(BlockDriverState *bs) static void nbd_refresh_limits(BlockDriverState *bs, Error **errp) { - bs->bl.max_pdiscard =3D NBD_MAX_BUFFER_SIZE; - bs->bl.max_pwrite_zeroes =3D NBD_MAX_BUFFER_SIZE; - bs->bl.max_transfer =3D NBD_MAX_BUFFER_SIZE; + NBDClientSession *s =3D nbd_get_client_session(bs); + uint32_t max =3D MIN_NON_ZERO(NBD_MAX_BUFFER_SIZE, s->info.max_block); + + bs->bl.max_pdiscard =3D max; + bs->bl.max_pwrite_zeroes =3D max; + bs->bl.max_transfer =3D max; + + if (s->info.opt_block && + s->info.opt_block > bs->bl.opt_transfer) { + bs->bl.opt_transfer =3D s->info.opt_block; + } } static void nbd_close(BlockDriverState *bs) diff --git a/nbd/client.c b/nbd/client.c index cb55f3d..af2b46d 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -369,12 +369,17 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wa= ntname, info->flags =3D 0; trace_nbd_opt_go_start(wantname); - buf =3D g_malloc(4 + len + 2 + 1); + buf =3D g_malloc(4 + len + 2 + 2 * info->request_sizes + 1); stl_be_p(buf, len); memcpy(buf + 4, wantname, len); - /* No requests, live with whatever server sends */ - stw_be_p(buf + 4 + len, 0); - if (nbd_send_option_request(ioc, NBD_OPT_GO, len + 6, buf, errp) < 0) { + /* At most one request, everything else up to server */ + stw_be_p(buf + 4 + len, info->request_sizes); + if (info->request_sizes) { + stw_be_p(buf + 4 + len + 2, NBD_INFO_BLOCK_SIZE); + } + if (nbd_send_option_request(ioc, NBD_OPT_GO, + 4 + len + 2 + 2 * info->request_sizes, buf, + errp) < 0) { return -1; } @@ -405,8 +410,9 @@ static int nbd_opt_go(QIOChannel *ioc, const char *want= name, return 1; } if (reply.type !=3D NBD_REP_INFO) { - error_setg(errp, "unexpected reply type %" PRIx32 ", expected = %x", - reply.type, NBD_REP_INFO); + error_setg(errp, "unexpected reply type %" PRIx32 + " (%s), expected %x", + reply.type, nbd_rep_lookup(reply.type), NBD_REP_INF= O); nbd_send_opt_abort(ioc); return -1; } @@ -446,6 +452,51 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wan= tname, trace_nbd_receive_negotiate_size_flags(info->size, info->flags= ); break; + case NBD_INFO_BLOCK_SIZE: + if (len !=3D sizeof(info->min_block) * 3) { + error_setg(errp, "remaining export info len %" PRIu32 + " is unexpected size", len); + nbd_send_opt_abort(ioc); + return -1; + } + if (nbd_read(ioc, &info->min_block, sizeof(info->min_block), + errp) < 0) { + error_prepend(errp, "failed to read info minimum block siz= e"); + nbd_send_opt_abort(ioc); + return -1; + } + be32_to_cpus(&info->min_block); + if (!is_power_of_2(info->min_block)) { + error_setg(errp, "server minimum block size %" PRId32 + "is not a power of two", info->min_block); + nbd_send_opt_abort(ioc); + return -1; + } + if (nbd_read(ioc, &info->opt_block, sizeof(info->opt_block), + errp) < 0) { + error_prepend(errp, "failed to read info preferred block s= ize"); + nbd_send_opt_abort(ioc); + return -1; + } + be32_to_cpus(&info->opt_block); + if (!is_power_of_2(info->opt_block) || + info->opt_block < info->min_block) { + error_setg(errp, "server preferred block size %" PRId32 + "is not valid", info->opt_block); + nbd_send_opt_abort(ioc); + return -1; + } + if (nbd_read(ioc, &info->max_block, sizeof(info->max_block), + errp) < 0) { + error_prepend(errp, "failed to read info maximum block siz= e"); + nbd_send_opt_abort(ioc); + return -1; + } + be32_to_cpus(&info->max_block); + trace_nbd_opt_go_info_block_size(info->min_block, info->opt_bl= ock, + info->max_block); + break; + default: trace_nbd_opt_go_info_unknown(type, nbd_info_lookup(type)); if (nbd_drop(ioc, len, errp) < 0) { @@ -729,8 +780,14 @@ fail: int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info, Error **errp) { - unsigned long sectors =3D info->size / BDRV_SECTOR_SIZE; - if (info->size / BDRV_SECTOR_SIZE !=3D sectors) { + unsigned long sector_size =3D MAX(BDRV_SECTOR_SIZE, info->min_block); + unsigned long sectors =3D info->size / sector_size; + + /* FIXME: Once the kernel module is patched to honor block sizes, + * and to advertise that fact to user space, we should update the + * hand-off to the kernel to use any block sizes we learned. */ + assert(!info->request_sizes); + if (info->size / sector_size !=3D sectors) { error_setg(errp, "Export size %" PRId64 " too large for 32-bit ker= nel", info->size); return -E2BIG; @@ -744,17 +801,17 @@ int nbd_init(int fd, QIOChannelSocket *sioc, NBDExpor= tInfo *info, return -serrno; } - trace_nbd_init_set_block_size(BDRV_SECTOR_SIZE); + trace_nbd_init_set_block_size(sector_size); - if (ioctl(fd, NBD_SET_BLKSIZE, (unsigned long)BDRV_SECTOR_SIZE) < 0) { + if (ioctl(fd, NBD_SET_BLKSIZE, sector_size) < 0) { int serrno =3D errno; error_setg(errp, "Failed setting NBD block size"); return -serrno; } trace_nbd_init_set_size(sectors); - if (info->size % BDRV_SECTOR_SIZE) { - trace_nbd_init_trailing_bytes(info->size % BDRV_SECTOR_SIZE); + if (info->size % sector_size) { + trace_nbd_init_trailing_bytes(info->size % sector_size); } if (ioctl(fd, NBD_SET_SIZE_BLOCKS, sectors) < 0) { diff --git a/qemu-nbd.c b/qemu-nbd.c index c8bd47f..78d05be 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -255,7 +255,7 @@ static void *show_parts(void *arg) static void *nbd_client_thread(void *arg) { char *device =3D arg; - NBDExportInfo info; + NBDExportInfo info =3D { .request_sizes =3D false, }; QIOChannelSocket *sioc; int fd; int ret; diff --git a/nbd/trace-events b/nbd/trace-events index a3ba4bc..b18d051 100644 --- a/nbd/trace-events +++ b/nbd/trace-events @@ -6,6 +6,7 @@ nbd_reply_err_unsup(uint32_t option, const char *name) "ser= ver doesn't understan nbd_opt_go_start(const char *name) "Attempting NBD_OPT_GO for export '%s'" nbd_opt_go_success(void) "Export is good to go" nbd_opt_go_info_unknown(int info, const char *name) "Ignoring unknown info= %d (%s)" +nbd_opt_go_info_block_size(uint32_t minimum, uint32_t preferred, uint32_t = maximum) "Block sizes are 0x%" PRIx32 ", 0x%" PRIx32 ", 0x%" PRIx32 nbd_receive_query_exports_start(const char *wantname) "Querying export lis= t for '%s'" nbd_receive_query_exports_success(const char *wantname) "Found desired exp= ort name '%s'" nbd_receive_starttls_request(void) "Requesting TLS from server" --=20 2.9.4