[Cluster-devel] Cluster Project branch, STABLE2, updated. cluster-2.03.02-7-ga6b6a30

[Steven Whitehouse]

Hi,

On Tue, 2008-05-13 at 15:13 -0500, David Teigland wrote:
> On Tue, May 13, 2008 at 09:01:13PM +0100, Steven Whitehouse wrote:
> > Hi,
> > 
> > It might be a silly question, but this looks to me like trying to fix a
> > kernel bug by adding a userland one. Why not simply update the kernel to
> > return the correct value?
> 
> Yes, there's already a kernel fix in dlm.git, see
>   "dlm: fix plock dev_write return value"
> 
> http://git.kernel.org/gitweb.cgi?p=linux/kernel/git/teigland/dlm.git;a=shortlog;h=next
> 
> Suppressing the message spamming is a good solution in the mean time, and
> has a better chance of getting to customers before the kernel patch.
> 
I'm afraid you've still not convinced me on this one. Why not just check
errno as well as the return value, then you can detect both "correct"
instances and still report errors when they occur.

Also the kernel fix doesn't look quite right to me. Surely we should be
reporting the error from dlm_plock_callback() if it occurs, rather than
just ignoring it?

In dlm_plock_callback() the return value from "notify()" seems to be
ignored in one case too. I spotted this while looking at the code:

struct plock_xop {
        struct plock_op xop;
        void *callback;
        void *fl;
        void *file;
        struct file_lock flc;
};

and I can't see the need for void pointers here, why not just use the
correct types? It looks like this code could do with some cleanup,

Steve.