Re: [Qemu-block] [Nbd] [PATCH] doc: Propose NBD_FLAG_INIT_ZEROES extension

[Alex Bligh]

> On 13 Dec 2016, at 12:18, Wouter Verhelst  wrote:
> 
> I'm not opposed either, but I do agree with you that we shouldn't add
> such a feature if it doesn't end up getting used. Especially so if it
> burns a flag in the (16-bit) "transmission flags" field, where space is
> at a premium.

I did suggest a few non-Qemu uses (see below). I think it might be
an idea if the reference implementation supported it before
merging (which per below should be trivial).

-- 
Alex Bligh


> Begin forwarded message:
> 
> From: Alex Bligh 
> Subject: Re: [Nbd] [Qemu-block] [PATCH] doc: Propose NBD_FLAG_INIT_ZEROES 
> extension
> Date: 6 December 2016 at 10:29:41 United Kingdom Time
> To: Kevin Wolf 
> Cc: Alex Bligh , Eric Blake , 
> "nbd-gene...@lists.sourceforge.net" , 
> xieying...@huawei.com, su...@huawei.com, qemu block , 
> "qemu-de...@nongnu.org" , Paolo Bonzini 
> 
> 
> 
>> On 6 Dec 2016, at 09:25, Kevin Wolf  wrote:
>> 
>> Am 06.12.2016 um 00:42 hat Eric Blake geschrieben:
>>> While not directly related to NBD_CMD_WRITE_ZEROES, the qemu
>>> team discovered that it is useful if a server can advertise
>>> whether an export is in a known-all-zeroes state at the time
>>> the client connects.
>> 
>> Does a server usually have the information to set this flag, other than
>> querying the block status of all blocks at startup? 
> 
> The server may have other ways of knowing this, for instance
> that it has just created the file (*), or that it stat'd the file
> before opening it (not unlikely) and noticed it had 0 allocated
> size. The latter I suspect would be trivial to implement in nbd-server
> 
> (*) = e.g. I had one application where nbd use the export path
> to signify it wanted to open a temporary file, the path consisting
> of a UUID and an encoded length. If the file was not present already
> it created it with ftruncate(). That could trivially have used this.
> 
>> If so, the client could just query this by itself.
> 
> Well there's no currently mainlined extension to do that, but yes
> it could. On the other hand I see no issue passing complete
> zero status back to the client if it's so obvious from a stat().
> 
> -- 
> Alex Bligh
> 
> 
> 
> 

Re: [Qemu-block] [Nbd] [PATCH] doc: Propose NBD_FLAG_INIT_ZEROES extension

[Wouter Verhelst]

On Tue, Dec 13, 2016 at 08:38:12AM +0100, Kevin Wolf wrote:
> Am 12.12.2016 um 19:12 hat Wouter Verhelst geschrieben:
> > I'm not opposed to this proposal, per se, but there seems to be some
> > disagreement (by Kevin, for instance) on whether this extension is at
> > all useful.
> 
> FWIW, I'm not opposed to adding the flag. I don't think it can hurt, but
> I wanted to ask the question so that we don't end up adding a feature
> that noone actually uses. Ultimately it's your call as a maintainer
> anyway how conservative you want to be with spec additions.

I'm not opposed either, but I do agree with you that we shouldn't add
such a feature if it doesn't end up getting used. Especially so if it
burns a flag in the (16-bit) "transmission flags" field, where space is
at a premium.

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
   people in the world who think they really understand all of its rules,
   and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

Re: [Qemu-block] [Nbd] [PATCH] doc: Propose NBD_FLAG_INIT_ZEROES extension

[Kevin Wolf]

Am 12.12.2016 um 19:12 hat Wouter Verhelst geschrieben:
> I'm not opposed to this proposal, per se, but there seems to be some
> disagreement (by Kevin, for instance) on whether this extension is at
> all useful.

FWIW, I'm not opposed to adding the flag. I don't think it can hurt, but
I wanted to ask the question so that we don't end up adding a feature
that noone actually uses. Ultimately it's your call as a maintainer
anyway how conservative you want to be with spec additions.

Kevin

Re: [Qemu-block] [Nbd] [PATCH] doc: Propose NBD_FLAG_INIT_ZEROES extension

[Wouter Verhelst]

Hi Eric,

I'm not opposed to this proposal, per se, but there seems to be some
disagreement (by Kevin, for instance) on whether this extension is at
all useful.

If you can clarify that, I'll be happy to merge it.

On Mon, Dec 05, 2016 at 05:42:35PM -0600, Eric Blake wrote:
> While not directly related to NBD_CMD_WRITE_ZEROES, the qemu
> team discovered that it is useful if a server can advertise
> whether an export is in a known-all-zeroes state at the time
> the client connects.
> 
> Signed-off-by: Eric Blake 
> ---
>  doc/proto.md | 5 +
>  1 file changed, 5 insertions(+)
> 
> This replaces the following qemu patch attempt:
> https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg00357.html
> which tried to add NBD_CMD_HAS_ZERO_INIT with poor semantics. The
> semantics in this proposal should be much better.
> 
> Patch is to the merge of the master branch and the
> extension-write-zeroes branch.  By the way, qemu 2.8 is due
> to be released "real soon now", and implements NBD_CMD_WRITE_ZEROES,
> so maybe it is time to consider promoting the extension-write-zeroes
> branch into master.
> 
> diff --git a/doc/proto.md b/doc/proto.md
> index afe71fc..7e4ec7f 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -697,6 +697,11 @@ The field has the following format:
>the export.
>  - bit 9, `NBD_FLAG_SEND_BLOCK_STATUS`: defined by the experimental
>`BLOCK_STATUS` 
> [extension](https://github.com/NetworkBlockDevice/nbd/blob/extension-blockstatus/doc/proto.md).
> +- bit 10, `NBD_FLAG_INIT_ZEROES`: Indicates that the server guarantees
> +  that at the time transmission phase begins, all offsets within the
> +  export read as zero bytes.  Clients MAY use this information to
> +  avoid writing to sections of the export that should still read as
> +  zero after the client is done writing.
> 
>  Clients SHOULD ignore unknown flags.
> 
> -- 
> 2.9.3
> 
> 
> --
> Developer Access Program for Intel Xeon Phi Processors
> Access to Intel Xeon Phi processor-based developer platforms.
> With one year of Intel Parallel Studio XE.
> Training and support from Colfax.
> Order your platform today.http://sdm.link/xeonphi
> ___
> Nbd-general mailing list
> nbd-gene...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nbd-general
> 

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
   people in the world who think they really understand all of its rules,
   and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12

Re: [Qemu-block] [Nbd] [PATCH] doc: Propose NBD_FLAG_INIT_ZEROES extension

[Alex Bligh]

> On 6 Dec 2016, at 08:46, Alex Bligh  wrote:
> 
> I would support this.
> 
> In fact the patch is sufficiently simple I think I'd merge this
> into extension-write-zeroes then merge that into master.

Hence:

Reviewed-By: Alex Bligh 

-- 
Alex Bligh

Re: [Qemu-block] [Nbd] [PATCH] doc: Propose NBD_FLAG_INIT_ZEROES extension

[Alex Bligh]

> On 6 Dec 2016, at 09:25, Kevin Wolf  wrote:
> 
> Am 06.12.2016 um 00:42 hat Eric Blake geschrieben:
>> While not directly related to NBD_CMD_WRITE_ZEROES, the qemu
>> team discovered that it is useful if a server can advertise
>> whether an export is in a known-all-zeroes state at the time
>> the client connects.
> 
> Does a server usually have the information to set this flag, other than
> querying the block status of all blocks at startup? 

The server may have other ways of knowing this, for instance
that it has just created the file (*), or that it stat'd the file
before opening it (not unlikely) and noticed it had 0 allocated
size. The latter I suspect would be trivial to implement in nbd-server

(*) = e.g. I had one application where nbd use the export path
to signify it wanted to open a temporary file, the path consisting
of a UUID and an encoded length. If the file was not present already
it created it with ftruncate(). That could trivially have used this.

> If so, the client could just query this by itself.

Well there's no currently mainlined extension to do that, but yes
it could. On the other hand I see no issue passing complete
zero status back to the client if it's so obvious from a stat().

-- 
Alex Bligh

Re: [Qemu-block] [Nbd] [PATCH] doc: Propose NBD_FLAG_INIT_ZEROES extension

[Alex Bligh]

> On 5 Dec 2016, at 23:42, Eric Blake  wrote:
> 
> While not directly related to NBD_CMD_WRITE_ZEROES, the qemu
> team discovered that it is useful if a server can advertise
> whether an export is in a known-all-zeroes state at the time
> the client connects.

I think this is good to go, and ...

> Signed-off-by: Eric Blake 
> ---
> doc/proto.md | 5 +
> 1 file changed, 5 insertions(+)
> 
> This replaces the following qemu patch attempt:
> https://lists.gnu.org/archive/html/qemu-devel/2016-12/msg00357.html
> which tried to add NBD_CMD_HAS_ZERO_INIT with poor semantics. The
> semantics in this proposal should be much better.
> 
> Patch is to the merge of the master branch and the
> extension-write-zeroes branch.  By the way, qemu 2.8 is due
> to be released "real soon now", and implements NBD_CMD_WRITE_ZEROES,
> so maybe it is time to consider promoting the extension-write-zeroes
> branch into master.

I would support this.

In fact the patch is sufficiently simple I think I'd merge this
into extension-write-zeroes then merge that into master.

Wouter?

Alex

> diff --git a/doc/proto.md b/doc/proto.md
> index afe71fc..7e4ec7f 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -697,6 +697,11 @@ The field has the following format:
>   the export.
> - bit 9, `NBD_FLAG_SEND_BLOCK_STATUS`: defined by the experimental
>   `BLOCK_STATUS` 
> [extension](https://github.com/NetworkBlockDevice/nbd/blob/extension-blockstatus/doc/proto.md).
> +- bit 10, `NBD_FLAG_INIT_ZEROES`: Indicates that the server guarantees
> +  that at the time transmission phase begins, all offsets within the
> +  export read as zero bytes.  Clients MAY use this information to
> +  avoid writing to sections of the export that should still read as
> +  zero after the client is done writing.
> 
> Clients SHOULD ignore unknown flags.
> 
> -- 
> 2.9.3
> 
> 
> --
> Developer Access Program for Intel Xeon Phi Processors
> Access to Intel Xeon Phi processor-based developer platforms.
> With one year of Intel Parallel Studio XE.
> Training and support from Colfax.
> Order your platform today.http://sdm.link/xeonphi
> ___
> Nbd-general mailing list
> nbd-gene...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nbd-general
> 

-- 
Alex Bligh