Re: [PATCH v7 10/10] NFSD stop queued async copies on client shutdown

[Olga Kornievskaia <aglo@umich.edu>]

On Tue, Apr 10, 2018 at 4:21 PM, J. Bruce Fields <bfields@redhat.com> wrote=
:
> On Tue, Apr 10, 2018 at 04:09:09PM -0400, Olga Kornievskaia wrote:
>> I have a question. I=E2=80=99m 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 =E2=80=9Cerr_delay=E2=80=9D until the copy finishes. That=E2=80=
=99s 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=E2=80=99t 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.