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

[Paolo Bonzini]

On 04/04/2016 16:15, 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.

I don't think this is the case; the flag is explicitly meant to override
the client.

Paolo

> 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.
> 
> Signed-off-by: Eric Blake <address@hidden>
> ---
>  doc/proto.md | 33 +++++++++++++++++++++++++++------
>  1 file changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/proto.md b/doc/proto.md
> index 35a3266..fb97217 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -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,27 @@ 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 transmisison 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.
> 
>  ### `STRUCTURED_REPLY` extension
>