[RFC] [WIP] tevent/glib integration

[Noel Power]

Hi, Metz, Ralf
On 16/12/15 14:08, Stefan Metzmacher wrote:
> Hi Noel,
>
> very interesting work! It would be really good to have this
> problem solved cleanly in future.
[...]
> Instead we can use a tracecallback and hook into the
> TEVENT_TRACE_BEFORE_WAIT (and maybe TEVENT_TRACE_AFTER_WAIT)
> events.
>
> For samba this could be done within smbd_tevent_trace_callback().
>
> In order to make this more generic we could add a helper function
> which can be called in a TEVENT_TRACE_BEFORE_WAIT hook.
>
> This function would maintain some metadata, it needs an array
> of tevent_fd pointers and old array returned by g_main_context_query()
> and have a 2nd array to get the new g_main_context_query() result.
> Then it uses this information to work out which tevent_fd pointers need
> to be added/modified/removed.
so, I played around with the TRACE stuff for a bit, my impression is
that it makes things more awkward and more complicated, unfortunately
trying to do this generically via the trace_callback functionality does
not seem to work easily for a number of reasons

+ imho it is not at all an intuitive api to use, that the fds of the
poll function could be modified by these trace points isn't at all
obvious even by name
+ position of the BEFORE and AFTER events, it seems that these have a
specific purpose (in the smbd at least) for profiling, if we piggy back
on top of these we add quite some extra overhead and that would seem
quite unexpected
+ but.. its not just the overhead of the whatever we call from the trace
events we more than likely also have to move more code (currently
outside) the BEFORE and AFTER events to inbetween it e.g. in the s3
tevent backend at least 'event_add_to_poll_args' and maybe even
'run_events_poll'. In anycase I would prefer to process the foreign
events separately for 2 reasons and that activity also needs to be done
inbetween the 'BEFORE' and 'AFTER' trace points. My reasons for
processing *all* foreign fds at once are
   a) I want the behaviour and results of the poll to be as close as
possible to what the external code would expect if it polled the fds
itself, that means if events for more than one fd are available I want
them all and not one at a time
   b) I think it could be wise to ensure the sequence (add foreign fds,
poll foreign fds, report foreign fd poll results, react to foreign fds)
happens all together without normal 'tevent' events happening inbetween
+ even in the s3 case I would expect that you would not want to enable
glib integration in 'all' smbd processes but instead leave it up to
specific children to do that, the problem then is how do these children
enable this, perhaps there could be some boolean in the
smbd_tevent_trace_state data, however I don't see an easy way for code
in other modules to influence that state without exposing more stuff
from source3/smbd/process.c. Of course child processes could create
their own tracepoint callback (and associated state) but then they need
to be aware of the previous trace callback and would need to
additionally take care to chain the new trackpoint callback to the old
+ the code we call from BEFORE/AFTER be it from trace method or custom
callbacks) needs to transfer information to the loop_wait code at the
very least a timeout, I don't see a way to do this, ok you could pollute
the tevent_context but tevent_context is not available from any public
header
+ is using the the 2 callbacks (BEFORE_WAIT & AFTER_WAIT)  really any
more complex than adding a function to the tevent api to provide
similarly 2 callbacks (but ones with more friendly signatures)

But... I want to avoid more analysis paralysis at this point, my main
goal is to integrate glib for smbd, it's probably wiser to ensure that
this stuff even works before going to anymore trouble (and Ralf
hopefully can help to verify that). With that in mind I am going to
shelve the generic part of this (note: I haven't given up on it... just
thinking let's come back to it) and instead will concentrate on the 's3'
backend, I'll try of course in the implementation to keep a focus on
writing code that could be easily moved out of the backend

Please find attached a new version of the patch, I've removed all the
lib/tevent specific stuff to concentrate on the s3 backend, the
implementation has only changed a little, I am not using the trace point
callbacks but my own custom callbacks (but not exposed externally), I
still use the 'lazy' method of just removing the old and adding the new
fds ;-) (I do intend to fix that)
The example application now uses the 's3' backend and any glib
integration functionality previously in the example has been removed

> Instead of using g_main_context_default(), we should make use of
> g_main_context_new(), g_main_loop_new() and
> g_main_context_{push,pop}_thread_default()

I wonder do I miss something about needing to use the above (note: I
don't have any deep knowledge of glib, I'm pretty clueless about it
really) but it seems to me from reading
https://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html
and the sections referenced from 'Customizing the main loop iteration'
that I am doing the correct thing so I am wondering what the specific
concern is and why I should use those methods

thanks,
Noel


ps - the attached patch is corresponds to the branch
http://cgit.freedesktop.org/~noelp/noelp-samba/log/?h=glib-integration-s3only

and happy Christmas 'cause I am now on holidays till start of January,
not sure if I will get a chance to look further at this until then
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-s3-tevent-backend-for-smbd-with-glib-integration.patch
Type: application/mbox
Size: 19281 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20151221/4e31ece9/0001-s3-tevent-backend-for-smbd-with-glib-integration.mbox>