Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Allow NBD_CMD_FLAG_NO_HOLE during NBD_CMD_WRITE

[Markus Pargmann]

Hi,

On Monday 04 April 2016 16:15:43 Eric Blake wrote:
> qemu already has an existing server implementation option that will
> explicitly search the payload of NBD_CMD_WRITE for large blocks of
> zeroes, and punch holes in the underlying file.  For old clients
> that don't know how to use the new NBD_CMD_WRITE_ZEROES, this is a
> workaround to keep the server's destination file approximately as
> sparse as the client's source.  However, for new clients that know
> how to explicitly request holes, it is unnecessary overhead; and
> can lead to the server punching a hole and risking fragmentation or
> future ENOSPC even when the client explicitly wanted to write
> zeroes rather than a hole.  So it makes sense to let the new
> NBD_CMD_FLAG_NO_HOLE work for WRITE as well as WRITE_ZEROES.

From the commit message it sounds like this is only for new clients
supporting WRITE_ZEROES because for those we don't want to search
through all the data of normal WRITEs. If you don't need to set this for
each WRITE individually perhaps we could move it to the negotiation
part?

Best Regards,

Markus

> 
> Signed-off-by: Eric Blake <address@hidden>
> ---
> 
> v2: fix some typos, add a sentence about server MUST support
> NBD_CMD_FLAG_NO_HOLE if it advertises NBD_FLAG_SEND_WRITE_ZEROES
> 
>  doc/proto.md | 38 ++++++++++++++++++++++++++++++--------
>  1 file changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/doc/proto.md b/doc/proto.md
> index 35a3266..bca0525 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -487,7 +487,7 @@ valid may depend on negotiation during the handshake 
> phase.
>    `NBD_CMD_WRITE_ZEROES` commands.  SHOULD be set to 1 if the client requires
>    "Force Unit Access" mode of operation.  MUST NOT be set unless transmission
>    flags included `NBD_FLAG_SEND_FUA`.
> -- bit 1, `NBD_CMD_NO_HOLE`; defined by the experimental `WRITE_ZEROES`
> +- bit 1, `NBD_CMD_FLAG_NO_HOLE`; defined by the experimental `WRITE_ZEROES`
>    extension; see below.
>  - bit 2, `NBD_CMD_FLAG_DF`; defined by the experimental `STRUCTURED_REPLY`
>    extension; see below
> @@ -574,7 +574,7 @@ The following request types exist:
> 
>      After issuing this command, a client MUST NOT make any assumptions
>      about the contents of the export affected by this command, until
> -    overwriting it again with `NBD_CMD_WRITE`.
> +    overwriting it again with `NBD_CMD_WRITE` or `NBD_CMD_WRITE_ZEROES`.
> 
>      A client MUST NOT send a trim request unless `NBD_FLAG_SEND_TRIM`
>      was set in the transmission flags field.
> @@ -737,8 +737,14 @@ by a sparse file. With current NBD command set, the 
> client has to issue
>  through the wire. The server has to write the data onto disk, effectively
>  losing the sparseness.
> 
> -To remedy this, a `WRITE_ZEROES` extension is envisioned. This extension adds
> -one new command and one new command flag.
> +To remedy this, a `WRITE_ZEROES` extension is envisioned. This
> +extension adds one new transmission flag, one new command, and one new
> +command flag; and refines an existing command.
> +
> +* `NBD_FLAG_SEND_WRITE_ZEROES`
> +
> +    The server SHOULD set this transmission flag to 1 if the
> +    `NBD_CMD_WRITE_ZEROES` request is supported.
> 
>  * `NBD_CMD_WRITE_ZEROES`
> 
> @@ -772,12 +778,28 @@ The server SHOULD return `ENOSPC` if it receives a 
> write zeroes request
>  including one or more sectors beyond the size of the device. It SHOULD
>  return `EPERM` if it receives a write zeroes request on a read-only export.
> 
> +* `NBD_CMD_WRITE`
> +
> +    By default, the server MAY search for large contiguous blocks of
> +    all zero content, and use trimming to zero out those portions of
> +    the write, even if it did not advertise `NBD_FLAG_SEND_TRIM`; but
> +    it MUST ensure that any trimmed areas of data read back as zero.
> +    However, the client MAY set the command flag
> +    `NBD_CMD_FLAG_NO_HOLE` to inform the server that the entire
> +    written area MUST be fully provisioned, ensuring that future
> +    writes to the same area will not cause fragmentation or cause
> +    failure due to insufficient space.  Clients SHOULD NOT set this
> +    flag unless the server advertised `NBD_FLAG_SEND_WRITE_ZEROES` in
> +    the transmission flags.
> +
>  The extension adds the following new command flag:
> 
> -- `NBD_CMD_FLAG_NO_HOLE`; valid during `NBD_CMD_WRITE_ZEROES`.
> -  SHOULD be set to 1 if the client wants to ensure that the server does
> -  not create a hole. The client MAY send `NBD_CMD_FLAG_NO_HOLE` even
> -  if `NBD_FLAG_SEND_TRIM` was not set in the transmission flags field.
> +- `NBD_CMD_FLAG_NO_HOLE`; valid during `NBD_CMD_WRITE` and
> +  `NBD_CMD_WRITE_ZEROES`.  SHOULD be set to 1 if the client wants to
> +  ensure that the server does not create a hole. The client MAY send
> +  `NBD_CMD_FLAG_NO_HOLE` even if `NBD_FLAG_SEND_TRIM` was not set in
> +  the transmission flags field.  The server MUST support the use of
> +  this flag if it advertises `NBD_FLAG_SEND_WRITE_ZEROES`.
> 
>  ### `STRUCTURED_REPLY` extension
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

signature.asc
Description: This is a digitally signed message part.