Subject: Re: [nfsv4] minor NFSv4.2 spec wording issues

From: "Haynes, Tom" <[email-masked]>
Date: Wed, 6 Nov 2013 01:46:37 +0000

On Nov 5, 2013, at 9:55 AM, Christoph Hellwig <[email-masked]> wrote:

> 5.3. claims WRITE is inefficient for transferring sparse files.  

The claim is actually:

   As both READ and WRITE are inefficient for transfer
   of sparse sections of the file, they are marked as OBSOLESCENT in
   NFSv4.2.  Instead, a client should utilize READ_PLUS and WRITE_PLUS.

And the reasoning is that you can READ a hole by transferring it across
the wire as zeros.

Likewise, the server could treat any incoming WRITE of all zeros as
a hole punch.

Both of these are inherently less efficient for transferring holes than
variants which can simply use more metadata information to describe
the intent.

> I don't
> think this is true as sparse files are created by punching holes or
> seeking beyond the file size and thus have no effect.  If I'd be in
> nitpicking mode I'd like to point out that WRITE is slightly more
> efficient than WRITE PLUS for any data write as the protocol overhead is
> slighly slower, although this amount of overhead is probably not
> relevant.
> 
> The last paragraph of 14.7.3.2. claims that WRITE_PLUS must not change
> the space reservation and thus directly contradicts the previous
> paragraph.  The sentence probably just needs an update to only apply
> to the WRITE_PLUS hole punching arm if di_allocated is set to TRUE

Ignore di_allocated for now.

   If used to hole punch, WRITE_PLUS will result in the space_used
   attribute being decreased by the number of bytes that were
   deallocated.  The space_freed attribute may or may not decrease,
   depending on the support and whether the blocks backing the specified
   range were shared or not.  The size attribute will remain unchanged.

^^ This paragraph states that space_used is being decremented. It does not
change the size of the file. Space reservation guarantees are directly
tied to the size.

   The WRITE_PLUS operation MUST NOT change the space reservation
   guarantee of the file.  While the server can deallocate the blocks
   specified by di_offset and di_length, future writes to this region
   MUST NOT fail with NFSERR_NOSPC.

^^ And this simply states that when we deallocate blocks, we must guarantee
that when we next go to use these byte ranges in the file, we will not
see NFSERR_NOSPC.

The main intent in these two paragraphs is that if a file has a space
reservation and blocks have been dedupped, then when we disassociate
the blocks from the file (and the blocks remain in the filesystem), then
we still maintain the space reservation for the file.

> 
> 14.10.3. talks about returning NFS4ERR_UNION_NOTSUPP, but given that the
> server choses which arm to return this error does not seem applicable.

Agreed. I'll fix it up.