On Tue, Apr 10, 2018 at 05:07:02PM -0400, Olga Kornievskaia wrote: > On Tue, Apr 10, 2018 at 4:21 PM, J. Bruce Fields <bfields@xxxxxxxxxx> wrote: > > On Tue, Apr 10, 2018 at 04:09:09PM -0400, Olga Kornievskaia wrote: > >> I have a question. I’m testing how the code works in > >> nfsd4_shutdown_copy. I have disabled canceling the copy (just > >> returning ok) and so then the client can stop its copy and do the > >> close and then I can issue an unmount. What I see is that the server > >> returns “err_delay” until the copy finishes. That’s because in > >> nfsd4_destroy_clientid() it calls mark_client_expired_locked() which > >> checks the refcount on the client structure and since copy holds a > >> reference server returns err_delay. Once the copy finished and > >> decrements the reference, and nfsd4_destroy_clientid() gets past that > >> then calling nfsd4_shutdown_copy() doesn’t find any copies. > >> > >> Should the logic of nfsd4_destroy_clientid() be changed to stop copies > >> then instead of during destruction of the client structure? > > > > Oh, good question, I don't know. Thinking about it.... > > > > DESTROY_CLIENTID doesn't throw away all the client's state for it, it's > > only meant to be called after the client has already cleaned up > > everything else. So: > > > > https://tools.ietf.org/html/rfc5661#section-18.50.3 > > > > If there are sessions (both idle and non-idle), opens, locks, > > delegations, layouts, and/or wants (Section 18.49) associated > > with the unexpired lease of the client ID, the server MUST > > return NFS4ERR_CLIENTID_BUSY. > > > > My feeling is that "ongoing copies" also belongs on that list. > > > > So the server behavior you're seeing sounds correct to me--the client > > should cancel any ongoing copies before calling DESTROY_CLIENTID. > > If the behavior of returning ERR_DELAY until the copy is done is > correct one, then I don't think I need this patch at all. Since copy > takes a reference on the nfs4_client structure, then in > __destroy_client() where nfsd4_shutdown_copy() is called the list will > always be empty. Actually I guess it should be returning CLIENTID_BUSY. Maybe that's a preexisting bug. We do still need to be able to handle the case where the lease expires. I guess the copy needs to be forcibly canceled in that case. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
