On 05/19/2011 12:30 PM, J. Bruce Fields wrote:
> On Thu, May 19, 2011 at 11:33:56AM -0400, Bryan Schumaker wrote:
>> On 05/18/2011 06:56 PM, J. Bruce Fields wrote:
>>> On Mon, May 16, 2011 at 03:43:40PM -0400, bjschuma@xxxxxxxxxx wrote:
>>>> +static __be32
>>>> +nfsd4_free_file_stateid(stateid_t *stateid)
>>>> +{
>>>> + struct nfs4_stateid *stp = search_for_stateid(stateid);
>>>> + if (!stp)
>>>> + return nfserr_bad_stateid;
>>>> + if (stateid->si_generation != 0) {
>>>> + if (stateid->si_generation < stp->st_stateid.si_generation)
>>>> + return nfserr_old_stateid;
>>>> + if (stateid->si_generation > stp->st_stateid.si_generation)
>>>> + return nfserr_bad_stateid;
>>>> + }
>>>> +
>>>> + if (check_for_locks(stp->st_file, stp->st_stateowner))
>>>> + return nfserr_locks_held;
>>>
>>> I think this catches a lock stateid, but not an open stateid that has
>>> associated lock stateid's that in turn hold locks.
>>>
>>> Hm, also:
>>>
>>> "The FREE_STATEID operation is used to free a stateid that no
>>> longer has any associated locks (including opens, byte-range
>>> locks, delegations, and layouts)"
>>>
>>> So an open stateid also shouldn't be freeable as long as there are opens
>>> associated with it.
>>
>> So having an open stateid doesn't necessarily mean that the file is open?
>
> Looking back at it.... Sorry, you're right, open stateid's are destroyed
> on close, so like delegation stateid's they should just never be
> freeable.
>
>> and having a lock stateid doesn't mean that the file is locked?
>
> Here we don't know whether the file's locked or not, so we do have to
> check.
>
>> I'll look at making a "check_for_opens()" function to help with this check.
>
> So actually I think it's really simple: always fail opens and
> delegations, but check for locks. (Except I'm not sure if
That is much simpler. I'm glad I asked!
> check_for_locks() does the right things, as it operates on a stateowner
> not a stateid--I'm forgetting how those work.... If there's a unique
> lock stateid per (stateowner,file) pair then check_for_locks() should do
> what you want.)
I'm not sure how they work either. I'll browse through the code to see what I can find.
Thanks!
>
> --b.
>
>>
>>>
>>> Also I guess a client shouldn't be permitted to free a delegation that
>>> it still holds. That means we'll always just return nfserr_locks for
>>> delegation stateid's. I assume free_stateid is only useful in this case
>>
>> Sounds simple enough.
>>
>>> for the case where a server forcibly revokes part of the client's state
>>> and leaves some "stub" stateid's around in place of the revoked ones.
>>>
>>> --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
