On 08/26/2013 11:39 AM, Pavel Březina wrote:
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@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel


--
Ondrej Kos
Associate Software Engineer
Identity Management - SSSD
Red Hat Czech
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to