This is a placeholder bug - as a feature request/blocker for 4.2.0 until Volker's leases patch is accepted into master and back-ported.
Created attachment 10394 [details] Current patchset on top of master that supports leases. Adding metze to review list just to remind him :-). Jeremy.
Created attachment 10396 [details] Leases prerequisite patchset (part1) cherry-picked from master. (already pushed) Metze, here is the current leases prerequisite patchset that applies to v4-2-test. I'd like to ensure this gets into the v4-2-test branch now so we can then apply the finished leases patchset on top. If you can +1 this I'll ask Karolin in import this for v4-2-test. Thanks, Jeremy.
Comment on attachment 10396 [details] Leases prerequisite patchset (part1) cherry-picked from master. (already pushed) Looks, Jeremy please add a WHATSNEW.txt update for the new option.
Karolin, can you add the: Leases prerequisite patchset cherry-picked from master. changes to v4-2-test please ? Once you're done re-assign it back to me to get the rest of the leases code in. I'll upload a WHATSNEW change to mention the new option as well. Thanks ! Jeremy.
Created attachment 10397 [details] git-am patch for 4.2.0 WHATSNEW Document "strict rename" in the WHATSNEW.
Created attachment 10399 [details] Current patchset on top of master that supports leases. Fixed compile error when have_read_oplocks got renamed introduced in the patchset that has already gone into master.
Created attachment 10400 [details] Original (working) patch So this is very frustrating. With this original patch, the one that got tested at Mathworks, we pass make test TESTS=samba3.smb2.lease However, with the approved changes into master, and the modified leases patch that removes the already approved changes on top (that is): https://attachments.samba.org/attachment.cgi?id=10399 we get errors in: make test TESTS=samba3.smb2.lease THAT WASN'T SUPPOSED TO HAPPEN ! The idea was that the logic wasn't being changed in the rewrites. Now I have to figure out what got broken...
Of course there are no changes to source4/torture/smb2/leases.c, I checked that :-).
Comment on attachment 10399 [details] Current patchset on top of master that supports leases. Oh hang on a minute, I think I messed up the compile fix for: https://attachments.samba.org/attachment.cgi?id=10399 (that's why I didn't ask for review on it :-). Marking obsolute...
Created attachment 10401 [details] Leases patch on top of master. OK - when updating the additional leases patch for master to fix the compile, I removed an additional "br_lock->num_locks = 0;" statement that caused the open code logic to think all files had outstanding locks. Fixed now, phew :-). Back to passing: make test TESTS=samba3.smb2.lease
(In reply to Jeremy Allison from comment #4) Done, re-assigning.
(In reply to Karolin Seeger from comment #11) Done = pushed to autobuild-v4-2-test.
(In reply to Karolin Seeger from comment #12) Thanks Karolin, now all we need is to get the code into master, and I'll prepare a back-port for 4.2.0. Cheers ! Jeremy.
Created attachment 10406 [details] Found one more case where d->num_leases was indexed by uint16_t, not uint32_t
Comment on attachment 10396 [details] Leases prerequisite patchset (part1) cherry-picked from master. (already pushed) Patch now applied to v4-2-test - remove from default attachments list so we can see what remains to be done.
Comment on attachment 10397 [details] git-am patch for 4.2.0 WHATSNEW Patch now applied to v4-2-test. Mark as obsolete so it remains clear what remains left to be done.
Created attachment 10407 [details] Latest version. Adds 4 new patches to the set: s3: leases : Add smb2_lease_equal() which compares client_guids and keys. s3: leases: Add fsp_client_guid() utility function to return the connected client guid. s3: leases: Ensure we check client_guid as well as lease key to ensure lease identity (using smb2_lease_equal()). s3: leases: Ensure the client guids match when doing a durable reconnect. Should deal with some more of the issues raised in metze's WIP git branch.
Created attachment 10411 [details] Leases patch on top of master. OK - here is the latest patchset. Changes include: (1). Restoring the logic in open_file_ntcreate() around setting lease/oplock state. Code now looks like: if (file_existed) { /* * stat opens on existing files don't get oplocks or leases. * * Note that we check for stat open on the *open_access_mask*, * i.e. the access mask we actually used to do the open, * not the one the client asked for (which is in * fsp->access_mask). This is due to the fact that * FILE_OVERWRITE and FILE_OVERWRITE_IF add in O_TRUNC, * which adds FILE_WRITE_DATA to open_access_mask. */ if (is_stat_open(open_access_mask)) { if (lease) { lease->lease_state = SMB2_LEASE_NONE; } else { oplock_request = NO_OPLOCK; } } } and I removed the code inside grant_fsp_oplock_type() that was doing the same thing based on create_disposition. This logic is now *identical* (modulo leases change) to the logic that is currently in master. (2). Addition of 2 new tests, smb2.oplock test batch9a and raw.oplock test batch9a that are expansion of your test hacks into new test functions. This patchset passes them. (3) Change the error code in downgrade_share_lease() from NT_STATUS_INVALID_PARAMETER to NT_STATUS_REQUEST_NOT_ACCEPTED if the client is responding with a superset of what we allow. Matches Windows return code.
Created attachment 10412 [details] Backport of leases patch on top of v4-2-test.
Created attachment 10415 [details] Leases patch on top of master. Fix compile after commit fbe40d21c8e0f5bc87635e71fb828dfc5479a1ff s3:smbd: fix file corruption using "write cache size != 0" went in.
I will update the v4-2-test back-port once Karolin has merged the previous data corruption patch to v4.2.
Created attachment 10416 [details] WHATSNEW announcement for leases. Karolin, feel free to review but don't push to v4-2-test until the leases code has gone in. Thanks ! Jeremy.
Created attachment 10417 [details] Leases rerequisite patch (part2) for master
Created attachment 10418 [details] Leases rerequisite patch (part2) for master This adds -Werror=return-type to avoid errors like this: ../source3/utils/status.c: In function ‘print_share_mode’: ../source3/utils/status.c:126:3: error: ‘return’ with no value, in function returning non-void [-Werror=return-type] return;
Comment on attachment 10417 [details] Leases rerequisite patch (part2) for master Ok, -Werror=return-type breaks other cases too. We'll add that later...
Comment on attachment 10417 [details] Leases rerequisite patch (part2) for master LGTM. Pushed to master.
Karolin, bouncing this over to you to add attachment 10417 [details] - Leases rerequisite patch (part2) for master to v4-2-test. Once you've done bounce it back to me so I can continue to hound Metze to review the rest of the code :-). Thanks ! Jeremy.
(In reply to Jeremy Allison from comment #27) Karolin, please wait until the individual patches are and master and use git cherry-pick -x for the release branch, thanks!
(In reply to Stefan (metze) Metzmacher from comment #28) I cherry-picked and pushed the Leases prerequisite patches (part2). That's only one patch. So I am not 100% certain if I missed anything. Please check! (All other patches don't apply/don't have a review flag).
Created attachment 10421 [details] WHATSNEW announcement for leases v2 -Move after "Larger IO sizes for SMB2/3 by default" (original patch did not apply (context)) -Change headline (remove "New feature", because it's in the section "NEW FEATURES")
Comment on attachment 10421 [details] WHATSNEW announcement for leases v2 LGTM.
Created attachment 10478 [details] Back-port from master of current leases prerequisites and tests.
Created attachment 10479 [details] Backports for v4-2-test part1
Comment on attachment 10421 [details] WHATSNEW announcement for leases v2 I think we should make it more clear that this is an experimental feature, as we did with SMB2_02 for v3-6
OK, I'll look at the text I used for 3.6 and see if I can adapt it for 4.2.0.
Comment on attachment 10479 [details] Backports for v4-2-test part1 LGTM.
Karolin, can you apply the attachment: "Backports for v4-2-test part1" https://attachments.samba.org/attachment.cgi?id=10479 to the v4-2-test tree please which will sync us up with pre-leases code master ? Once that's done, we'll backport the next (nearly final) part of the leases code if it goes successfully into master via autobuild. Thanks ! Jeremy.
Created attachment 10481 [details] Backports for v4-2-test part2
Comment on attachment 10481 [details] Backports for v4-2-test part2 LGTM. Karolin please push to v4-2-test.
Created attachment 10484 [details] Additional rename test and patch for master. Please review and push to master, then I'll do the cherry-pick for 4.2.
Created attachment 10486 [details] Fixed rename patch for master :-). Fixed version (iterates over leases array, not share mode entry array as we have multiple share mode entries pointing to one lease array entry).
(In reply to Jeremy Allison from comment #39) Pushed Backports for v4-2-test part1 and 2 to autobuild-v4-2-test.
Created attachment 10488 [details] Version 3 of rename patch. Makes SMB2 setinfo async for the SMB2_FILE_RENAME_INFORMATION_INTERNAL call and includes a test that shows why we need it.
Created attachment 10492 [details] Backports for v4-2-test part3 Cherry-pick of code that went into master.
Created attachment 10493 [details] New data model to support rename leases on dynamic shares. What I proposed for master. Implements Metze's new data model for leases.db Please take a look - thanks !
(In reply to Jeremy Allison from comment #44) Karolin, can you push attachment 10492 [details] ( Backports for v4-2-test part3 ) to v4-2-test please ? Then we only need Metze's approval on the new data model patch going into master (or his version of it :-) and a back-port of that code and I think we're good for 4.2.0rc3 ! Cheers, Jeremy.
(In reply to Jeremy Allison from comment #46) Pushed to autobuild-v4-2-test.
Created attachment 10507 [details] Backports for v4-2-test part4 Last chunk that went into master, cheery-picked for 4.2. Once this goes in I think we can close this out !
Karolin, please pick part4 and close the bug...
Comment on attachment 10421 [details] WHATSNEW announcement for leases v2 I think the WHATSNEW.txt is ok for 4.2.0rc3...
(In reply to Stefan (metze) Metzmacher from comment #49) Pushed to autobuild-v4-2-test.
(In reply to Karolin Seeger from comment #51) Pushed to v4-2-test. Closing out bug report! Great work, guys! Thanks! :)
There's a regression in the cluster case where brl_get_locks_readonly_parser() returns uninitialized data.
Created attachment 10546 [details] Backports for v4-2-test part5
Pushed to autobuild-v4-2-test.
(In reply to Stefan (metze) Metzmacher from comment #55) Pushed to v4-2-test