[SSSD] [PATCHES] Remove requests when AD subdomain goes away

Pavel Březina pbrezina at redhat.com
Mon Aug 26 09:39:57 UTC 2013


On 08/08/2013 02:21 PM, Ondrej Kos wrote:
> On 08/08/2013 01:08 AM, Pavel Březina wrote:
>> On 08/05/2013 04:10 PM, Ondrej Kos wrote:
>>> Hi,
>>>
>>> Attached are three patches,
>>>
>>> [PATCH 1/3] Make subdomain refresh period configurable
>>> * Adds the ad_subdomain_refresh_period and ipa_subdomain_refresh_period
>>> configuration options. This isn't needed to be pushed, but I think it
>>> can be beneficial. Also, I needed to write this anyway to work with the
>>> refresh.
>>>
>>> [PATCH 2/3] DP: Store list of back-end tevent requests
>>> * Adds every created request to list, and removes every terminated. This
>>> is to enable iteration through active requests, to fix the issue
>>> addressed in https://fedorahosted.org/sssd/ticket/1968
>>>
>>> [PATCH 3/3] Clean list of domain requests
>>> * fixes https://fedorahosted.org/sssd/ticket/1968
>>> * Goes through the list of tevent requests introduced in previous patch
>>> an those, which match the vanished domain are terminated.
>>>
>>> Ondra
>>
>> Hi,
>>
>>> +void be_clean_dom_req_list(struct be_ctx *be_ctx)
>>> +{
>>> +    struct be_req *be_req;
>>> +    struct sss_domain_info *domain;
>>> +    char *domain_name;
>>> +
>>> +    domain_name = be_ctx->domain->name;
>>> +
>>> +    DEBUG(SSSDBG_TRACE_FUNC,
>>> +            ("Removing requests for domain %s\n", domain_name));
>>> +
>>> +    DLIST_FOR_EACH(be_req, be_ctx->be_reqlist) {
>>> +
>>> +        domain = be_req_get_domain(be_req);
>>> +
>>> +        if (strcmp(domain_name, domain->name) == 0) {
>>> +            be_req_terminate(be_req, DP_ERR_FATAL, EIO, NULL);
>>> +        }
>>> +    }
>>> +}
>>
>>> +struct sss_domain_info *be_req_get_domain(struct be_req *be_req)
>>> +{
>>> +    struct be_ctx *be_ctx = be_req_get_be_ctx(be_req);
>>> +
>>> +    return be_ctx ? be_ctx->domain : NULL;
>>> +}
>>
>>
>> please, correct me if I'm wrong (I haven't test those patches yet), but
>> it won't work as expected. This will always remove *all* be_req, since
>> you're comparing be_ctx->domain with be_req->be_ctx->domain which is the
>> same.
>>
>> You want to compare name of the subdomain that was removed with the
>> actual domain (original domain or one of the subdomain) used in the
>> request - this information is not available at the moment.
>>
>
> Hi,
>
> I don't know how this happened, I probably sent patchset from VM. Actual
> and rebased patches are attached, thanks for noticing this!
>
> Ondra

Hi,

Patch 1: Make subdomain refresh period configurable

You are creating two options:
- ad_subdomain_refresh_period
- ipa_subdomain_refresh_period

I'd merge it into one subdomain_refresh_period. Does anyone else have 
any opinion on this?

Anyway, you put AD option into IPA manpage:
> +++ b/src/man/sssd-ipa.5.xml
> @@ -208,6 +208,19 @@
>                  </varlistentry>
>
>                  <varlistentry>
> +                    <term>ad_subdomain_refresh_period (integer)</term>

Patch 3:
You are calling be_clean_dom_req_list() only in ad subdomains, it needs 
to be called also in ipa.

The main point of this ticket is to safely free sdap_domain structure. 
However, you don't call talloc_free() in these patches :-)


> void be_clean_dom_req_list(struct be_ctx *be_ctx, const char *domain_name)
> {
>     struct be_req *be_req;
>     struct sss_domain_info *domain;
>
>     DEBUG(SSSDBG_TRACE_FUNC,
>             ("Removing requests for domain %s\n", domain_name));
>
>     DLIST_FOR_EACH(be_req, be_ctx->be_reqlist) {
>
>         domain = be_req_get_domain(be_req);
>
>         if (domain == NULL) continue;
>
>         if (strcmp(domain_name, domain->name) == 0) {

be_ctx->domain is always the domain from sssd.conf [domain] section. 
That means that this is a dead code. You need to associate the subdomain 
with be_req and then remove those request that works with this subdomain.

>             be_req_terminate(be_req, DP_ERR_FATAL, EIO, NULL);
>         }
>     }
> }






More information about the sssd-devel mailing list