From nobody Thu Apr 25 05:23:16 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.zoho.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 1487645382875347.76407360297424; Mon, 20 Feb 2017 18:49:42 -0800 (PST) Received: from localhost ([::1]:41910 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0WL-00020c-OB for importer@patchew.org; Mon, 20 Feb 2017 21:49:41 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40658) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0Pt-0005S7-Aa for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cg0Ps-0003Qd-IQ for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:01 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55790) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cg0Pq-0003PE-9o; Mon, 20 Feb 2017 21:42:58 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 653F43D957; Tue, 21 Feb 2017 02:42:58 +0000 (UTC) Received: from red.redhat.com (ovpn-123-67.rdu2.redhat.com [10.10.123.67] (may be forged)) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1L2gnuA023090; Mon, 20 Feb 2017 21:42:57 -0500 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 20 Feb 2017 20:42:41 -0600 Message-Id: <20170221024248.11027-2-eblake@redhat.com> In-Reply-To: <20170221024248.11027-1-eblake@redhat.com> References: <20170221024248.11027-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Tue, 21 Feb 2017 02:42:58 +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 v4 1/8] nbd/client: fix drop_sync [CVE-2017-2630] 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 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" From: Vladimir Sementsov-Ogievskiy Comparison symbol is misused. It may lead to memory corruption. Introduced in commit 7d3123e. Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20170203154757.36140-6-vsementsov@virtuozzo.com> [eblake: add CVE details] Signed-off-by: Eric Blake Reviewed-by: Marc-Andr=C3=A9 Lureau --- nbd/client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nbd/client.c b/nbd/client.c index ffb0743..0d16cd1 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -94,7 +94,7 @@ static ssize_t drop_sync(QIOChannel *ioc, size_t size) char small[1024]; char *buffer; - buffer =3D sizeof(small) < size ? small : g_malloc(MIN(65536, size)); + buffer =3D sizeof(small) > size ? small : g_malloc(MIN(65536, size)); while (size > 0) { ssize_t count =3D read_sync(ioc, buffer, MIN(65536, size)); --=20 2.9.3 From nobody Thu Apr 25 05:23:16 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.zoho.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 1487645423669334.3357291606668; Mon, 20 Feb 2017 18:50:23 -0800 (PST) Received: from localhost ([::1]:41913 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0X0-0002XR-1N for importer@patchew.org; Mon, 20 Feb 2017 21:50:22 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40784) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0Q0-0005Yf-UQ for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cg0Pz-0003VF-3g for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53236) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cg0Pr-0003Q2-SE; Mon, 20 Feb 2017 21:43:00 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EB5A661B91; Tue, 21 Feb 2017 02:42:59 +0000 (UTC) Received: from red.redhat.com (ovpn-123-67.rdu2.redhat.com [10.10.123.67] (may be forged)) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1L2gnuB023090; Mon, 20 Feb 2017 21:42:58 -0500 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 20 Feb 2017 20:42:42 -0600 Message-Id: <20170221024248.11027-3-eblake@redhat.com> In-Reply-To: <20170221024248.11027-1-eblake@redhat.com> References: <20170221024248.11027-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 21 Feb 2017 02:43:00 +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 v4 2/8] 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 , 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: Marc-Andr=C3=A9 Lureau --- 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 | 47 +++++++++++++++++++++++++---------------------- qemu-nbd.c | 10 ++++------ 6 files changed, 50 insertions(+), 45 deletions(-) diff --git a/block/nbd-client.h b/block/nbd-client.h index f8d6006..098b65c 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 3e373f0..8cc9cbe 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -123,16 +123,23 @@ 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_wr_syncv(QIOChannel *ioc, struct iovec *iov, size_t niov, size_t length, bool do_read); -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); ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request); ssize_t nbd_receive_reply(QIOChannel *ioc, NBDReply *reply); int nbd_client(int fd); diff --git a/block/nbd-client.c b/block/nbd-client.c index 06f1532..32d7c90 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -258,7 +258,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; } @@ -287,12 +287,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)) { @@ -317,7 +317,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; } @@ -346,7 +346,7 @@ int nbd_client_co_pdiscard(BlockDriverState *bs, int64_= t offset, int count) NBDReply reply; ssize_t ret; - if (!(client->nbdflags & NBD_FLAG_SEND_TRIM)) { + if (!(client->info.flags & NBD_FLAG_SEND_TRIM)) { return 0; } @@ -405,19 +405,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 35f24be..c43fa35 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -485,7 +485,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 0d16cd1..69f0e09 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -454,13 +454,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; @@ -573,17 +573,19 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char= *name, uint16_t *flags, } /* Read the response */ - if (read_sync(ioc, &s, sizeof(s)) !=3D sizeof(s)) { + if (read_sync(ioc, &info->size, sizeof(info->size)) !=3D + sizeof(info->size)) { error_setg(errp, "Failed to read export length"); goto fail; } - *size =3D be64_to_cpu(s); + be64_to_cpus(&info->size); - if (read_sync(ioc, flags, sizeof(*flags)) !=3D sizeof(*flags)) { + if (read_sync(ioc, &info->flags, sizeof(info->flags)) !=3D + sizeof(info->flags)) { error_setg(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; @@ -596,12 +598,12 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char= *name, uint16_t *flags, goto fail; } - if (read_sync(ioc, &s, sizeof(s)) !=3D sizeof(s)) { + if (read_sync(ioc, &info->size, sizeof(info->size)) !=3D + sizeof(info->size)) { error_setg(errp, "Failed to read export length"); goto fail; } - *size =3D be64_to_cpu(s); - TRACE("Size is %" PRIu64, *size); + be64_to_cpus(&info->size); if (read_sync(ioc, &oldflags, sizeof(oldflags)) !=3D sizeof(oldfla= gs)) { error_setg(errp, "Failed to read export flags"); @@ -612,13 +614,14 @@ 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("Size is %" PRIu64 ", export flags %" PRIx16, *size, *flags); + TRACE("Size is %" PRIu64 ", export flags %" PRIx16, + info->size, info->flags); if (zeroes && drop_sync(ioc, 124) !=3D 124) { error_setg(errp, "Failed to read reserved block"); goto fail; @@ -630,11 +633,11 @@ 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) { - unsigned long sectors =3D size / BDRV_SECTOR_SIZE; - if (size / BDRV_SECTOR_SIZE !=3D sectors) { - LOG("Export size %lld too large for 32-bit kernel", (long long) si= ze); + unsigned long sectors =3D info->size / BDRV_SECTOR_SIZE; + if (info->size / BDRV_SECTOR_SIZE !=3D sectors) { + LOG("Export size %" PRId64 " too large for 32-bit kernel", info->s= ize); return -E2BIG; } @@ -655,9 +658,9 @@ int nbd_init(int fd, QIOChannelSocket *sioc, uint16_t f= lags, off_t size) } TRACE("Setting size to %lu block(s)", sectors); - if (size % BDRV_SECTOR_SIZE) { + if (info->size % BDRV_SECTOR_SIZE) { TRACE("Ignoring trailing %d bytes of export", - (int) (size % BDRV_SECTOR_SIZE)); + (int) (info->size % BDRV_SECTOR_SIZE)); } if (ioctl(fd, NBD_SET_SIZE_BLOCKS, sectors) < 0) { @@ -666,9 +669,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("Setting readonly attribute"); if (ioctl(fd, BLKROSET, (unsigned long) &read_only) < 0) { @@ -726,7 +729,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) { return -ENOTSUP; } diff --git a/qemu-nbd.c b/qemu-nbd.c index e4fede6..c598cbe 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -254,8 +254,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; @@ -270,9 +269,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); @@ -287,7 +285,7 @@ static void *nbd_client_thread(void *arg) goto out_socket; } - ret =3D nbd_init(fd, sioc, nbdflags, size); + ret =3D nbd_init(fd, sioc, &info); if (ret < 0) { goto out_fd; } --=20 2.9.3 From nobody Thu Apr 25 05:23:16 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.zoho.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 1487645163086902.1482835329148; Mon, 20 Feb 2017 18:46:03 -0800 (PST) Received: from localhost ([::1]:41896 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0Sn-0007GC-Ok for importer@patchew.org; Mon, 20 Feb 2017 21:46:01 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40746) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0Q0-0005Xp-0x for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cg0Pz-0003VU-4W for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:08 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41778) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cg0Pt-0003Qa-46; Mon, 20 Feb 2017 21:43:01 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 33728C054C59; Tue, 21 Feb 2017 02:43:01 +0000 (UTC) Received: from red.redhat.com (ovpn-123-67.rdu2.redhat.com [10.10.123.67] (may be forged)) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1L2gnuC023090; Mon, 20 Feb 2017 21:42:59 -0500 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 20 Feb 2017 20:42:43 -0600 Message-Id: <20170221024248.11027-4-eblake@redhat.com> In-Reply-To: <20170221024248.11027-1-eblake@redhat.com> References: <20170221024248.11027-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Tue, 21 Feb 2017 02:43:01 +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 v4 3/8] block: Add blk_get_opt_transfer() 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 , 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 would like to advertise the optimal I/O size to the client; but it would be a layering violation to peek into blk_bs(blk)->bl, when we only have a BB. This copies the existing blk_get_max_transfer() in reading a value from the top BDS; where that value was picked via bdrv_refresh_limits() to reflect the overall constraints of the entire BDS chain. Signed-off-by: Eric Blake Reviewed-by: Marc-Andr=C3=A9 Lureau --- v4: retitle, as part of rebasing to byte limits v3: new patch --- include/sysemu/block-backend.h | 1 + block/block-backend.c | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 6444e41..882480a 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -174,6 +174,7 @@ void blk_lock_medium(BlockBackend *blk, bool locked); void blk_eject(BlockBackend *blk, bool eject_flag); int blk_get_flags(BlockBackend *blk); uint32_t blk_get_max_transfer(BlockBackend *blk); +uint32_t blk_get_opt_transfer(BlockBackend *blk); int blk_get_max_iov(BlockBackend *blk); void blk_set_guest_block_size(BlockBackend *blk, int align); void *blk_try_blockalign(BlockBackend *blk, size_t size); diff --git a/block/block-backend.c b/block/block-backend.c index efbf398..4d91ff8 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1426,6 +1426,18 @@ uint32_t blk_get_max_transfer(BlockBackend *blk) return MIN_NON_ZERO(max, INT_MAX); } +/* Returns the optimum transfer length, in bytes; may be 0 if no optimum */ +uint32_t blk_get_opt_transfer(BlockBackend *blk) +{ + BlockDriverState *bs =3D blk_bs(blk); + + if (bs) { + return bs->bl.opt_transfer; + } else { + return 0; + } +} + int blk_get_max_iov(BlockBackend *blk) { return blk->root->bs->bl.max_iov; --=20 2.9.3 From nobody Thu Apr 25 05:23:16 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.zoho.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 1487645606812655.4553928946392; Mon, 20 Feb 2017 18:53:26 -0800 (PST) Received: from localhost ([::1]:41929 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0Zx-00055k-Bh for importer@patchew.org; Mon, 20 Feb 2017 21:53:25 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40826) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0Q2-0005ZX-Oq for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cg0Q1-0003X8-29 for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:10 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53248) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cg0Pu-0003RM-D5; Mon, 20 Feb 2017 21:43:02 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7BFCE61BB8; Tue, 21 Feb 2017 02:43:02 +0000 (UTC) Received: from red.redhat.com (ovpn-123-67.rdu2.redhat.com [10.10.123.67] (may be forged)) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1L2gnuD023090; Mon, 20 Feb 2017 21:43:01 -0500 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 20 Feb 2017 20:42:44 -0600 Message-Id: <20170221024248.11027-5-eblake@redhat.com> In-Reply-To: <20170221024248.11027-1-eblake@redhat.com> References: <20170221024248.11027-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 21 Feb 2017 02:43:02 +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 v4 4/8] 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 , 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. Doing this points out a debug message in server.c that got parameters mixed up. Signed-off-by: Eric Blake Reviewed-by: Marc-Andr=C3=A9 Lureau --- v4: new patch --- include/block/nbd.h | 34 +++++++++++++++++++------- nbd/nbd-internal.h | 9 +++---- nbd/client.c | 56 ++++++++++++++++++++++++++++--------------- nbd/common.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++= ++++ nbd/server.c | 17 +++++++------ 5 files changed, 145 insertions(+), 40 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index 8cc9cbe..c84e022 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,36 @@ 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) + +/* 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 f43d990..aa5b2fd 100644 --- a/nbd/nbd-internal.h +++ b/nbd/nbd-internal.h @@ -76,12 +76,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. @@ -122,5 +116,8 @@ 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); #endif diff --git a/nbd/client.c b/nbd/client.c index 69f0e09..f96539b 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 @@ -130,7 +130,8 @@ 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("Sending option request %" PRIu32", len %" PRIu32, opt, len); + TRACE("Sending option request %" PRIu32" (%s), len %" PRIu32, opt, + nbd_opt_lookup(opt), len); stq_be_p(&req.magic, NBD_OPTS_MAGIC); stl_be_p(&req.option, opt); @@ -180,8 +181,10 @@ static int nbd_receive_option_reply(QIOChannel *ioc, u= int32_t opt, be32_to_cpus(&reply->type); be32_to_cpus(&reply->length); - TRACE("Received option reply %" PRIx32", type %" PRIx32", len %" PRIu3= 2, - reply->option, reply->type, reply->length); + TRACE("Received option reply %" PRIx32" (%s), type %" PRIx32 + " (%s), len %" PRIu32, + reply->option, nbd_opt_lookup(reply->option), + reply->type, nbd_rep_lookup(reply->type), reply->length); if (reply->magic !=3D NBD_REP_MAGIC) { error_setg(errp, "Unexpected option reply magic"); @@ -215,12 +218,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 (read_sync(ioc, msg, reply->length) !=3D reply->length) { - error_setg(errp, "failed to read option error message"); + error_setg(errp, "failed to read option error 0x%" PRIx32 + " (%s) message", + reply->type, nbd_rep_lookup(reply->type)); goto cleanup; } msg[reply->length] =3D '\0'; @@ -229,38 +236,49 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_= opt_reply *reply, switch (reply->type) { case NBD_REP_ERR_UNSUP: TRACE("server doesn't understand request %" PRIx32 - ", attempting fallback", reply->option); + " (%s), attempting fallback", + reply->option, nbd_opt_lookup(reply->option)); 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 a5f39ea..85622f9 100644 --- a/nbd/common.c +++ b/nbd/common.c @@ -89,3 +89,72 @@ 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"; + 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 ""; + } +} diff --git a/nbd/server.c b/nbd/server.c index efe5cb8..767ca0f 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 @@ -206,8 +206,8 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc, = uint32_t type, { uint64_t magic; - TRACE("Reply opt=3D%" PRIx32 " type=3D%" PRIx32 " len=3D%" PRIu32, - type, opt, len); + TRACE("Reply opt=3D%" PRIx32 " (%s), type=3D%" PRIx32 " (%s), len=3D%"= PRIu32, + opt, nbd_opt_lookup(opt), type, nbd_rep_lookup(type), len); magic =3D cpu_to_be64(NBD_REP_MAGIC); if (nbd_negotiate_write(ioc, &magic, sizeof(magic)) !=3D sizeof(magic)= ) { @@ -493,7 +493,8 @@ static int nbd_negotiate_options(NBDClient *client) } length =3D be32_to_cpu(length); - TRACE("Checking option 0x%" PRIx32, clientflags); + TRACE("Checking option 0x%" PRIx32 " (%s)", clientflags, + nbd_opt_lookup(clientflags)); if (client->tlscreds && client->ioc =3D=3D (QIOChannel *)client->sioc) { QIOChannel *tioc; @@ -581,8 +582,9 @@ static int nbd_negotiate_options(NBDClient *client) NBD_REP_ERR_UNSUP, clientflags, "Unsupported option 0x%" - PRIx32, - clientflags); + PRIx32 " (%s)", + clientflags, + nbd_opt_lookup(clientflag= s)); if (ret < 0) { return ret; } @@ -598,7 +600,8 @@ static int nbd_negotiate_options(NBDClient *client) return nbd_negotiate_handle_export_name(client, length); default: - TRACE("Unsupported option 0x%" PRIx32, clientflags); + TRACE("Unsupported option 0x%" PRIx32 " (%s)", clientflags, + nbd_opt_lookup(clientflags)); return -EINVAL; } } --=20 2.9.3 From nobody Thu Apr 25 05:23:16 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.zoho.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 1487645139565486.51266220557216; Mon, 20 Feb 2017 18:45:39 -0800 (PST) Received: from localhost ([::1]:41889 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0SL-0006q5-5b for importer@patchew.org; Mon, 20 Feb 2017 21:45:33 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40827) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0Q2-0005ZY-P8 for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cg0Q1-0003XG-6z for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:10 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52002) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cg0Pv-0003Rr-Dg; Mon, 20 Feb 2017 21:43:03 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8003C80F97; Tue, 21 Feb 2017 02:43:03 +0000 (UTC) Received: from red.redhat.com (ovpn-123-67.rdu2.redhat.com [10.10.123.67] (may be forged)) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1L2gnuE023090; Mon, 20 Feb 2017 21:43:02 -0500 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 20 Feb 2017 20:42:45 -0600 Message-Id: <20170221024248.11027-6-eblake@redhat.com> In-Reply-To: <20170221024248.11027-1-eblake@redhat.com> References: <20170221024248.11027-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Tue, 21 Feb 2017 02:43: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 v4 5/8] 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 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 --- v4: revamp to another round of NBD protocol changes v3: revamp to match latest version of NBD protocol --- nbd/server.c | 208 +++++++++++++++++++++++++++++++++++++++++++++++++++++++= ---- 1 file changed, 195 insertions(+), 13 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 767ca0f..3b1a4a5 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -209,6 +209,7 @@ static int nbd_negotiate_send_rep_len(QIOChannel *ioc, = uint32_t type, TRACE("Reply opt=3D%" PRIx32 " (%s), type=3D%" PRIx32 " (%s), len=3D%"= PRIu32, 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_negotiate_write(ioc, &magic, sizeof(magic)) !=3D sizeof(magic)= ) { LOG("write failed (rep magic)"); @@ -331,6 +332,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); } +/* 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) { int rc =3D -EINVAL; @@ -365,6 +368,171 @@ fail: return rc; } +/* 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) +{ + int rc; + + TRACE("Sending NBD_REP_INFO type %" PRIu16 " (%s) with remaining lengt= h %" + PRIu32, info, nbd_info_lookup(info), length); + rc =3D nbd_negotiate_send_rep_len(client->ioc, NBD_REP_INFO, opt, + sizeof(info) + length); + if (rc < 0) { + return rc; + } + cpu_to_be16s(&info); + if (nbd_negotiate_write(client->ioc, &info, sizeof(info)) !=3D + sizeof(info)) { + LOG("write failed"); + return -EIO; + } + if (nbd_negotiate_write(client->ioc, buf, length) !=3D length) { + LOG("write failed"); + 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) +{ + 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: + 2 bytes: N, number of requests (can be 0) + N * 2 bytes: N requests + 4 bytes: L, name length (can be 0) + L bytes: export name + */ + if (length < sizeof(requests) + sizeof(namelen)) { + msg =3D "overall request too short"; + goto invalid; + } + if (nbd_negotiate_read(client->ioc, &requests, sizeof(requests)) !=3D + sizeof(requests)) { + LOG("read failed"); + return -EIO; + } + be16_to_cpus(&requests); + length -=3D sizeof(requests); + TRACE("Client requested %d items of info", requests); + if (requests > (length - sizeof(namelen)) / sizeof(request)) { + msg =3D "too many requests for overall length"; + goto invalid; + } + while (requests--) { + if (nbd_negotiate_read(client->ioc, &request, sizeof(request)) != =3D + sizeof(request)) { + LOG("read failed"); + return -EIO; + } + be16_to_cpus(&request); + length -=3D sizeof(request); + TRACE("Client requested info %d (%s)", 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; + } + } + + if (nbd_negotiate_read(client->ioc, &namelen, sizeof(namelen)) !=3D + sizeof(namelen)) { + LOG("read failed"); + return -EIO; + } + be32_to_cpus(&namelen); + length -=3D sizeof(namelen); + TRACE("Client requested namelen %u", namelen); + if (length !=3D namelen || namelen > sizeof(name)) { + msg =3D "name too long"; + goto invalid; + } + if (nbd_negotiate_read(client->ioc, name, length) !=3D length) { + LOG("read failed"); + return -EIO; + } + name[length] =3D '\0'; + + TRACE("Client requested info on export '%s'", name); + + exp =3D nbd_export_find(name); + if (!exp) { + return nbd_negotiate_send_rep_err(client->ioc, NBD_REP_ERR_UNKNOWN, + opt, "export '%s' not present", + 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); + 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); + if (rc < 0) { + return rc; + } + } + + /* Send NBD_INFO_EXPORT always */ + TRACE("advertising size %" PRIu64 " and flags %" PRIx16, + 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); + if (rc < 0) { + return rc; + } + + /* Final reply */ + rc =3D nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, opt); + 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_negotiate_drop_sync(client->ioc, length) !=3D length) { + return -EIO; + } + return nbd_negotiate_send_rep_err(client->ioc, NBD_REP_ERR_INVALID, op= t, + "%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, @@ -420,9 +588,10 @@ static QIOChannel *nbd_negotiate_handle_starttls(NBDCl= ient *client, } -/* Process all NBD_OPT_* client option commands. - * Return -errno on error, 0 on success. */ -static int nbd_negotiate_options(NBDClient *client) +/* Process all NBD_OPT_* client option commands, during fixed newstyle + * negotiation. Return -errno on error, 0 on successful NBD_OPT_EXPORT_NAM= E, + * and 1 on successful NBD_OPT_GO. */ +static int nbd_negotiate_options(NBDClient *client, uint16_t myflags) { uint32_t flags; bool fixedNewstyle =3D false; @@ -555,6 +724,16 @@ static int nbd_negotiate_options(NBDClient *client) case NBD_OPT_EXPORT_NAME: return nbd_negotiate_handle_export_name(client, length); + case NBD_OPT_INFO: + case NBD_OPT_GO: + ret =3D nbd_negotiate_handle_info(client, length, clientfl= ags, + myflags); + if (ret) { + assert(ret < 0 || clientflags =3D=3D NBD_OPT_GO); + return ret; + } + break; + case NBD_OPT_STARTTLS: if (nbd_negotiate_drop_sync(client->ioc, length) !=3D leng= th) { return -EIO; @@ -675,20 +854,23 @@ static coroutine_fn int nbd_negotiate(NBDClientNewDat= a *data) LOG("write failed"); goto fail; } - rc =3D nbd_negotiate_options(client); - if (rc !=3D 0) { + rc =3D nbd_negotiate_options(client, myflags); + if (rc < 0) { LOG("option negotiation failed"); goto fail; } - TRACE("advertising size %" PRIu64 " and flags %x", - 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; - if (nbd_negotiate_write(client->ioc, buf + 18, len) !=3D len) { - LOG("write failed"); - goto fail; + if (!rc) { + /* If options ended with NBD_OPT_GO, we already sent this. */ + TRACE("advertising size %" PRIu64 " and flags %x", + 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; + if (nbd_negotiate_write(client->ioc, buf + 18, len) !=3D len) { + LOG("write failed"); + goto fail; + } } } --=20 2.9.3 From nobody Thu Apr 25 05:23:16 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.zoho.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 1487645142282884.0884207200313; Mon, 20 Feb 2017 18:45:42 -0800 (PST) Received: from localhost ([::1]:41890 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0SO-0006tG-T0 for importer@patchew.org; Mon, 20 Feb 2017 21:45:36 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40809) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0Q2-0005ZW-7H for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cg0Q0-0003Wm-RJ for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:10 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43000) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cg0Pw-0003S7-Fx; Mon, 20 Feb 2017 21:43:04 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8AA3AC04B317; Tue, 21 Feb 2017 02:43:04 +0000 (UTC) Received: from red.redhat.com (ovpn-123-67.rdu2.redhat.com [10.10.123.67] (may be forged)) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1L2gnuF023090; Mon, 20 Feb 2017 21:43:03 -0500 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 20 Feb 2017 20:42:46 -0600 Message-Id: <20170221024248.11027-7-eblake@redhat.com> In-Reply-To: <20170221024248.11027-1-eblake@redhat.com> References: <20170221024248.11027-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Tue, 21 Feb 2017 02:43: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 v4 6/8] 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 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 --- v4: NBD protocol changes, again v3: revamp to match latest version of NBD protocol --- nbd/nbd-internal.h | 3 ++ nbd/client.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++++= +++- 2 files changed, 131 insertions(+), 2 deletions(-) diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h index aa5b2fd..96c204b 100644 --- a/nbd/nbd-internal.h +++ b/nbd/nbd-internal.h @@ -56,8 +56,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 f96539b..b408945 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -380,6 +380,118 @@ 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("Attempting NBD_OPT_GO for export '%s'", wantname); + buf =3D g_malloc(2 + 4 + len + 1); + stw_be_p(buf, 0); /* No requests, live with whatever server sends */ + stl_be_p(buf + 2, len); + memcpy(buf + 6, wantname, len); + if (nbd_send_option_request(ioc, NBD_OPT_GO, len + 6, buf, errp) < 0) { + return -1; + } + + TRACE("Reading export info"); + 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("export is good to go"); + 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 (read_sync(ioc, &type, sizeof(type)) !=3D sizeof(type)) { + error_setg(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 (read_sync(ioc, &info->size, sizeof(info->size)) !=3D + sizeof(info->size)) { + error_setg(errp, "failed to read info size"); + nbd_send_opt_abort(ioc); + return -1; + } + be64_to_cpus(&info->size); + if (read_sync(ioc, &info->flags, sizeof(info->flags)) !=3D + sizeof(info->flags)) { + error_setg(errp, "failed to read info flags"); + nbd_send_opt_abort(ioc); + return -1; + } + be16_to_cpus(&info->flags); + TRACE("Size is %" PRIu64 ", export flags %" PRIx16, + info->size, info->flags); + break; + + default: + TRACE("ignoring unknown export info %" PRIu16 " (%s)", type, + nbd_info_lookup(type)); + if (drop_sync(ioc, len) !=3D len) { + error_setg(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, @@ -574,11 +686,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; --=20 2.9.3 From nobody Thu Apr 25 05:23:16 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.zoho.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 1487645650548235.52929970946548; Mon, 20 Feb 2017 18:54:10 -0800 (PST) Received: from localhost ([::1]:41932 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0af-0005c3-7r for importer@patchew.org; Mon, 20 Feb 2017 21:54:09 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40800) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0Q1-0005ZV-Qp for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cg0Q0-0003WT-OO for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43004) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cg0Px-0003Sh-IG; Mon, 20 Feb 2017 21:43:05 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 95BF7C04BD40; Tue, 21 Feb 2017 02:43:05 +0000 (UTC) Received: from red.redhat.com (ovpn-123-67.rdu2.redhat.com [10.10.123.67] (may be forged)) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1L2gnuG023090; Mon, 20 Feb 2017 21:43:04 -0500 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 20 Feb 2017 20:42:47 -0600 Message-Id: <20170221024248.11027-8-eblake@redhat.com> In-Reply-To: <20170221024248.11027-1-eblake@redhat.com> References: <20170221024248.11027-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Tue, 21 Feb 2017 02:43:05 +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 v4 7/8] 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 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 --- v4: new patch --- nbd/server.c | 47 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 3b1a4a5..d63e5d3 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -409,6 +409,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; @@ -444,11 +446,16 @@ static int nbd_negotiate_handle_info(NBDClient *clien= t, uint32_t length, length -=3D sizeof(request); TRACE("Client requested info %d (%s)", 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; } } @@ -499,6 +506,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)->request_align? */ + sizes[0] =3D (opt =3D=3D NBD_OPT_INFO || blocksize) ? BDRV_SECTOR_SIZE= : 1; + /* preferred - At least 4096, but larger as appropriate. */ + sizes[1] =3D MAX(blk_get_opt_transfer(exp->blk), 4096); + /* maximum - At most 32M, but smaller as appropriate. */ + sizes[2] =3D MIN(blk_get_max_transfer(exp->blk), NBD_MAX_BUFFER_SIZE); + TRACE("advertising minimum 0x%" PRIx32 ", preferred 0x%" PRIx32 + ", maximum 0x%" PRIx32, 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); + if (rc < 0) { + return rc; + } + /* Send NBD_INFO_EXPORT always */ TRACE("advertising size %" PRIu64 " and flags %" PRIx16, exp->size, exp->nbdflags | myflags); @@ -510,6 +538,17 @@ 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, + "request NBD_INFO_BLOCK_SIZE to " + "use this export"); + } + /* Final reply */ rc =3D nbd_negotiate_send_rep(client->ioc, NBD_REP_ACK, opt); if (rc < 0) { --=20 2.9.3 From nobody Thu Apr 25 05:23:16 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.zoho.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 1487645810983855.245870008005; Mon, 20 Feb 2017 18:56:50 -0800 (PST) Received: from localhost ([::1]:41951 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0dF-0007a0-Ob for importer@patchew.org; Mon, 20 Feb 2017 21:56:49 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40919) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cg0QA-0005fq-1J for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cg0Q5-0003ZZ-13 for qemu-devel@nongnu.org; Mon, 20 Feb 2017 21:43:18 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40278) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cg0Py-0003UW-Se; Mon, 20 Feb 2017 21:43:07 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id EE7E38047A; Tue, 21 Feb 2017 02:43:06 +0000 (UTC) Received: from red.redhat.com (ovpn-123-67.rdu2.redhat.com [10.10.123.67] (may be forged)) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1L2gnuH023090; Mon, 20 Feb 2017 21:43:05 -0500 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 20 Feb 2017 20:42:48 -0600 Message-Id: <20170221024248.11027-9-eblake@redhat.com> In-Reply-To: <20170221024248.11027-1-eblake@redhat.com> References: <20170221024248.11027-1-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.27 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Tue, 21 Feb 2017 02:43: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 v4 8/8] 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 , 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 --- v4: new patch --- include/block/nbd.h | 6 ++++ block/nbd-client.c | 4 +++ block/nbd.c | 14 +++++++-- nbd/client.c | 86 ++++++++++++++++++++++++++++++++++++++++++++-----= ---- qemu-nbd.c | 2 +- 5 files changed, 94 insertions(+), 18 deletions(-) diff --git a/include/block/nbd.h b/include/block/nbd.h index c84e022..ae8cc12 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -143,8 +143,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 32d7c90..7dfe4ec 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -404,6 +404,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); @@ -418,6 +419,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 c43fa35..3afd475 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -465,9 +465,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 b408945..9f62a02 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -399,11 +399,17 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wa= ntname, info->flags =3D 0; TRACE("Attempting NBD_OPT_GO for export '%s'", wantname); - buf =3D g_malloc(2 + 4 + len + 1); - stw_be_p(buf, 0); /* No requests, live with whatever server sends */ - stl_be_p(buf + 2, len); - memcpy(buf + 6, wantname, len); - if (nbd_send_option_request(ioc, NBD_OPT_GO, len + 6, buf, errp) < 0) { + /* At most one request, everything else up to server */ + buf =3D g_malloc(2 + 2 * info->request_sizes + 4 + len); + stw_be_p(buf, info->request_sizes); + if (info->request_sizes) { + stw_be_p(buf + 2, NBD_INFO_BLOCK_SIZE); + } + stl_be_p(buf + 2 + 2 * info->request_sizes, len); + memcpy(buf + 2 + 2 * info->request_sizes + 4, wantname, len); + if (nbd_send_option_request(ioc, NBD_OPT_GO, + 2 + 2 * info->request_sizes + 4 + len, buf, + errp) < 0) { return -1; } @@ -435,8 +441,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; } @@ -479,6 +486,51 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wan= tname, 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 (read_sync(ioc, &info->min_block, sizeof(info->min_block)) = !=3D + sizeof(info->min_block)) { + error_setg(errp, "failed to read info minimum block size"); + 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 (read_sync(ioc, &info->opt_block, sizeof(info->opt_block)) = !=3D + sizeof(info->opt_block)) { + error_setg(errp, "failed to read info preferred block size= "); + 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 (read_sync(ioc, &info->max_block, sizeof(info->max_block)) = !=3D + sizeof(info->max_block)) { + error_setg(errp, "failed to read info maximum block size"); + nbd_send_opt_abort(ioc); + return -1; + } + be32_to_cpus(&info->max_block); + TRACE("Block sizes are 0x%" PRIx32 ", 0x%" PRIx32 ", 0x%" PRIx= 32, + info->min_block, info->opt_block, info->max_block); + break; + default: TRACE("ignoring unknown export info %" PRIu16 " (%s)", type, nbd_info_lookup(type)); @@ -779,8 +831,14 @@ fail: #ifdef __linux__ int nbd_init(int fd, QIOChannelSocket *sioc, NBDExportInfo *info) { - 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) { LOG("Export size %" PRId64 " too large for 32-bit kernel", info->s= ize); return -E2BIG; } @@ -793,18 +851,18 @@ int nbd_init(int fd, QIOChannelSocket *sioc, NBDExpor= tInfo *info) return -serrno; } - TRACE("Setting block size to %lu", (unsigned long)BDRV_SECTOR_SIZE); + TRACE("Setting block size to %lu", 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; LOG("Failed setting NBD block size"); return -serrno; } TRACE("Setting size to %lu block(s)", sectors); - if (info->size % BDRV_SECTOR_SIZE) { - TRACE("Ignoring trailing %d bytes of export", - (int) (info->size % BDRV_SECTOR_SIZE)); + if (info->size % sector_size) { + TRACE("Ignoring trailing %" PRId64 " bytes of export", + info->size % sector_size); } if (ioctl(fd, NBD_SET_SIZE_BLOCKS, sectors) < 0) { diff --git a/qemu-nbd.c b/qemu-nbd.c index c598cbe..45405ce 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -254,7 +254,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; --=20 2.9.3