[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libguestfs] [PATCH v4 24/24] nbd/server: Add FLAG_PAYLOAD support t
From: |
Eric Blake |
Subject: |
Re: [Libguestfs] [PATCH v4 24/24] nbd/server: Add FLAG_PAYLOAD support to CMD_BLOCK_STATUS |
Date: |
Thu, 8 Jun 2023 14:19:58 -0500 |
User-agent: |
NeoMutt/20230517 |
On Thu, Jun 08, 2023 at 08:56:53AM -0500, Eric Blake wrote:
> Allow a client to request a subset of negotiated meta contexts. For
> example, a client may ask to use a single connection to learn about
> both block status and dirty bitmaps, but where the dirty bitmap
> queries only need to be performed on a subset of the disk; forcing the
> server to compute that information on block status queries in the rest
> of the disk is wasted effort (both at the server, and on the amount of
> traffic sent over the wire to be parsed and ignored by the client).
>
> Qemu as an NBD client never requests to use more than one meta
> context, so it has no need to use block status payloads. Testing this
> instead requires support from libnbd, which CAN access multiple meta
> contexts in parallel from a single NBD connection; an interop test
> submitted to the libnbd project at the same time as this patch
> demonstrates the feature working, as well as testing some corner cases
> (for example, when the payload length is longer than the export
> length), although other corner cases (like passing the same id
> duplicated) requires a protocol fuzzer because libnbd is not wired up
> to break the protocol that badly.
>
> This also includes tweaks to 'qemu-nbd --list' to show when a server
> is advertising the capability, and to the testsuite to reflect the
> addition to that output.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> +++ b/nbd/server.c
> @@ -512,6 +512,9 @@ static int nbd_negotiate_handle_export_name(NBDClient
> *client, bool no_zeroes,
> if (client->mode >= NBD_MODE_STRUCTURED) {
> myflags |= NBD_FLAG_SEND_DF;
> }
> + if (client->mode >= NBD_MODE_EXTENDED && client->contexts.count) {
> + myflags |= NBD_FLAG_BLOCK_STAT_PAYLOAD;
> + }
This one's awkward. At the last minute, I changed what went into
upstream NBD to state that a client that successfully negotiates
extended headers MUST NOT use NBD_OPT_EXPORT_NAME, which means this
branch should never fire for a compliant client. I don't think it
hurts to leave this in, but it does point out that I am missing code
(either here or in 17/24) that at least logs when we detect a
non-compliant client trying to connect with the wrong command.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
- [PATCH v4 09/24] nbd: Replace bool structured_reply with mode enum, (continued)
- [PATCH v4 09/24] nbd: Replace bool structured_reply with mode enum, Eric Blake, 2023/06/08
- [PATCH v4 11/24] nbd: Add types for extended headers, Eric Blake, 2023/06/08
- [PATCH v4 01/24] nbd/client: Use smarter assert, Eric Blake, 2023/06/08
- [PATCH v4 23/24] nbd/server: Prepare for per-request filtering of BLOCK_STATUS, Eric Blake, 2023/06/08
- [PATCH v4 24/24] nbd/server: Add FLAG_PAYLOAD support to CMD_BLOCK_STATUS, Eric Blake, 2023/06/08
- [PATCH v4 13/24] nbd/server: Refactor handling of request payload, Eric Blake, 2023/06/08
- [PATCH v4 16/24] nbd/server: Support 64-bit block status, Eric Blake, 2023/06/08
- [PATCH v4 18/24] nbd/client: Plumb errp through nbd_receive_replies, Eric Blake, 2023/06/08
- [PATCH v4 20/24] nbd/client: Accept 64-bit block status chunks, Eric Blake, 2023/06/08