[#LU-104] Lustre grants flock exclusive locks on two file descriptors for the same file

[LU-104] Lustre grants flock exclusive locks on two file descriptors for the same file Created: 28/Feb/11 Updated: 28/Jun/11 Resolved: 03/May/11
Status:	Resolved
Project:	Lustre
Component/s:	None
Affects Version/s:	Lustre 2.1.0
Fix Version/s:	Lustre 2.1.0

Type:

Bug

Priority:

Blocker

Reporter:

Christopher Morrone

Assignee:

Oleg Drokin

Resolution:

Fixed

Votes:

Labels:

None

Severity:

Bugzilla ID:

22,660

Rank (Obsolete):

5058

Description

Lustre grants flock exclusive locks on two file descriptors for the same
file in the same process. However, the second flock SHOULD fail.

Fortunately, the VFS already has the correct behavior. And since we are
already calling the flock_lock_file_wait() or posix_lock_file_wait() VFS
calls, we just need to record that return code and return it in
ll_file_lock().

This patch was already landed on 1.8, needs to land on master.

Comments

Comment by Christopher Morrone [ 28/Feb/11 ]

Patch for head pushed to gerrit:

http://review.whamcloud.com/279

Comment by Oleg Drokin [ 28/Feb/11 ]

the flock (not posix locks you get with fcntl) in lustre is totally broken.
The proper fix for tihs problem that actually addresses the problem at the lustre level + fixes nfs4 locking is located in bug 22040.

Comment by Peter Jones [ 01/Mar/11 ]

Oleg

So, is the patch from bz 22040 something that we could include in Lustre 2.1?

Peter

Comment by Oleg Drokin [ 01/Mar/11 ]

Yes. I think we need in in 2.1. I am still debating with myself if I need to include it verbatim or if I should add compat flag + its check for extra safety.

Comment by Peter Jones [ 01/Mar/11 ]

ok, I have marked it as a blocker and assigned it to you while you ponder. We can reassign to another engineer once you have decided the approach if you prefer

Comment by Christopher Morrone [ 01/Mar/11 ]

I could not really understand what was being said on the phone, so I'll start the conversation here again.

Regardless of how the bug is fixed, I think the test from our patch should be added (or something equivalent). There was no existing test that did what our test added as far as I can see.

Comment by Oleg Drokin [ 01/Mar/11 ]

Ok.
What I was trying to convey was that your test went into flocks_test.c and that one deal with posix locks.
There is also flock_test.c that deals with bsd locks and your test belongs there.
Of course the problem is flock_test.c is never run because bsd locks just don't work correctly in Lustre anyway.

Comment by Christopher Morrone [ 01/Mar/11 ]

Ah, I see. Well, flocks_test.c already does one flock() test (in t1), so it isn't so obvious that it shouldn't have more. Additionally, flocks_test.c has a crude framework for adding multiple tests while flock_test.c does not. Perhaps flock_test.c should just be folded into flocks_test.c and removed.

Or we should clean them up and give them sane names...it would probably be best of all if we had a good way in test suite to easily compile and run individual C regression tests (sort of like autoconf does), so we don't need to piggy-back multiple tests into one C file.

In any event, I'd like our test to appear somewhere that actually gets run.

Comment by Peter Jones [ 15/Mar/11 ]

LLNL confirmed on the weekly 2.1 call that the patch attached to 22040 fixes this issue in their tests

Comment by Peter Jones [ 03/May/11 ]

Landed for 2.1. Please reopen if any further instances of this issue seen