Re: [nfsv4] RFC: Is Open/Claim_Delegate_Prev done before/after Reclaim_complete?

[Rick Macklem <[e-mail]>]

On Sun, Apr 7, 2024 at 8:33 AM Rick Macklem <[e-mail]> wrote:
>
> On Sun, Apr 7, 2024 at 7:52 AM David Noveck <[e-mail]> wrote:
> >>
> >>
> >>
> >> The more I think about it, the more I think you are correct, at least
> >> by default.
> I was referring to the case where the client cannot reacquire the delegation,
> such as when the Open/Claim_delegate_prev gets a reply of NFS4ERR_RECLAIM_BAD.
> (Btw, Sec. 15.1.9.4 states "before the server restart or file system
> migration event".
> If we are going to use NFS4ERR_RECLAIM_BAD instead of NFS4ERR_EXPIRED, these
> words will need to be tweaked.)
>
> I am fine with either error return, but the current wording in Sec.
> 15.1.9.4 defines it for
> specific cases that are not this one.
>
> >
> >
> > Not fully correct.  Some of what I told you is wrong and will be corrected in -04.    The current target for submission is 4/23 but it is likely I canpull that forward by a week.   In any case, I hope the following fragment will be helpful:
> >
> > /*
> >  * Includes multiple types of operations:
> >  *
> >  *    - Non-reclaim operations valid independent of grace period status.
> >  *    - Reclaim operations only used during a grace period.
> >  *    - Reclaim operations only used during a special delegation recovery
> >  *      period.
> >  */
> >
> > enum open_claim_type4 {
> >
> >         CLAIM_NULL              = 0,    /* Non-reclaim operation, */
> >         CLAIM_PREVIOUS          = 1,    /* Reclaim operation -- grace
> >                                                                period only. */
> >         CLAIM_DELEGATE_CUR      = 2,    /* Non-reclaim operation. */
> >         CLAIM_DELEGATE_PREV     = 3,    /* Reclaim operation -- special
> >                                                                       delegation recovery period
> >                                                                       only. */
> Sounds correct to me, assuming that "special delegation recovery period" is
> described somewhere. Otherwise I'd just say something like
> "delegation reclaim - not during
> grace period".
>
> >         /*
> >          * Beyond this point, all values are new to v4.1.
> >          */
> >
> >        /*
> >          * Like CLAIM_NULL, but object identified
> >          * by the current filehandle.
> >          */
> >         CLAIM_FH                = 4,    /* Non-reclaim operation. */
> >
> >         /*
> >          * Like CLAIM_DELEGATE_CUR, but object identified
> >          * by current filehandle.
> >          */
> >         CLAIM_DELEG_CUR_FH      = 5,    /* Non-reclaim operation. */
> >
> >         /*
> >          * Like CLAIM_DELEGATE_PREV, but object identified
> >          * by current filehandle.
> >          */
> >         CLAIM_DELEG_PREV_FH     = 6     /* Reclaim operation -- special
> >                                                                       delgation recovery period
> >                                                                        only. */
> > };
> >
> >> Why?
> >> Because when a client crashes/reboots and there is dirty data in the
> >> RAM cache it gets
> >> tossed, so tossing dirty data in the non-volatile cache when it cannot safely
> >> write it back to the server would be consistent with what happens now.
> >
> >
> > Yes, but the idea is to do better than that, when it can be written safely.
> >>
> >>
> >> For the case where the server is in grace, the client can use
> >> Open/Claim_previous
> >> and then, if the reply indicates an immediate recall of the
> >> delegation, it can write the dirty data
> >> back to the server before doing a Delegreturn.
> >
> >
> > True but that is an unusual case.
> >>
> >>
> >> For the case where the server is not in grace and the Open/Claim_delegate_prev
> >> fails, I think there is another way that the client can safely write
> >> the dirty data back.
> >> - If the client keeps track of the server's value of the change
> >> attribute for the file
> >>   in non-volatile storage.
> >>   - The client can try an
> >> Open/Claim_null/Open_share_access_write/Open_share_deny_write
> >>      followed by a Getattr of change in the same compound.
> >>      - if this succeeds and the change attribute is the same as the
> >> stored one, then I think the client can
> >
> >
> > If the  Open/Claim_delegate_prev fails, it would be because the delegation was revoked, in which
> > case, change attribute will be different.
> Why would the change attribute be different? It changes when there is
> a data/metadata change
> done to the file. Typically, a delegation would be revoked (instead of
> recalled) if there is a conflicting Open
> request and the lease has expired. This does not imply a change in the
> change attribute, does it?
> Of course, if any data/metadata changes are applied to the file by
> other clients (such as the one
> that acquired the conflicting open), then change would change.
> Or, I do not understand what you mean by revoke?
>
> >  Also, in that case, checking change attribute and proceding
> > to write is not safe, since the attribute can change after the check.  You need continuous posession
> > of the delegation which is why clain-delege-prev was created.
> Wouldn't a OPEN4_SHARE_DENY_WRITE be sufficient to guarantee that
> other clients are
> not writing to the file during the write-back? Once change is checked
> and found the same after the Open,
> there shouldn't have been any writes done by other clients already
> done and the deny_write should guarantee
> that no other clients write to the file until the open_stateid is
> closed (or there is something ugly
> like a network partitioning/lease expiration). For this case, all the
> client is trying to do is write the dirty data
> back to the server safely, since it was not able to re-acquire the
> delegation. It will no longer be able to use
> the cached data once the open_stateid is closed.
> I have not tried this yet, so there might be issues that I have not
> recognized w.r.t. doing this.
Although I think what I stated above is correct, I now realize that it is
probably not that useful.
Why?
Because all it takes is another client to do a Getattr of change that
triggers a CB_GETATTR to change the value of change on the server.
As such, the likelyhood of that change attribute not being changed is
fairly low.

I am not planning on implementing the above Open with WRITE_ACCESS
and WRITE_DENY for now, rick

>
> I agree that reclaiming the delegation is preferable, since caching of
> the file can continue and
> the write-back does not need to be done right away.
>
> >
> >>        safely write the dirty data back to the server using the open_stateid
> >>        - then close the open_stateid
> >>
> >> An option other than tossing the data would be to flag it and let a
> >> sysadmin/user decide if
> >> merging it back to the server's file makes sense. Ugly but doable when
> >> the dirty data is in
> >> client non-volatile storage.
> >
> >
> > He could decide but I'm not sure on what basis he could decide, especially since telling
> > him all the relevant information could be a security hole a megaparsec wide. :-(
> Well, I wore a sysadmin hat for 30+ years and one of my more enjoyable
> work items
> went something like...
> A prof. would email saying they had made a bunch of changes to a file
> and then lost them.
> Can you get the changes back?
> - The attempt usually involved finding the most recent backup of the
> file, along with stuff
>    like an editor tmp file left around to try and get back what I could.
> So, yes, for the user it could easily be a security issue (it happens
> that this caching mechanism
> uses a separate file for each file being cached, so access to the
> cache for the file could be
> the same as the file), but for a sysadmin it is just a bothersome manual action.
>
> rick
>
> >>
> >>
> >> rick
> >>
> >> >
> >> > --
> >> > Trond Myklebust
> >> > Linux NFS client maintainer, Hammerspace
> >> > [e-mail]
> >> >
> >> >
> >>
> >> _______________________________________________
> >> nfsv4 mailing list
> >> [e-mail]
> >> https://www.ietf.org/mailman/listinfo/nfsv4