On 08/30/2013 10:47 AM, Pavel Březina wrote:
On 08/28/2013 11:01 AM, Ondrej Kos wrote:
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);
        }
    }
}

FYI we talked about this offline and it turned out that I was right :-)

be_req_get_domain(be_req) returns be_req->be_ctx->domain, which is
always the main domain.

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
New patchset attached.

With this, we store the domain name to the request, where available. I'd suggest to file a ticket and store it for every request, but for this purpose it's enough.

Ondra

--
Ondrej Kos
Associate Software Engineer
Identity Management - SSSD
Red Hat Czech
From c90f8c36322f8e8d9e5368f7005a661fff0a97e1 Mon Sep 17 00:00:00 2001
From: Ondrej Kos <o...@redhat.com>
Date: Mon, 5 Aug 2013 14:44:31 +0200
Subject: [PATCH 1/5] Make subdomain refresh period configurable

Adds following config file options:

ad_subdomain_refresh_period
ipa_subdomain_refresh_period

Both are in seconds and have the same usage - setting the refresh period
of AD/IPA subdomains.
---
 src/config/SSSDConfig/__init__.py.in    |  1 +
 src/config/etc/sssd.api.d/sssd-ad.conf  |  1 +
 src/config/etc/sssd.api.d/sssd-ipa.conf |  1 +
 src/man/sssd-ad.5.xml                   | 13 +++++++++++++
 src/man/sssd-ipa.5.xml                  | 13 +++++++++++++
 src/providers/ad/ad_opts.h              |  1 +
 src/providers/ad/ad_subdomains.c        |  9 +++++----
 src/providers/ipa/ipa_opts.h            |  1 +
 src/providers/ipa/ipa_subdomains.c      |  8 +++++---
 src/providers/ldap/ldap_opts.h          |  1 +
 src/providers/ldap/sdap.h               |  1 +
 11 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index f073419e9c0aacdaad4343d05e58a6a2c9a732e2..4e7750129443b5ffcf57d94af26dde7cc0f5856c 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -312,6 +312,7 @@ option_strings = {
     'ldap_initgroups_use_matching_rule_in_chain' : _('Use LDAP_MATCHING_RULE_IN_CHAIN for initgroup lookups'),
     'ldap_min_id' : _('Set lower boundary for allowed IDs from the LDAP server'),
     'ldap_max_id' : _('Set upper boundary for allowed IDs from the LDAP server'),
+    'subdomain_refresh_period': _("How often should be subdomains list refreshed"),
 
     # [provider/ldap/auth]
     'ldap_pwd_policy' : _('Policy to evaluate the password expiration'),
diff --git a/src/config/etc/sssd.api.d/sssd-ad.conf b/src/config/etc/sssd.api.d/sssd-ad.conf
index 120c827523d5b103396b3a38bca8cc75b25d0cc2..63bb63ac94348d0a20595cdfdac372442b5cf5b6 100644
--- a/src/config/etc/sssd.api.d/sssd-ad.conf
+++ b/src/config/etc/sssd.api.d/sssd-ad.conf
@@ -42,6 +42,7 @@ ldap_page_size = int, None, false
 ldap_deref_threshold = int, None, false
 ldap_connection_expire_timeout = int, None, false
 ldap_disable_paging = bool, None, false
+subdomain_refresh_period = int, None, false
 
 [provider/ad/id]
 ldap_search_timeout = int, None, false
diff --git a/src/config/etc/sssd.api.d/sssd-ipa.conf b/src/config/etc/sssd.api.d/sssd-ipa.conf
index bc14fbe3d4153bd7a7ca4ffe0351edf0b8c02ee4..e69c9bc934ef88997be598fcb8cc562eda7aeacb 100644
--- a/src/config/etc/sssd.api.d/sssd-ipa.conf
+++ b/src/config/etc/sssd.api.d/sssd-ipa.conf
@@ -51,6 +51,7 @@ ldap_page_size = int, None, false
 ldap_deref_threshold = int, None, false
 ldap_connection_expire_timeout = int, None, false
 ldap_disable_paging = bool, None, false
+subdomain_refresh_period = int, None, false
 
 [provider/ipa/id]
 ldap_search_timeout = int, None, false
diff --git a/src/man/sssd-ad.5.xml b/src/man/sssd-ad.5.xml
index 4a8c60b49e4ac30788d1f07b50ea05274d1686e5..1ae0d0ec4cf86296892f73845d64e3a34d6934db 100644
--- a/src/man/sssd-ad.5.xml
+++ b/src/man/sssd-ad.5.xml
@@ -164,6 +164,19 @@ ldap_id_mapping = False
                 </varlistentry>
 
                 <varlistentry>
+                    <term>subdomain_refresh_period (integer)</term>
+                    <listitem>
+                        <para>
+                            Optional. Specifies how often should SSSD refresh
+                            the list of subdomains.
+                        </para>
+                        <para>
+                            Default: 14400 (seconds)
+                        </para>
+                    </listitem>
+                </varlistentry>
+
+                <varlistentry>
                     <term>dyndns_update (boolean)</term>
                     <listitem>
                         <para>
diff --git a/src/man/sssd-ipa.5.xml b/src/man/sssd-ipa.5.xml
index 28ac252abbeb508d62ca1a94f2440afc6b5b5c88..4be506b1db845f07008b0ee1066be62b6006bede 100644
--- a/src/man/sssd-ipa.5.xml
+++ b/src/man/sssd-ipa.5.xml
@@ -208,6 +208,19 @@
                 </varlistentry>
 
                 <varlistentry>
+                    <term>subdomain_refresh_period (integer)</term>
+                    <listitem>
+                        <para>
+                            Optional. Specifies how often should SSSD refresh
+                            the list of subdomains.
+                        </para>
+                        <para>
+                            Default: 14400 (seconds)
+                        </para>
+                    </listitem>
+                </varlistentry>
+
+                <varlistentry>
                     <term>dyndns_refresh_interval (integer)</term>
                     <listitem>
                         <para>
diff --git a/src/providers/ad/ad_opts.h b/src/providers/ad/ad_opts.h
index f3b6cd61632601ec92a408a6a73cff6446fd48bf..050c35e1ef72af4375e58971014ec0e6fbe6ff57 100644
--- a/src/providers/ad/ad_opts.h
+++ b/src/providers/ad/ad_opts.h
@@ -126,6 +126,7 @@ struct dp_option ad_def_ldap_opts[] = {
     { "ldap_disable_range_retrieval", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "ldap_min_id", DP_OPT_NUMBER, NULL_NUMBER, NULL_NUMBER},
     { "ldap_max_id", DP_OPT_NUMBER, NULL_NUMBER, NULL_NUMBER},
+    { "subdomain_refresh_period", DP_OPT_NUMBER, {.number = 14400}, NULL_NUMBER}, /* 4 hours */
     DP_OPTION_TERMINATOR
 };
 
diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index afd2031fe6be557f43555b6dd8b47731d9833585..e6d85541d0f93af4ed7dcabce2236c4d225a326e 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -55,9 +55,6 @@
 /* do not refresh more often than every 5 seconds for now */
 #define AD_SUBDOMAIN_REFRESH_LIMIT 5
 
-/* refresh automatically every 4 hours */
-#define AD_SUBDOMAIN_REFRESH_PERIOD (3600 * 4)
-
 struct ad_subdomains_ctx {
     struct be_ctx *be_ctx;
     struct sdap_id_ctx *sdap_id_ctx;
@@ -714,6 +711,7 @@ static void ad_subdom_online_cb(void *pvt)
     struct ad_subdomains_ctx *ctx;
     struct be_req *be_req;
     struct timeval tv;
+    int refresh_period;
 
     ctx = talloc_get_type(pvt, struct ad_subdomains_ctx);
     if (!ctx) {
@@ -730,7 +728,10 @@ static void ad_subdom_online_cb(void *pvt)
 
     ad_subdomains_retrieve(ctx, be_req);
 
-    tv = tevent_timeval_current_ofs(AD_SUBDOMAIN_REFRESH_PERIOD, 0);
+    refresh_period = dp_opt_get_int(ctx->sdap_id_ctx->opts->basic,
+            SDAP_SUBDOMAIN_REFRESH_PERIOD);
+
+    tv = tevent_timeval_current_ofs(refresh_period, 0);
     ctx->timer_event = tevent_add_timer(ctx->be_ctx->ev, ctx, tv,
                                         ad_subdom_timer_refresh, ctx);
     if (!ctx->timer_event) {
diff --git a/src/providers/ipa/ipa_opts.h b/src/providers/ipa/ipa_opts.h
index 5ec36c550b166e07a9ed2f2c31474c55d0ecdaee..7c5d0102e4a2bc2ae9e74670140083e80d4c4ad9 100644
--- a/src/providers/ipa/ipa_opts.h
+++ b/src/providers/ipa/ipa_opts.h
@@ -151,6 +151,7 @@ struct dp_option ipa_def_ldap_opts[] = {
     { "ldap_disable_range_retrieval", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "ldap_min_id", DP_OPT_NUMBER, NULL_NUMBER, NULL_NUMBER},
     { "ldap_max_id", DP_OPT_NUMBER, NULL_NUMBER, NULL_NUMBER},
+    { "subdomain_refresh_period", DP_OPT_NUMBER, {.number = 14400}, NULL_NUMBER}, /* 4 hours */
     DP_OPTION_TERMINATOR
 };
 
diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index c28af0e76a41352b80ab816076de156607243c1d..15d71c6ebfee405ec54e4a5c48e36ee39b9b1c86 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -47,8 +47,6 @@
 /* do not refresh more often than every 5 seconds for now */
 #define IPA_SUBDOMAIN_REFRESH_LIMIT 5
 
-/* refresh automatically every 4 hours */
-#define IPA_SUBDOMAIN_REFRESH_PERIOD (3600 * 4)
 #define IPA_SUBDOMAIN_DISABLED_PERIOD 3600
 
 enum ipa_subdomains_req_type {
@@ -1018,6 +1016,7 @@ static void ipa_subdom_online_cb(void *pvt)
     struct ipa_subdomains_ctx *ctx;
     struct be_req *be_req;
     struct timeval tv;
+    int refresh_period;
 
     ctx = talloc_get_type(pvt, struct ipa_subdomains_ctx);
     if (!ctx) {
@@ -1036,7 +1035,10 @@ static void ipa_subdom_online_cb(void *pvt)
 
     ipa_subdomains_retrieve(ctx, be_req);
 
-    tv = tevent_timeval_current_ofs(IPA_SUBDOMAIN_REFRESH_PERIOD, 0);
+    refresh_period = dp_opt_get_int(ctx->sdap_id_ctx->opts->basic,
+            SDAP_SUBDOMAIN_REFRESH_PERIOD);
+
+    tv = tevent_timeval_current_ofs(refresh_period, 0);
     ctx->timer_event = tevent_add_timer(ctx->be_ctx->ev, ctx, tv,
                                         ipa_subdom_timer_refresh, ctx);
     if (!ctx->timer_event) {
diff --git a/src/providers/ldap/ldap_opts.h b/src/providers/ldap/ldap_opts.h
index a6c821f3ac3ad951a3b45168b298b96fefb96b60..fb024cf87aff050e5af03c9e8e3acef1d2e312ee 100644
--- a/src/providers/ldap/ldap_opts.h
+++ b/src/providers/ldap/ldap_opts.h
@@ -117,6 +117,7 @@ struct dp_option default_basic_opts[] = {
     { "ldap_disable_range_retrieval", DP_OPT_BOOL, BOOL_FALSE, BOOL_FALSE },
     { "ldap_min_id", DP_OPT_NUMBER, NULL_NUMBER, NULL_NUMBER},
     { "ldap_max_id", DP_OPT_NUMBER, NULL_NUMBER, NULL_NUMBER},
+    { "subdomain_refresh_period", DP_OPT_NUMBER, {.number = 14400}, NULL_NUMBER}, /* 4 hours */
     DP_OPTION_TERMINATOR
 };
 
diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
index 441ac904bd2d03618376f6770304dc5c4db75923..8f10300fd622a0bc0bec2879d228a970811a8a59 100644
--- a/src/providers/ldap/sdap.h
+++ b/src/providers/ldap/sdap.h
@@ -221,6 +221,7 @@ enum sdap_basic_opt {
     SDAP_DISABLE_RANGE_RETRIEVAL,
     SDAP_MIN_ID,
     SDAP_MAX_ID,
+    SDAP_SUBDOMAIN_REFRESH_PERIOD,
 
     SDAP_OPTS_BASIC /* opts counter */
 };
-- 
1.8.1.4

From 72d48d112776257aeac08749208dff7f92ca7641 Mon Sep 17 00:00:00 2001
From: Ondrej Kos <o...@redhat.com>
Date: Mon, 5 Aug 2013 12:39:04 +0200
Subject: [PATCH 2/5] DP: Store list of back-end tevent requests

---
 src/providers/data_provider_be.c | 25 +++++++++++++++++++++++++
 src/providers/dp_backend.h       |  3 +++
 2 files changed, 28 insertions(+)

diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index 912b4191c0f6984babf96bb8073db6c01b48afbf..a80bfa136068e593f8afdd5a4b3e52a363a54463 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -125,6 +125,8 @@ static struct bet_data bet_data[] = {
 #define REQ_PHASE_SELINUX 1
 
 struct be_req {
+    struct be_req *prev;
+    struct be_req *next;
     struct be_client *becli;
     struct be_ctx *be_ctx;
     void *req_data;
@@ -166,10 +168,25 @@ void *be_req_get_data(struct be_req *be_req)
     return be_req->req_data;
 }
 
+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;
+}
+
 void be_req_terminate(struct be_req *be_req,
                       int dp_err_type, int errnum, const char *errstr)
 {
+    struct be_ctx *be_ctx = NULL;
+
     if (be_req->fn == NULL) return;
+
+    be_ctx = be_req_get_be_ctx(be_req);
+    if (be_ctx == NULL) return;
+
+    DLIST_REMOVE(be_ctx->be_reqlist, be_req);
+
     be_req->fn(be_req, dp_err_type, errnum, errstr);
 }
 
@@ -274,6 +291,7 @@ static errno_t be_file_request(TALLOC_CTX *mem_ctx,
                                be_req_fn_t fn)
 {
     errno_t ret;
+    struct be_ctx *be_ctx = NULL;
     struct be_async_req *areq;
     struct tevent_timer *te;
     struct timeval tv;
@@ -300,6 +318,13 @@ static errno_t be_file_request(TALLOC_CTX *mem_ctx,
         return EIO;
     }
 
+    be_ctx = be_req_get_be_ctx(be_req);
+    if (be_ctx == NULL) {
+        return EINVAL;
+    }
+
+    DLIST_ADD(be_ctx->be_reqlist, be_req);
+
     return EOK;
 }
 
diff --git a/src/providers/dp_backend.h b/src/providers/dp_backend.h
index 76590a39ef1aa4af5e54254828980e52d31368e2..ff36015fbc139288d4d1fcea465037b49a80164e 100644
--- a/src/providers/dp_backend.h
+++ b/src/providers/dp_backend.h
@@ -145,6 +145,9 @@ struct be_ctx {
     struct be_refresh_ctx *refresh_ctx;
 
     size_t check_online_ref_count;
+
+    struct be_req *be_reqlist;
+
 };
 
 struct bet_ops {
-- 
1.8.1.4

From 57c2a7c4390a1549a2dbe3c6ae25fffd1e6e9f4d Mon Sep 17 00:00:00 2001
From: Ondrej Kos <o...@redhat.com>
Date: Thu, 29 Aug 2013 17:12:34 +0200
Subject: [PATCH 3/5] DP: Save request domain name, if available

---
 src/providers/ad/ad_subdomains.c   |  2 +-
 src/providers/data_provider_be.c   | 31 ++++++++++++++++++++++---------
 src/providers/dp_backend.h         |  2 +-
 src/providers/ipa/ipa_subdomains.c |  2 +-
 4 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index e6d85541d0f93af4ed7dcabce2236c4d225a326e..6e44680e9fc7797a1924cdaff6dfcd60e090a2b5 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -719,7 +719,7 @@ static void ad_subdom_online_cb(void *pvt)
         return;
     }
 
-    be_req = be_req_create(ctx, NULL, ctx->be_ctx,
+    be_req = be_req_create(ctx, NULL, NULL, ctx->be_ctx,
                            ad_subdom_be_req_callback, NULL);
     if (be_req == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("be_req_create() failed.\n"));
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index a80bfa136068e593f8afdd5a4b3e52a363a54463..52dc5e0358656cf93e1a8c47111fa45a667665ba 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -129,6 +129,7 @@ struct be_req {
     struct be_req *next;
     struct be_client *becli;
     struct be_ctx *be_ctx;
+    char *domain_name;
     void *req_data;
 
     be_async_callback_t fn;
@@ -141,7 +142,7 @@ struct be_req {
     int phase;
 };
 
-struct be_req *be_req_create(TALLOC_CTX *mem_ctx,
+struct be_req *be_req_create(TALLOC_CTX *mem_ctx, char *domain_name,
                              struct be_client *becli, struct be_ctx *be_ctx,
                              be_async_callback_t fn, void *pvt_fn_data)
 {
@@ -154,6 +155,12 @@ struct be_req *be_req_create(TALLOC_CTX *mem_ctx,
     be_req->be_ctx = be_ctx;
     be_req->fn = fn;
     be_req->pvt = pvt_fn_data;
+    if (domain_name != NULL) {
+        be_req->domain_name = talloc_strdup(mem_ctx, domain_name);
+        if (!be_req->domain_name) {
+           return NULL;
+        }
+    }
 
     return be_req;
 }
@@ -631,7 +638,7 @@ static int be_get_subdomains(DBusMessage *message, struct sbus_connection *conn)
 
     /* process request */
 
-    be_req = be_req_create(becli, becli, becli->bectx,
+    be_req = be_req_create(becli, NULL, becli, becli->bectx,
                            get_subdomains_callback, reply);
     if (!be_req) {
         err_maj = DP_ERR_FATAL;
@@ -998,7 +1005,7 @@ be_get_account_info_send(TALLOC_CTX *mem_ctx,
                             struct be_get_account_info_state);
     if (!req) return NULL;
 
-    be_req = be_req_create(state, becli, be_ctx,
+    be_req = be_req_create(state, ar->domain, becli, be_ctx,
                            be_get_account_info_done, req);
     if (!be_req) {
         ret = ENOMEM;
@@ -1145,7 +1152,7 @@ static int be_get_account_info(DBusMessage *message, struct sbus_connection *con
          */
     }
 
-    be_req = be_req_create(becli, becli, becli->bectx,
+    be_req = be_req_create(becli, domain, becli, becli->bectx,
                            acctinfo_callback, reply);
     if (!be_req) {
         err_maj = DP_ERR_FATAL;
@@ -1343,7 +1350,7 @@ static int be_pam_handler(DBusMessage *message, struct sbus_connection *conn)
         return ENOMEM;
     }
 
-    be_req = be_req_create(becli, becli, becli->bectx,
+    be_req = be_req_create(becli, NULL, becli, becli->bectx,
                            be_pam_handler_callback, reply);
     if (!be_req) {
         DEBUG(7, ("talloc_zero failed.\n"));
@@ -1367,6 +1374,12 @@ static int be_pam_handler(DBusMessage *message, struct sbus_connection *conn)
             talloc_free(be_req);
             return ENOMEM;
         }
+    } else {
+        be_req->domain_name = talloc_strdup(becli, pd->domain);
+        if (be_req->domain_name == NULL) {
+            talloc_free(be_req);
+            return ENOMEM;
+        }
     }
 
 
@@ -1528,7 +1541,7 @@ static int be_sudo_handler(DBusMessage *message, struct sbus_connection *conn)
     }
 
     /* create be request */
-    be_req = be_req_create(be_cli, be_cli, be_cli->bectx,
+    be_req = be_req_create(be_cli, NULL, be_cli, be_cli->bectx,
                            be_sudo_handler_callback, reply);
     if (be_req == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_zero failed.\n"));
@@ -1764,7 +1777,7 @@ static int be_autofs_handler(DBusMessage *message, struct sbus_connection *conn)
     }
 
     /* create be request */
-    be_req = be_req_create(be_cli, be_cli, be_cli->bectx,
+    be_req = be_req_create(be_cli, NULL, be_cli, be_cli->bectx,
                            be_autofs_handler_callback, reply);
     if (be_req == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("talloc_zero failed.\n"));
@@ -1980,7 +1993,7 @@ static int be_host_handler(DBusMessage *message, struct sbus_connection *conn)
          */
     }
 
-    be_req = be_req_create(becli, becli, becli->bectx,
+    be_req = be_req_create(becli, NULL, becli, becli->bectx,
                            acctinfo_callback, reply);
     if (!be_req) {
         err_maj = DP_ERR_FATAL;
@@ -2256,7 +2269,7 @@ static void check_if_online(struct be_ctx *ctx)
         goto failed;
     }
 
-    be_req = be_req_create(ctx, NULL, ctx,
+    be_req = be_req_create(ctx, NULL, NULL, ctx,
                            check_online_callback, NULL);
     if (be_req == NULL) {
         DEBUG(1, ("talloc_zero failed.\n"));
diff --git a/src/providers/dp_backend.h b/src/providers/dp_backend.h
index ff36015fbc139288d4d1fcea465037b49a80164e..42e03647623d988e18b00722c878afc456f717a5 100644
--- a/src/providers/dp_backend.h
+++ b/src/providers/dp_backend.h
@@ -276,7 +276,7 @@ errno_t be_res_init(struct be_ctx *ctx);
 
 /* be_req helpers */
 
-struct be_req *be_req_create(TALLOC_CTX *mem_ctx,
+struct be_req *be_req_create(TALLOC_CTX *mem_ctx, char *domain_name,
                              struct be_client *becli, struct be_ctx *be_ctx,
                              be_async_callback_t fn, void *pvt_fn_data);
 struct be_ctx *be_req_get_be_ctx(struct be_req *be_req);
diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index 15d71c6ebfee405ec54e4a5c48e36ee39b9b1c86..8c3f176538909f2eed86077bb2fe899239189579 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -1026,7 +1026,7 @@ static void ipa_subdom_online_cb(void *pvt)
 
     ctx->disabled_until = 0;
 
-    be_req = be_req_create(ctx, NULL, ctx->be_ctx,
+    be_req = be_req_create(ctx, NULL, NULL, ctx->be_ctx,
                            ipa_subdom_be_req_callback, NULL);
     if (be_req == NULL) {
         DEBUG(SSSDBG_CRIT_FAILURE, ("be_req_create() failed.\n"));
-- 
1.8.1.4

From 28719c1a91f39b99b6a2e7ec4c9052a6fd41b807 Mon Sep 17 00:00:00 2001
From: Ondrej Kos <o...@redhat.com>
Date: Mon, 5 Aug 2013 13:44:15 +0200
Subject: [PATCH 4/5] Clean list of domain requests

https://fedorahosted.org/sssd/ticket/1968

When subdomain is removed, there still might be some requests pending.
Provided function goes through the list and matching requests are removed.
---
 src/providers/ad/ad_subdomains.c   |  1 +
 src/providers/data_provider_be.c   | 17 +++++++++++++++++
 src/providers/dp_backend.h         |  2 ++
 src/providers/ipa/ipa_subdomains.c |  1 +
 4 files changed, 21 insertions(+)

diff --git a/src/providers/ad/ad_subdomains.c b/src/providers/ad/ad_subdomains.c
index 6e44680e9fc7797a1924cdaff6dfcd60e090a2b5..12965a4fb0d007b90b8b39c07daec67107ec636b 100644
--- a/src/providers/ad/ad_subdomains.c
+++ b/src/providers/ad/ad_subdomains.c
@@ -208,6 +208,7 @@ static errno_t ad_subdomains_refresh(struct ad_subdomains_ctx *ctx,
             }
 
             /* Remove the subdomain from the list of LDAP domains */
+            be_clean_dom_req_list(ctx->be_ctx, dom->name);
             sdap_domain_remove(ctx->sdap_id_ctx->opts, dom);
         } else {
             /* ok let's try to update it */
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c
index 52dc5e0358656cf93e1a8c47111fa45a667665ba..e751b46444a668aa4c0dace2249ccb182d724da1 100644
--- a/src/providers/data_provider_be.c
+++ b/src/providers/data_provider_be.c
@@ -454,6 +454,23 @@ done:
     talloc_free(next_be_req);
 }
 
+void be_clean_dom_req_list(struct be_ctx *be_ctx, const char *domain_name)
+{
+    struct be_req *be_req;
+
+    DEBUG(SSSDBG_TRACE_FUNC,
+            ("Removing requests for domain %s\n", domain_name));
+
+    DLIST_FOR_EACH(be_req, be_ctx->be_reqlist) {
+
+        if (be_req->domain_name == NULL) continue;
+
+        if (strcmp(domain_name, be_req->domain_name) == 0) {
+            be_req_terminate(be_req, DP_ERR_FATAL, EIO, NULL);
+        }
+    }
+}
+
 bool be_is_offline(struct be_ctx *ctx)
 {
     time_t now = time(NULL);
diff --git a/src/providers/dp_backend.h b/src/providers/dp_backend.h
index 42e03647623d988e18b00722c878afc456f717a5..5763c12273aba5b4e373e62c8e83f9c16dda41e5 100644
--- a/src/providers/dp_backend.h
+++ b/src/providers/dp_backend.h
@@ -211,6 +211,8 @@ int be_add_offline_cb(TALLOC_CTX *mem_ctx,
                      struct be_cb **online_cb);
 void be_run_offline_cb(struct be_ctx *be);
 
+void be_clean_dom_req_list(struct be_ctx *be_ctx, const char *domain_name);
+
 /* from data_provider_fo.c */
 enum be_fo_protocol {
     BE_FO_PROTO_TCP,
diff --git a/src/providers/ipa/ipa_subdomains.c b/src/providers/ipa/ipa_subdomains.c
index 8c3f176538909f2eed86077bb2fe899239189579..cf483df5ffda1f2c0d4705c4fd6c99cd8436980d 100644
--- a/src/providers/ipa/ipa_subdomains.c
+++ b/src/providers/ipa/ipa_subdomains.c
@@ -551,6 +551,7 @@ static errno_t ipa_subdomains_refresh(struct ipa_subdomains_ctx *ctx,
             }
 
             /* Remove the AD ID ctx from the list of LDAP domains */
+            be_clean_dom_req_list(ctx->be_ctx, dom->name);
             ipa_ad_subdom_remove(ctx, dom);
         } else {
             /* ok let's try to update it */
-- 
1.8.1.4

From 21ed5ba0910935810f799c05cd0e50e53335077c Mon Sep 17 00:00:00 2001
From: Ondrej Kos <o...@redhat.com>
Date: Fri, 30 Aug 2013 15:55:19 +0200
Subject: [PATCH 5/5] LDAP: free subdomain structure

Resolves:
https://fedorahosted.org/sssd/ticket/1968
---
 src/providers/ldap/ldap_common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/providers/ldap/ldap_common.c b/src/providers/ldap/ldap_common.c
index f7ad71182a4f46a20386e70ffda129347c5e3d87..0bf21d779c3bd1e16963187d35ef5701e7b81f1e 100644
--- a/src/providers/ldap/ldap_common.c
+++ b/src/providers/ldap/ldap_common.c
@@ -177,6 +177,8 @@ sdap_domain_remove(struct sdap_options *opts,
     if (sdom == NULL) return;
 
     DLIST_REMOVE(*(sdom->head), sdom);
+
+    talloc_free(sdom);
 }
 
 int ldap_get_options(TALLOC_CTX *memctx,
-- 
1.8.1.4

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to