Re: [Qemu-devel] [PATCH] nbd: Implement NBD_CMD_HAS_ZERO_INIT

[Eric Blake]

On 12/05/2016 05:09 AM, Stefan Hajnoczi wrote:
> On Sun, Dec 04, 2016 at 11:44:57PM +0000, Yi Fang wrote:
>> NBD client has not implemented callback for .bdrv_has_zero_init. So
>> bdrv_has_zero_init always returns 0 when doing non-shared storage
>> migration.
>> This patch implemented NBD_CMD_HAS_ZERO_INIT and will avoid unnecessary
>> set-dirty.
> 
> Please link to the NBD spec where this new command is defined.  Has it
> already been accepted by the NBD community?

No. This is the first I've seen of such a proposal.

> 
> I think this is a weird command because it's information that doesn't
> change over the lifetime of an NBD session.  Your patch sends a command
> and waits for the reply every time has_zero_init() is queried.
> 
> Is there a better place to put this information in the NBD protocols,
> like some export information that is retried during connection?  (I
> haven't checked the protocol.)  It seems weird to send a command and
> wait for the response.

Agreed - this patch is wrong. Even if you DO get the NBD community to
buy into this, it should be done as a new NBD_FLAG_* sent once up-front
during handshake phase in reply to NBD_OPT_EXPORT_NAME/NBD_OPT_GO, and
NOT a new command that can be arbitrarily invoked multiple times during
transmission phase.

Is the goal of the flag for the server to be able to advertise to the
client that upon initial connection, the server asserts that the volume
currently reads as all zeroes (and therefore the client can optimize by
not writing zeroes)?

Do you need me to help re-write this into a proposal to the NBD
community that might stand a chance of being accepted?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature