[SSSD] [PATCH] Use a single instance of resolver context

Pavel Březina pbrezina at redhat.com
Wed Apr 3 09:39:28 UTC 2013


On 04/02/2013 04:29 PM, Jakub Hrozek wrote:
> On Fri, Mar 29, 2013 at 03:34:18PM +0100, Pavel Březina wrote:
>> On 03/26/2013 10:20 PM, Jakub Hrozek wrote:
>>> Hi,
>>>
>>> these two patches are part of the dyndns update work, but we're working
>>> in the same area with Pavel and it might make sense to send them now to
>>> avoid stepping on one another.
>>>
>>> [PATCH 1/2] Init failover with be_res options
>>> Instead of rereading the resolver parameters from confdb in different
>>> locations, use dp_options structure for the params and define new
>>> structure held by be_ctx that holds both the resolver context and the
>>> option. I think this abstraction makes sense as the resolver doesn't
>>> need to know about the backend etc.
>>>
>>> [PATCH 2/2] Centralize resolv_init, remove resolv context list
>>> Instead of every user of resolver (dyndns, failover, sudo) initializing
>>> its own context, this patch uses the resolver context from be_ctx
>>> everywhere.
>>>
>>> Now that also the support for multiple resolv contexts is redundant in
>>> resolver, it is removed.
>>>
>>> Related tohttps://fedorahosted.org/sssd/ticket/1504
>>
>> Hi,
>> I haven't done a thorough review yet, but everything seems to work
>> fine. But I'd like to pin out one thing that may result in rebasing my
>> patches.
>>
>>> 0001-Init-failover-with-be_res-options.patch
>>
>>> diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
>>> index df077b249df6cf79d0a50c6fe2c28955b63c7316..712d7e7159029d956a4dbc93c905e238b4879a70 100644
>>> --- a/src/providers/data_provider_be.c
>>> +++ b/src/providers/data_provider_be.c
>>> @@ -2490,7 +2490,14 @@ int be_process_init(TALLOC_CTX *mem_ctx,
>>>           goto fail;
>>>       }
>>>
>>> -    ret = be_init_failover(ctx);
>>> +    ret = be_res_init(ctx);
>>> +    if (ret != EOK) {
>>> +        DEBUG(SSSDBG_FATAL_FAILURE,
>>> +              ("fatal error initializing resolver context\n"));
>>> +        goto fail;
>>> +    }
>>> +
>>> +    ret = be_init_failover(ctx, ctx->be_res);
>>
>> Since be_init_failover takes be_ctx as an argument and actively
>> assigns to be_ctx->be_fo, I the second parameter should be avoided.
>
> I put it there to make sure that resolver is initialized at the time
> be_res_init is called.
>
> The attached patch gets rid of the parameter and currently leaves it up
> to the user of the interface to make sure resolver is initialized. In
> the attached patches I turned be_resolv_ctx into a kind of a singleton
> so that be_init_failover initizalizes it on its own but also other
> potential be_res users can call the init function safely.

Hi,
looks like you forgot to ammend be_init_failover declaration. I squashed 
following diff into the first patch.

--- a/src/providers/dp_backend.h
+++ b/src/providers/dp_backend.h

-int be_init_failover(struct be_ctx *ctx, struct be_resolv_ctx *be_res);
+int be_init_failover(struct be_ctx *ctx);

Ack.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Init-failover-with-be_res-options.patch
Type: text/x-patch
Size: 18203 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130403/ed327263/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Centralize-resolv_init-remove-resolv-context-list.patch
Type: text/x-patch
Size: 5383 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130403/ed327263/attachment-0001.bin>


More information about the sssd-devel mailing list