[PATCH] F_SETLEASE only on regular files.

[Jeff Layton]

On Sun, 23 Jun 2013 10:31:38 +0200
Andreas Schneider <asn at samba.org> wrote:

> On Sunday, June 23, 2013 09:38:27 you wrote:
> > On Sat, Jun 22, 2013 at 10:48:56AM +0200, Andreas Schneider wrote:
> > > Hello,
> > > 
> > > we should set a lease with
> > > 
> > >     ret = fcntl(fd, F_SETLEASE, leasetype);
> > > 
> > > in source3/smbd/oplock_linux.c only on regular files. Attached is a patch
> > > to do this. I'm not a VFS expert so the questions is if we shouldn't add
> > > errno (EINVAL) handling to functions which call SMB_VFS_LINUX_SETLEASE or
> > > SMB_VFS_NEXT_LINUX_SETLEASE. Maybe at least to not log an error.
> > 
> > Question -- what made you look at this? Is there some real
> > error that you want to fix?
> 
> I got a report cause people saw the error in the log file that setting the 
> lease failed for a pipe.
> 
> So if it is not a regular file we should not report and error but just 
> silently skip it. So it is more about avoiding more of these kind of bug 
> reports.
> 

This patch won't do that though, will it? You'll still return -EINVAL
here and it'll still log the messages. You're just skipping the
SETLEASE attempt. Now that I look too, linux_set_kernel_oplock() has
this:

        if ( SMB_VFS_LINUX_SETLEASE(fsp, F_WRLCK) == -1) {
                DEBUG(3,("linux_set_kernel_oplock: Refused oplock on file %s, "
                         "fd = %d, file_id = %s. (%s)\n",
                         fsp_str_dbg(fsp), fsp->fh->fd,
                         file_id_string_tos(&fsp->file_id),
                         strerror(errno)));
                return False;
        }


...so this is being logged at debug level 3. I think we should just
close that request as NOTABUG. It seems entirely appropriate to be
logging these messages at a DEBUG level.

-- 
Jeff Layton <jlayton at samba.org>