[Libguestfs] [nbdkit PATCH] protocol: Trivially implement NBD_CMD_FLAG_DF
Richard W.M. Jones
rjones at redhat.com
Fri Mar 29 12:43:33 UTC 2019
On Thu, Mar 28, 2019 at 10:44:42PM -0500, Eric Blake wrote:
> The DF flag is only available to clients that negotiated structured
> replies, and even then, the spec does not require that it be
> implemented. However, since our current implementation can't fragment
> NBD_CMD_READ replies, we trivially implement the flag (by ignoring
> it); we don't even have to pass it on to the plugins.
>
> Enhance some documentation about sparse reads while at it (when we
> finally do allow plugins to implement sparse reads, we'll have to pass
> on the DF flag, as well as perform reassembly back into one reply
> ourselves if the plugin ignored the flag).
>
> tests/test-eflags.sh can't really test this, as it requires the
> negotiation of structured replies (which in turn requires newstyle,
> not oldstyle...)
>
> Signed-off-by: Eric Blake <eblake at redhat.com>
Looks good to me, so ACK.
Thanks, Rich.
> docs/nbdkit-protocol.pod | 13 +++++++++++++
> common/protocol/protocol.h | 2 ++
> server/protocol-handshake.c | 3 +++
> server/protocol.c | 16 +++++++++++++++-
> TODO | 10 +++++++---
> 5 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/docs/nbdkit-protocol.pod b/docs/nbdkit-protocol.pod
> index a9a3390..f706cfd 100644
> --- a/docs/nbdkit-protocol.pod
> +++ b/docs/nbdkit-protocol.pod
> @@ -139,6 +139,19 @@ Supported in nbdkit E<ge> 1.11.10.
> Only C<base:allocation> (ie. querying which parts of an image are
> sparse) is supported.
>
> +Sparse reads (using C<NBD_REPLY_TYPE_OFFSET_HOLE> are not directly
> +supported, but a client can use block status to infer which portions
> +of the export do not need to be read.
> +
> +=item C<NBD_FLAG_DF>
> +
> +Supported in nbdkit E<ge> 1.11.11.
> +
> +This protocol extension allows a client to force an all-or-none read
> +when structured replies are in effect. However, the flag is a no-op
> +until we extend the plugin API to allow a fragmented read in the first
> +place.
> +
> =item Resize Extension
>
> I<Not supported>.
> diff --git a/common/protocol/protocol.h b/common/protocol/protocol.h
> index a7de2f0..4334be4 100644
> --- a/common/protocol/protocol.h
> +++ b/common/protocol/protocol.h
> @@ -94,6 +94,7 @@ extern const char *name_of_nbd_flag (int);
> #define NBD_FLAG_ROTATIONAL (1 << 4)
> #define NBD_FLAG_SEND_TRIM (1 << 5)
> #define NBD_FLAG_SEND_WRITE_ZEROES (1 << 6)
> +#define NBD_FLAG_SEND_DF (1 << 7)
> #define NBD_FLAG_CAN_MULTI_CONN (1 << 8)
>
> /* NBD options (new style handshake only). */
> @@ -217,6 +218,7 @@ extern const char *name_of_nbd_cmd (int);
> extern const char *name_of_nbd_cmd_flag (int);
> #define NBD_CMD_FLAG_FUA (1<<0)
> #define NBD_CMD_FLAG_NO_HOLE (1<<1)
> +#define NBD_CMD_FLAG_DF (1<<2)
> #define NBD_CMD_FLAG_REQ_ONE (1<<3)
>
> /* Error codes (previously errno).
> diff --git a/server/protocol-handshake.c b/server/protocol-handshake.c
> index 9653210..f0e5a2c 100644
> --- a/server/protocol-handshake.c
> +++ b/server/protocol-handshake.c
> @@ -121,6 +121,9 @@ protocol_compute_eflags (struct connection *conn, uint16_t *flags)
> if (fl)
> conn->can_extents = true;
>
> + if (conn->structured_replies)
> + eflags |= NBD_FLAG_SEND_DF;
> +
> *flags = eflags;
> return 0;
> }
> diff --git a/server/protocol.c b/server/protocol.c
> index 383938f..d94cd19 100644
> --- a/server/protocol.c
> +++ b/server/protocol.c
> @@ -110,7 +110,7 @@ validate_request (struct connection *conn,
>
> /* Validate flags */
> if (flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE |
> - NBD_CMD_FLAG_REQ_ONE)) {
> + NBD_CMD_FLAG_DF | NBD_CMD_FLAG_REQ_ONE)) {
> nbdkit_error ("invalid request: unknown flag (0x%x)", flags);
> *error = EINVAL;
> return false;
> @@ -121,6 +121,20 @@ validate_request (struct connection *conn,
> *error = EINVAL;
> return false;
> }
> + if (flags & NBD_CMD_FLAG_DF) {
> + if (cmd != NBD_CMD_READ) {
> + nbdkit_error ("invalid request: DF flag needs READ request");
> + *error = EINVAL;
> + return false;
> + }
> + if (!conn->structured_replies) {
> + nbdkit_error ("invalid request: "
> + "%s: structured replies was not negotiated",
> + name_of_nbd_cmd (cmd));
> + *error = EINVAL;
> + return false;
> + }
> + }
> if ((flags & NBD_CMD_FLAG_REQ_ONE) &&
> cmd != NBD_CMD_BLOCK_STATUS) {
> nbdkit_error ("invalid request: REQ_ONE flag needs BLOCK_STATUS request");
> diff --git a/TODO b/TODO
> index 81d692b..2b944e9 100644
> --- a/TODO
> +++ b/TODO
> @@ -24,8 +24,8 @@ General ideas for improvements
> to inform nbdkit when the response is ready:
> https://www.redhat.com/archives/libguestfs/2018-January/msg00149.html
>
> -* More NBD protocol features. The only currently missing feature is
> - online resize.
> +* More NBD protocol features. The only currently missing features are
> + sparse reads and online resize.
>
> * Add a callback to let plugins request minimum alignment for the
> buffer to pread/pwrite; useful for a plugin utilizing O_DIRECT or
> @@ -216,7 +216,11 @@ using ‘#define NBDKIT_API_VERSION <version>’.
>
> * pread could be changed to allow it to support Structured Replies
> (SRs). This could mean allowing it to return partial data, holes,
> - zeroes, etc.
> + zeroes, etc. For a client that negotiates SR coupled with a plugin
> + that supports .extents, the v2 protocol would allow us to at least
> + synthesize NBD_REPLY_TYPE_OFFSET_HOLE for less network traffic, even
> + though the plugin will still have to fully populate the .pread
> + buffer; the v3 protocol should make sparse reads more direct.
>
> * Parameters should be systematized so that they aren't just (key,
> value) strings. nbdkit should know the possible keys for the plugin
> --
> 2.20.1
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
More information about the Libguestfs
mailing list