SMB2 compound request create/delete_on_close/close doesn't work as windows
Created attachment 7908 [details] Fix for master.
Comment on attachment 7908 [details] Fix for master. LGTM. Pushed to master. Patch also applies cleanly to 3.6.next.
Karolin please pick for 3.6.next. Thanks ! Jeremy.
Oh hang on a minute... Things are a little more complicated. Working on this (sorry for the confusion Karolin :-). Jeremy.
Comment on attachment 7908 [details] Fix for master. The full patchset will follow
Created attachment 7920 [details] Patches for v4-0-test
Created attachment 7923 [details] Patchset for 3.6 Patchset adapted for v3-6-test.
Metze and I have done these patchsets for v4-0-test and v3-6-test based on Ira's initial patch. Jeremy, would you review them?
Comment on attachment 7920 [details] Patches for v4-0-test Just wanted to let you know I'm still reviewing this. I think I need to understand tevent_req_defer_callback() better. I haven't found any good docs on that. Jeremy.
(In reply to comment #9) > Comment on attachment 7920 [details] > Patches for v4-0-test > > Just wanted to let you know I'm still reviewing this. I think I need to > understand tevent_req_defer_callback() better. I haven't found any good docs on > that. With tevent_req_defer_callback() you setup an event context, that will be used to schedule an immediate event from within tevent_req_done(), tevent_req_error() and wrappers and call tevent_req_notify_callback() from within the immediate event. Normally you have tevent_req_done() and tevent_req_error() trigger the callback inline, which means that after calling them the only valid operation is to call "return", as all your state is likely to be gone! With tevent_req_defer_callback() before tevent_req_done/tevent_req_error you're sure the callback will be triggered later and your state is still valid after calling tevent_req_done/tevent_req_error. From within a cancel function you should typically make use of tevent_req_defer_callback(). And also in shutdown code like smbXcli_conn_disconnect(), where we loop over a list of requests we have to use it. So related to this patchset it mainly replaces the custom immediate events we had. And return the NT_STATUS_CANCELLED from the right layer. Note the 3.6 patchset doesn't include this changes as we don't have tevent_req_defer_callback() in 3.6.
(In reply to comment #10) > (In reply to comment #9) > > Comment on attachment 7920 [details] [details] > > Patches for v4-0-test > > > > Just wanted to let you know I'm still reviewing this. I think I need to > > understand tevent_req_defer_callback() better. I haven't found any good docs on > > that. > > With tevent_req_defer_callback() you setup an event context, > that will be used to schedule an immediate event from > within tevent_req_done(), tevent_req_error() and wrappers > and call tevent_req_notify_callback() from within the immediate event. There's also a doxygen comment for the function, if it's not enough for you please propose a patch to add more once you understand it. /** * @brief Finish multiple requests within one function * * Normally tevent_req_notify_callback() and all wrappers * (e.g. tevent_req_done() and tevent_req_error()) * need to be the last thing an event handler should call. * This is because the callback is likely to destroy the * context of the current function. * * If a function wants to notify more than one caller, * it is dangerous if it just triggers multiple callbacks * in a row. With tevent_req_defer_callback() it is possible * to set an event context that will be used to defer the callback * via an immediate event (similar to tevent_req_post()). * * @code * struct complete_state { * struct tevent_context *ev; * * struct tevent_req **reqs; * }; * * void complete(struct complete_state *state) * { * size_t i, c = talloc_array_length(state->reqs); * * for (i=0; i < c; i++) { * tevent_req_defer_callback(state->reqs[i], state->ev); * tevent_req_done(state->reqs[i]); * } * } * @endcode * * @param[in] req The finished request. * * @param[in] ev The tevent_context for the immediate event. * * @return The given request will be returned. */
Yes I read the doxygen comment, thanks. Once I fully understand it I'll try and update the docs. Thanks metze ! Jeremy.
(In reply to comment #7) > Created attachment 7923 [details] > Patchset for 3.6 > > Patchset adapted for v3-6-test. Karolin, I think this also fixes the smb2.compound error you always had in make test:-)
Comment on attachment 7920 [details] Patches for v4-0-test Ok, *NOW* I understand tevent_req_defer_callback() - it's using an immediate event internal to the req struct via tevent_req_post(). Phew, that is subtle - but really nice code ! Karolin please pick for 4.0.0rc.
Comment on attachment 7923 [details] Patchset for 3.6 Looks good to me.
Re-assigning to Karolin for inclusion in 3.6.next and 4.0.0rc.next Jeremy.
Pushed to autobuild-v4-0-test and v3-6-test. Closing out bug report. Thanks!
(In reply to comment #13) > (In reply to comment #7) > > Created attachment 7923 [details] [details] > > Patchset for 3.6 > > > > Patchset adapted for v3-6-test. > > Karolin, I think this also fixes the smb2.compound error > you always had in make test:-) It does indeed! :-) Thanks a lot, Metze!