Subject: [nfsv4] one umask-02 question

From: Mike Kupfer <[email-masked]>
Date: Thu, 01 Dec 2016 12:10:47 -0800

Section 3 has

>    The server MUST ignore any mode attribute set in the same operation
>    as mode_umask.

I don't understand why the server doesn't return an error in this case.
Silently throwing stuff away makes me nervous.  (If this was already
discussed, feel free to just give me a pointer to the discussion.)

thanks,
mike

---

From: David Noveck <[email-masked]>
Date: Thu, 1 Dec 2016 16:49:18 -0500

> I don't understand why the server doesn't return an error in

I'm not sure either.  Part of the problem may be that it isn't clear what
error to return.  INVAL is ised to inicate an unknown attribute or one used
with the wrong operation (get vs. set) so maybe we would need
NFS4ERR_BADATTRCOMB.

> Silently throwing stuff away makes me nervous.

I can see how that statement would raise red flags.

On the other hand, suppose there were no such statement and a create
contained both mode and umask.  Allowing these to be applied in whatever
order the server chooses is a recipe for non-interoperable
implementations.  The natural thing is to say that if both of these
attributes are present, umask is (MUST be) applied  last, in which case
mode would have no effect.  The effect is the same as saying it MUST be
ignored, but in this case, it is not a directive but a consequence of the
fact that anything it says will be overwritten as part of creating the file.

> (If this was already discussed, feel free to just give me a
> pointer to the discussion.)

D

On Thu, Dec 1, 2016 at 3:10 PM, Mike Kupfer <[email-masked]> wrote:

> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4
>

---

From: "J. Bruce Fields" <[email-masked]>
Date: Thu, 1 Dec 2016 17:01:37 -0500

On Thu, Dec 01, 2016 at 04:49:18PM -0500, David Noveck wrote:
> with the wrong operation (get vs. set)

It has other uses too; e.g. see rfc5661 6.2.5, which is very similar:

	If the mode and mode_set_masked attributes are both specified in
	the same SETATTR, the server MUST also return NFS4ERR_INVAL.

I think INVAL would be fine.

> so maybe we would need NFS4ERR_BADATTRCOMB.
> 

I don't have any strong reason to care whether we require mode to be
ignored (or, equivalently, applied first), or we require INVAL be
returned.  Andreas, am I missing anything?

--b.

---

From: Andreas Gruenbacher <[email-masked]>
Date: Fri, 2 Dec 2016 01:57:43 +0100

> returned.  Andreas, am I missing anything?

Ignoring the mode attribute when mode_umask is set should be the
easiest thing to implement; returning INVAL will always require
additional code that doesn't actually add any benefits. If it makes
anyone feel better, we might just as well require INVAL to be
returned, though.

Thanks,
Andreas

---

From: Mike Kupfer <[email-masked]>
Date: Fri, 02 Dec 2016 12:03:53 -0800

Mike Kupfer wrote:

> Particularly since we already have the precedent of "mode" and
> "mode_set_masked", I recommend returning INVAL.

Of course, after I send this it occurs to me to ask if it's an error to
set mode_set_masked and mode_umask in the same SETATTR.  Sorry, I'm a
little slow here...

mike

---

From: Mike Kupfer <[email-masked]>
Date: Fri, 02 Dec 2016 11:59:46 -0800

Andreas Gruenbacher wrote:

> returning INVAL will always require
> additional code that doesn't actually add any benefits.

The benefit comes in when trying to troubleshoot unexpected behavior.

> If it makes

Thanks.

mike

---

From: David Noveck <[email-masked]>
Date: Fri, 2 Dec 2016 15:27:04 -0500

It is. It is an error to set mode_umask in any SETATTR,  and I'm pretty
sure it is an error to set mode_set_masked in any operation that is not a
SETATTR.

---

From: Andreas Gruenbacher <[email-masked]>
Date: Fri, 2 Dec 2016 21:32:20 +0100

> The benefit comes in when trying to troubleshoot unexpected behavior.

Okay.

Thanks,
Andreas

---

From: "J. Bruce Fields" <[email-masked]>
Date: Fri, 2 Dec 2016 16:16:53 -0500

> > The benefit comes in when trying to troubleshoot unexpected behavior.

OK, I've applied the below locally.  (And I'll fix up the linux server
patch.)

--b.

---

From: "J. Bruce Fields" <[email-masked]>
Date: Fri, 2 Dec 2016 16:20:16 -0500

On Fri, Dec 02, 2016 at 04:16:53PM -0500, J. Bruce Fields wrote:
> OK, I've applied the below locally.

Oops:

diff --git a/umask_middle_attribute.xml b/umask_middle_attribute.xml
index 519c48b9a794..f0923a6969cb 100644
--- a/umask_middle_attribute.xml
+++ b/umask_middle_attribute.xml
@@ -36,8 +36,8 @@
   </t>
 
   <t>
-    The server MUST ignore any mode attribute set in the same operation
-    as mode_umask.
+    The server MUST return NFS4ERR_INVAL if the client attempts to set
+    both mode and mode_umask in the same operation.
   </t>
 
   <t>

---

From: Mike Kupfer <[email-masked]>
Date: Fri, 02 Dec 2016 13:20:38 -0800

David Noveck wrote:

> It is an error to set mode_umask in any SETATTR,  and I'm pretty

Yes, that's right (I found the relevant text in RFC5661).

thanks,
mike