[linux-cifs-client] [PATCH 0/2] posix locks behaviour on Windows server

[Jeff Layton]

On Sat, 14 Mar 2009 11:00:24 +0300
Pavel Shilovsky <piastry at etersoft.ru> wrote:

> On Thursday 05 March 2009 22:15:01 you wrote:
> > Pavel Shilovsky wrote:
> > > What is about my patch?
> > >
> > >
> > > --
> > > Best regards,
> > > Pavel Shilovsky.
> >
> > My first reaction was that it added complexity to a mainline path, for a
> > case in which posix behavior was unlikely to be understood by developers
> > (and this odd quirk of posix locking behavior is unlikely to be an app
> > dependency) so would like more convincing evidence that it actually
> > breaks an app before turning it on by default (I am not arguing against
> > implementing posix behaviors in general, but just that "Linux
> > developers" are just as likely to assume the opposite behavior as posix
> > in this case, so this is riskier than it looks to turn on by default).
> >
> > IIRC you noted that Wine depends on this behavior- but was hoping a
> > better way to solve Wine's needs was by passing through the mandatory
> > locking behavior that Windows apps would need (I realize that this has
> > obstacles ... in getting the linux-fsdevel community to understand why
> > headers or minor vfs changes would be needed for this)
> >
> It is connected with some more troubles when we have windows locks behavior on 
> share but posix locks behaviour on other system. That's why special workaround 
> is used in wine, but It isn't a good way, I think.
> 
> Yes, "Linux developers" don't like such posix behaviour, but standard is 
> standard... If we follow it in one things and don't - in another, It can make 
> many problems! It's my opinion, of course.
> 

Some general comments...

I agree with you that we need to follow posix behavior. For better or
worse, it is the standard that Linux is based on and we have to do our
best to match it.
 
The patches were sent inline via Thunderbird which is notorious for
mangling them. Please send them in such a way that it doesn't end up
word-wrapped. Adding the patches to git and using git-send-email is
good for this.

It's nice that this set breaks this code out into separate functions
(cifs has far too many huge functions that need to be broken up), but
there are no comments in any of the new code. That makes it hard to
follow at a glance what all of this code is doing. You might want to
consider adding some so that it's more obvious to newcomers and those
of us who need to review it.

I'll go over the patches in more detail once I have a non-mangled copy.

-- 
Jeff Layton <jlayton at redhat.com>