Latest leases patchset - getting there !

[Jeremy Allison]

On Tue, Nov 11, 2014 at 05:35:21PM +0100, Stefan (metze) Metzmacher wrote:
> Hi Jeremy,
> 
> here're 3 patch files.
> 
> ok12.diff.txt - this is ready to be pushed to master

LGTM - pushed !

> leases12.diff.txt - contains the last patchset with the undo patch
> squashed.
> 
> ontop12.diff.txt - contains fixes for "kernel oplocks = yes" and "level2
> oplocks = no"
> I'll squash them in the next round...
> 
> So please push ok12.diff.txt and make sure the result of all patches
> still works for you.

Yep, checked dirdiff and the changes after leases12.diff.txt are still
minimal - except for the gating change to turn on leases :

reviews-1/source3/smbd/smb2_negprot.c  2014-11-11 11:14:35.778008276 -0800
leases/source3/smbd/smb2_negprot.c     2014-11-11 11:19:06.162889850 -0800

                capabilities |= SMB2_CAP_DFS;
        }

NEW     if (protocol >= PROTOCOL_SMB2_10 &&
NEW         lp_smb2_leases() &&
NEW         lp_oplocks(GLOBAL_SECTION_SNUM) &&
NEW         !lp_kernel_oplocks(GLOBAL_SECTION_SNUM))
NEW     {
ORIG    if (protocol >= PROTOCOL_SMB2_10 && lp_smb2_leases()) {
                capabilities |= SMB2_CAP_LEASING;
        }

In ontop12.diff.txt:

diff --git a/source3/smbd/open.c b/source3/smbd/open.c
index af99f36..838e550 100644
--- a/source3/smbd/open.c
+++ b/source3/smbd/open.c
@@ -1727,6 +1727,10 @@ static NTSTATUS grant_fsp_oplock_type(struct smb_request *req,
        if (oplock_request == LEASE_OPLOCK) {
                granted = lease->lease_state;
 
+               if (lp_kernel_oplocks(SNUM(fsp->conn)) {
+                       DEBUG(10, ("No lease granted because kernel oplocks are enabled\n"));
+                       granted = SMB2_LEASE_NONE;
+               }

is missing a closing parenthesis ')' in:

+               if (lp_kernel_oplocks(SNUM(fsp->conn)) {

In ontop12.diff.txt you have a change:

        if ((granted & SMB2_LEASE_READ) && !(granted & SMB2_LEASE_WRITE)) {
                bool allow_level2 =
                        (global_client_caps & CAP_LEVEL_II_OPLOCKS) &&
                        lp_level2_oplocks(SNUM(fsp->conn));

                if (!allow_level2) {
                        granted = SMB2_LEASE_NONE;
                }
        }

This means that if a client asks for (SMB2_LEASE_HANDLE|SMB2_LEASE_READ)
and "level2 oplocks = no", you also remove SMB2_LEASE_HANDLE when you
remove SMB2_LEASE_READ. Can you add a comment here that

granted = SMB2_LEASE_HANDLE;

only is not a valid lease request type (that's already enforced above in:

                if ((granted & (SMB2_LEASE_READ|SMB2_LEASE_WRITE)) == 0) {
                        DEBUG(10, ("No read or write lease requested\n"));
                        granted = SMB2_LEASE_NONE;
                }

but this makes it clearer (to me at least :-).

Also the change for "level2 oplocks = no" must be documented
in the new "smb2 leases" section in the manpage. Here is a
new version of that patch (attached).

Cheers,

	Jeremy.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s3-param-Add-smb2-leases-parameter.-Default-false.patch
Type: text/x-diff
Size: 4199 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20141111/e556800c/attachment.patch>