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.
I don't see what you're implying here.
*be_ctx->domain* is not used here at all
*be_ctx->be_reqlist* should contain all request for the provider
*domain_name* is name of subdomain which is getting terminated
*domain* is subdomain info fetched from request which is being processed
in current iteration through the request list
> be_req_terminate(be_req, DP_ERR_FATAL, EIO, NULL);
> }
> }
> }
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
--
Ondrej Kos
Associate Software Engineer
Identity Management - SSSD
Red Hat Czech