[PATCH] tevent-glib-glue

[Noel Power]

Replying to this as it bounced from samba-tech (don't have my samba.org
registered there) and I never got it at my work address either, very odd

Noel

On 26/03/2019 08:57, npower wrote:
> Hi Ralf,
>
> On 25/03/2019 15:53, Ralph Böhme wrote:
>> On Mon, Mar 25, 2019 at 12:37:04PM +0000, Noel Power wrote:
>>> On 22/03/2019 15:21, Ralph Böhme wrote: 
> [...]
>>> Also I'd feel safer if glue->skip_glib_refresh was reset inside the 'if
>>> (point == TEVENT_TRACE_AFTER_LOOP_ONCE) ' check, maybe I am being
>>> paranoid but I am not sure for example what way the trace event order
>>> might be if for example the discouraged (but still used) nested tevent
>>> processing happens
>> I'm a review whore, changed the check to match your preferred style.
>> :) I've also added paranoia bit, I think you're right about the nested
>> tevent loop.
> Just to confirm you intentionally are leaving out the the
> glue->skip_glib_refresh reset now ?  (which is fine afaics)
>>> I had a look at the remainder mdssvc changes so
>>>
>>> [PATCH 7/9] s3-mdssvc: add tevent context arg to mds_init_ctx
>>>
>>> [PATCH 8/9] s3-mdssvc: call [un]become_authenticated_pipe_user()
>>>
>>> af as I understand the changes these look good, the
>>> 'become_authenticated_pipe_user' would have been necessary anyway 
>>> regardless of the glue changes. right ?
>> Yup.
>>
>>> [[PATCH 9/9] s3-mdssvc: use tevent_glib_glue in mdssvc RPC service
>>>
>>> These all look fine to me except I confess that I don't understand the
>>> reason for using g_main_context_new() rather than the default context
>>> and then additional use of g_main_context_push_thread_default() From
>>> what I read I understood this is something you may want to do in a
>>> multithreaded environment to ensure gio threads for example to deliver
>>> events to thread default context (and associated main loop) rather than
>>> the global default context. If we are not multitheaded should we just
>>> not use the default global context anyway (and avoid
>>> g_main_context_push/pop_thread_default context calls) To be honest this
>>> stuff is pretty unfamiliar to me so I actually have no clue :-)
>> Same here. Iirc it was needed because otherwise libtracker-sparql
>> wouldn't dispatch the callbacks in the right context. Maybe it was a
>> bug, it's woring now with the default gmain context. Fixed in attached
>> patchset (extra patch).
> yep sounds strange
>> I've added a few more small mdssvc related patches on-top of the
>> patchset.
> all look good
>> Please review&push if happy. Thanks!
> will do
>
> Noel
>
>