Allow backends to set a callback in the be_ctx that should be invoked when the ID provider goes online.
This can be used to perform regular maintenance tasks that are valid only when going online.
Specifically, we can use this to perform a deferred kinit on behalf of the user when we go online. We can also use this to trigger a dynamic DNS update for the IPA provider.
On 04/27/2010 03:37 PM, Stephen Gallagher wrote:
Allow backends to set a callback in the be_ctx that should be invoked when the ID provider goes online.
This can be used to perform regular maintenance tasks that are valid only when going online.
Specifically, we can use this to perform a deferred kinit on behalf of the user when we go online. We can also use this to trigger a dynamic DNS update for the IPA provider.
I forgot to mention: I didn't implement this callback for the proxy ID provider, because it's basically impossible to identify when it is actually going online.
On Tue, 27 Apr 2010 15:40:36 -0400 Stephen Gallagher sgallagh@redhat.com wrote:
I forgot to mention: I didn't implement this callback for the proxy ID provider, because it's basically impossible to identify when it is actually going online.
you can mark it online when it replies successfully to a call.
Simo.
The point of this callback is that it should only fire when transitioning from offline to online. We don't have a way at present to tell the difference in proxy.
On Apr 27, 2010, at 4:22 PM, Simo Sorce ssorce@redhat.com wrote:
On Tue, 27 Apr 2010 15:40:36 -0400 Stephen Gallagher sgallagh@redhat.com wrote:
I forgot to mention: I didn't implement this callback for the proxy ID provider, because it's basically impossible to identify when it is actually going online.
you can mark it online when it replies successfully to a call.
Simo.
-- Simo Sorce * Red Hat, Inc * New York _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
On Tue, Apr 27, 2010 at 03:37:02PM -0400, Stephen Gallagher wrote:
Allow backends to set a callback in the be_ctx that should be invoked when the ID provider goes online.
This can be used to perform regular maintenance tasks that are valid only when going online.
Specifically, we can use this to perform a deferred kinit on behalf of the user when we go online. We can also use this to trigger a dynamic DNS update for the IPA provider.
-- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
}
- /* Reconnection succeeded
* Run any post-connection routines*/- if (state->be->online_cb_list) {
DLIST_FOR_EACH(callback, state->be->online_cb_list) {ret = callback->cb(callback->pvt);if (ret != EOK) {DEBUG(0, ("Post-connection callback returned [%d][%s]",ret, strerror(ret)));tevent_req_error(req, ret);break;}}- }
I have several comments here: - I would suggest to move this loop into a separate subroutine like be_run_online_cb() - Maybe it would make sense to call the callbacks with a timer event to make sure that they do not block the current request or other callbacks - Currently the callbacks are run on every reconnect which might be intended, but is IMO different from running after the transition from offline to online. E.g. with GSSAPI we need to reconnect if the ticket becomes invalid although we haven't been offline. To catch only the offline-online transition I would suggest to add a run_online_cb flag which is set by be_mark_offline() and checked and unset by the proposed be_run_online_cb(). If the callbacks should be run after every successful connection it would be better to replace '*online_cb*' by something like '*connect_cb*'. - Is it intended that only the id provider is running the callbacks?
bye, Sumit
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
On 04/28/2010 06:04 AM, Sumit Bose wrote:
On Tue, Apr 27, 2010 at 03:37:02PM -0400, Stephen Gallagher wrote:
Allow backends to set a callback in the be_ctx that should be invoked when the ID provider goes online.
This can be used to perform regular maintenance tasks that are valid only when going online.
Specifically, we can use this to perform a deferred kinit on behalf of the user when we go online. We can also use this to trigger a dynamic DNS update for the IPA provider.
-- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
}
- /* Reconnection succeeded
* Run any post-connection routines*/- if (state->be->online_cb_list) {
DLIST_FOR_EACH(callback, state->be->online_cb_list) {ret = callback->cb(callback->pvt);if (ret != EOK) {DEBUG(0, ("Post-connection callback returned [%d][%s]",ret, strerror(ret)));tevent_req_error(req, ret);break;}}- }
I have several comments here:
- I would suggest to move this loop into a separate subroutine like be_run_online_cb()
Yeah, that would make sense. Then we could expose that routine.
- Maybe it would make sense to call the callbacks with a timer event to make sure that they do not block the current request or other callbacks
I was going back and forth on this. As you said, the risk is that it could cause a block. So I think I'll do as you suggest and make it a stepped series of timer events. The catch is that return values from the callbacks won't affect the request they're attached to.
- Currently the callbacks are run on every reconnect which might be intended, but is IMO different from running after the transition from offline to online. E.g. with GSSAPI we need to reconnect if the ticket becomes invalid although we haven't been offline. To catch only the offline-online transition I would suggest to add a run_online_cb flag which is set by be_mark_offline() and checked and unset by the proposed be_run_online_cb().
Good idea, I didn't think of that. I also wasn't thinking of the GSSAPI case. Doing it this way would also allow me to make these callbacks available to the proxy provider.
If the callbacks should be run after
every successful connection it would be better to replace '*online_cb*' by something like '*connect_cb*'.
- Is it intended that only the id provider is running the callbacks?
This was mostly an optimization. The ID provider must be online for any of the other providers to actually work (since they will all verify the user before proceeding). So it's not actually possible in our current design for anything but the ID provider to cause the transition from offline to online operation. (Obviously the reverse transition can occur in any of them)
Completely rewritten design attached.
On 04/28/2010 07:57 AM, Stephen Gallagher wrote:
On 04/28/2010 06:04 AM, Sumit Bose wrote:
On Tue, Apr 27, 2010 at 03:37:02PM -0400, Stephen Gallagher wrote:
Allow backends to set a callback in the be_ctx that should be invoked when the ID provider goes online.
This can be used to perform regular maintenance tasks that are valid only when going online.
Specifically, we can use this to perform a deferred kinit on behalf of the user when we go online. We can also use this to trigger a dynamic DNS update for the IPA provider.
-- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
}
- /* Reconnection succeeded
* Run any post-connection routines*/- if (state->be->online_cb_list) {
DLIST_FOR_EACH(callback, state->be->online_cb_list) {ret = callback->cb(callback->pvt);if (ret != EOK) {DEBUG(0, ("Post-connection callback returned [%d][%s]",ret, strerror(ret)));tevent_req_error(req, ret);break;}}- }
I have several comments here:
- I would suggest to move this loop into a separate subroutine like be_run_online_cb()
Yeah, that would make sense. Then we could expose that routine.
- Maybe it would make sense to call the callbacks with a timer event to make sure that they do not block the current request or other callbacks
I was going back and forth on this. As you said, the risk is that it could cause a block. So I think I'll do as you suggest and make it a stepped series of timer events. The catch is that return values from the callbacks won't affect the request they're attached to.
- Currently the callbacks are run on every reconnect which might be intended, but is IMO different from running after the transition from offline to online. E.g. with GSSAPI we need to reconnect if the ticket becomes invalid although we haven't been offline. To catch only the offline-online transition I would suggest to add a run_online_cb flag which is set by be_mark_offline() and checked and unset by the proposed be_run_online_cb().
Good idea, I didn't think of that. I also wasn't thinking of the GSSAPI case. Doing it this way would also allow me to make these callbacks available to the proxy provider.
If the callbacks should be run after
every successful connection it would be better to replace '*online_cb*' by something like '*connect_cb*'.
- Is it intended that only the id provider is running the callbacks?
This was mostly an optimization. The ID provider must be online for any of the other providers to actually work (since they will all verify the user before proceeding). So it's not actually possible in our current design for anything but the ID provider to cause the transition from offline to online operation. (Obviously the reverse transition can occur in any of them)
Simo made some excellent suggestions on IRC, so I have incorporated them:
First, I changed the interface for be_add_online_cb() so that it will now take a mem_ctx and pass back (if requested) the online_cb object. (This object is now opaque). This way, the callback will be automatically freed if the parent context goes away for any reason.
I removed the be_remove_online_cb() function, so the interface to remove a callback will simply be to call talloc_free() on the opaque object returned from be_add_online_cb. This will allow us to potentially set the same callback function multiple times, with different private data.
New patch attached.
On 04/28/2010 02:57 PM, Stephen Gallagher wrote:
Simo made some excellent suggestions on IRC, so I have incorporated them:
First, I changed the interface for be_add_online_cb() so that it will now take a mem_ctx and pass back (if requested) the online_cb object. (This object is now opaque). This way, the callback will be automatically freed if the parent context goes away for any reason.
I removed the be_remove_online_cb() function, so the interface to remove a callback will simply be to call talloc_free() on the opaque object returned from be_add_online_cb. This will allow us to potentially set the same callback function multiple times, with different private data.
New patch attached.
Fix some style issues with the previous patch.
On Wed, Apr 28, 2010 at 03:26:29PM -0400, Stephen Gallagher wrote:
On 04/28/2010 02:57 PM, Stephen Gallagher wrote:
Simo made some excellent suggestions on IRC, so I have incorporated them:
First, I changed the interface for be_add_online_cb() so that it will now take a mem_ctx and pass back (if requested) the online_cb object. (This object is now opaque). This way, the callback will be automatically freed if the parent context goes away for any reason.
I removed the be_remove_online_cb() function, so the interface to remove a callback will simply be to call talloc_free() on the opaque object returned from be_add_online_cb. This will allow us to potentially set the same callback function multiple times, with different private data.
New patch attached.
Fix some style issues with the previous patch.
-- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
+static int online_cb_destructor(TALLOC_CTX *ptr); +int be_add_online_cb(TALLOC_CTX *mem_ctx,
struct be_ctx *ctx,be_conn_online_callback_t cb,void *pvt,struct be_conn_online_cb **online_cb)+{
- struct be_conn_online_cb *on_cb;
- if (!ctx || !cb) {
return EINVAL;- }
- on_cb = talloc(mem_ctx, struct be_conn_online_cb);
- if (!online_cb) {
if (!on_cb) ??
bye, Sumit
return ENOMEM;- }
- on_cb->cb = cb;
- on_cb->pvt = pvt;
- on_cb->be = ctx;
- DLIST_ADD(ctx->online_cb_list, on_cb);
- talloc_set_destructor((TALLOC_CTX *)on_cb, online_cb_destructor);
- /* Make sure we run the callback for the first
* connection after startup.*/- ctx->run_online_cb = true;
- if (online_cb) {
*online_cb = on_cb;- }
- return EOK;
+}
On 04/29/2010 05:52 AM, Sumit Bose wrote:
+static int online_cb_destructor(TALLOC_CTX *ptr); +int be_add_online_cb(TALLOC_CTX *mem_ctx,
struct be_ctx *ctx,be_conn_online_callback_t cb,void *pvt,struct be_conn_online_cb **online_cb)+{
- struct be_conn_online_cb *on_cb;
- if (!ctx || !cb) {
return EINVAL;- }
- on_cb = talloc(mem_ctx, struct be_conn_online_cb);
- if (!online_cb) {
if (!on_cb) ??
bye, Sumit
Thanks for catching that. When I added the **online_cb return, I changed the name of the internal struct to be on_cb, but I missed that use of it. That would have been a bug, since it would have returned on error if called without expecting a returned struct.
New patch attached.
On 04/29/2010 07:41 AM, Stephen Gallagher wrote:
On 04/29/2010 05:52 AM, Sumit Bose wrote:
+static int online_cb_destructor(TALLOC_CTX *ptr); +int be_add_online_cb(TALLOC_CTX *mem_ctx,
- struct be_ctx *ctx,
- be_conn_online_callback_t cb,
- void *pvt,
- struct be_conn_online_cb **online_cb)
+{
- struct be_conn_online_cb *on_cb;
- if (!ctx || !cb) {
- return EINVAL;
- }
- on_cb = talloc(mem_ctx, struct be_conn_online_cb);
- if (!online_cb) {
if (!on_cb) ??
bye, Sumit
Thanks for catching that. When I added the **online_cb return, I changed the name of the internal struct to be on_cb, but I missed that use of it. That would have been a bug, since it would have returned on error if called without expecting a returned struct.
New patch attached.
Rebased to current sssd-1-2.
On Mon, May 03, 2010 at 03:41:18PM -0400, Stephen Gallagher wrote:
On 04/29/2010 07:41 AM, Stephen Gallagher wrote:
On 04/29/2010 05:52 AM, Sumit Bose wrote:
+static int online_cb_destructor(TALLOC_CTX *ptr); +int be_add_online_cb(TALLOC_CTX *mem_ctx,
- struct be_ctx *ctx,
- be_conn_online_callback_t cb,
- void *pvt,
- struct be_conn_online_cb **online_cb)
+{
- struct be_conn_online_cb *on_cb;
- if (!ctx || !cb) {
- return EINVAL;
- }
- on_cb = talloc(mem_ctx, struct be_conn_online_cb);
- if (!online_cb) {
if (!on_cb) ??
bye, Sumit
Thanks for catching that. When I added the **online_cb return, I changed the name of the internal struct to be on_cb, but I missed that use of it. That would have been a bug, since it would have returned on error if called without expecting a returned struct.
New patch attached.
Rebased to current sssd-1-2.
ACK
bye, Sumit
-- Stephen Gallagher RHCE 804006346421761
Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/
From 5aaf82cdf4838105dab6e7601ead134a5309f89f Mon Sep 17 00:00:00 2001 From: Stephen Gallagher sgallagh@redhat.com Date: Tue, 27 Apr 2010 14:57:37 -0400 Subject: [PATCH 1/3] Add callback when the ID provider switches from offline to online
Allow backends to set a callback in the be_ctx that should be invoked when the ID provider goes online.
This can be used to perform regular maintenance tasks that are valid only when going online.
src/providers/data_provider_be.c | 138 ++++++++++++++++++++++++++++ src/providers/dp_backend.h | 17 ++++ src/providers/ldap/sdap_async_connection.c | 9 ++ src/providers/proxy.c | 10 ++ 4 files changed, 174 insertions(+), 0 deletions(-)
diff --git a/src/providers/data_provider_be.c b/src/providers/data_provider_be.c index 22f18024858a73da8788ad93dcc863a1c3222069..320821ae41daf1f7a64ba69aac0e8f0d91c3e5f1 100644 --- a/src/providers/data_provider_be.c +++ b/src/providers/data_provider_be.c @@ -166,8 +166,146 @@ void be_mark_offline(struct be_ctx *ctx)
ctx->offstat.went_offline = time(NULL); ctx->offstat.offline = true;
- ctx->run_online_cb = true;
}
+struct be_conn_online_cb {
- struct be_conn_online_cb *prev;
- struct be_conn_online_cb *next;
- be_conn_online_callback_t cb;
- void *pvt;
- struct be_ctx *be;
+};
+static int online_cb_destructor(TALLOC_CTX *ptr); +int be_add_online_cb(TALLOC_CTX *mem_ctx,
struct be_ctx *ctx,be_conn_online_callback_t cb,void *pvt,struct be_conn_online_cb **online_cb)+{
- struct be_conn_online_cb *on_cb;
- if (!ctx || !cb) {
return EINVAL;- }
- on_cb = talloc(mem_ctx, struct be_conn_online_cb);
- if (!on_cb) {
return ENOMEM;- }
- on_cb->cb = cb;
- on_cb->pvt = pvt;
- on_cb->be = ctx;
- DLIST_ADD(ctx->online_cb_list, on_cb);
- talloc_set_destructor((TALLOC_CTX *)on_cb, online_cb_destructor);
- /* Make sure we run the callback for the first
* connection after startup.*/- ctx->run_online_cb = true;
- if (online_cb) {
*online_cb = on_cb;- }
- return EOK;
+}
+static int online_cb_destructor(TALLOC_CTX *ptr) +{
- struct be_conn_online_cb *cb =
talloc_get_type(ptr, struct be_conn_online_cb);- DLIST_REMOVE(cb->be->online_cb_list, cb);
- return 0;
+}
+struct be_online_cb_ctx {
- struct be_ctx *be;
- struct be_conn_online_cb *callback;
+};
+static void be_run_online_cb_step(struct tevent_context *ev,
struct tevent_timer *te,struct timeval current_time,void *pvt);+void be_run_online_cb(struct be_ctx *be) {
- struct timeval soon;
- struct tevent_timer *te;
- struct be_online_cb_ctx *cb_ctx;
- if (be->run_online_cb && be->online_cb_list) {
/* Reset the flag. We only want to run these* callbacks when transitioning to online*/be->run_online_cb = false;DEBUG(3, ("Going online. Running callbacks.\n"));cb_ctx = talloc(be, struct be_online_cb_ctx);if (!cb_ctx) {DEBUG(0, ("Out of memory. Could not invoke callbacks\n"));return;}cb_ctx->be = be;cb_ctx->callback = be->online_cb_list;/* Delay 30ms so we don't block any other events */soon = tevent_timeval_current_ofs(0, 30000);te = tevent_add_timer(be->ev, cb_ctx, soon,be_run_online_cb_step,cb_ctx);if (!te) {DEBUG(0, ("Out of memory. Could not invoke callbacks\n"));talloc_free(cb_ctx);}- }
+}
+static void be_run_online_cb_step(struct tevent_context *ev,
struct tevent_timer *te,struct timeval current_time,void *pvt)+{
- struct be_online_cb_ctx *cb_ctx =
talloc_get_type(pvt, struct be_online_cb_ctx);- struct tevent_timer *tev;
- struct timeval soon;
- /* Call the callback */
- cb_ctx->callback->cb(cb_ctx->callback->pvt);
- if (cb_ctx->callback->next) {
cb_ctx->callback = cb_ctx->callback->next;/* Delay 30ms so we don't block any other events */soon = tevent_timeval_current_ofs(0, 30000);tev = tevent_add_timer(cb_ctx->be->ev, cb_ctx, soon,be_run_online_cb_step,cb_ctx);if (!te) {DEBUG(0, ("Out of memory. Could not invoke callbacks\n"));goto final;}return;- }
+final:
- /* Steal the timer event onto the be_ctx so it doesn't
* get freed with the cb_ctx*/- talloc_steal(cb_ctx->be, te);
- talloc_free(cb_ctx);
+}
static int be_check_online(DBusMessage *message, struct sbus_connection *conn) { struct be_client *becli; diff --git a/src/providers/dp_backend.h b/src/providers/dp_backend.h index f1069d0db313ca278e6125908668e2aad2c25b76..330f0f03b7b107cfbe272c92afed83e687f32d70 100644 --- a/src/providers/dp_backend.h +++ b/src/providers/dp_backend.h @@ -35,6 +35,8 @@ typedef void (*be_shutdown_fn)(void *); typedef void (*be_req_fn_t)(struct be_req *); typedef void (*be_async_callback_t)(struct be_req *, int, int, const char *);
+typedef void (*be_conn_online_callback_t)(void *);
enum bet_type { BET_NULL = 0, BET_ID, @@ -76,6 +78,8 @@ struct be_client {
struct be_failover_ctx;
+struct be_conn_online_cb;
struct be_ctx { struct tevent_context *ev; struct confdb_ctx *cdb; @@ -85,6 +89,12 @@ struct be_ctx { const char *conf_path; struct be_failover_ctx *be_fo;
/* Functions to be invoked when the
* backend goes online*/struct be_conn_online_cb *online_cb_list;
bool run_online_cb;
struct be_offline_status offstat;
struct sbus_connection *mon_conn;
@@ -122,6 +132,13 @@ struct be_acct_req { bool be_is_offline(struct be_ctx *ctx); void be_mark_offline(struct be_ctx *ctx);
+int be_add_online_cb(TALLOC_CTX *mem_ctx,
struct be_ctx *ctx,be_conn_online_callback_t cb,void *pvt,struct be_conn_online_cb **online_cb);+void be_run_online_cb(struct be_ctx *be);
/* from data_provider_fo.c */ typedef void (be_svc_callback_fn_t)(void *, struct fo_server *);
diff --git a/src/providers/ldap/sdap_async_connection.c b/src/providers/ldap/sdap_async_connection.c index 9871dc2109d2c0eac00f95401ba5dae2d16486ce..214c58efb63d71580f64dce95c8a9a1bec302581 100644 --- a/src/providers/ldap/sdap_async_connection.c +++ b/src/providers/ldap/sdap_async_connection.c @@ -813,6 +813,7 @@ struct sdap_cli_connect_state { struct tevent_context *ev; struct sdap_options *opts; struct sdap_service *service;
struct be_ctx *be;
bool use_rootdse; struct sysdb_attrs *rootdse;
@@ -848,6 +849,7 @@ struct tevent_req *sdap_cli_connect_send(TALLOC_CTX *memctx, state->opts = opts; state->service = service; state->srv = NULL;
state->be = be;
if (rootdse) { state->use_rootdse = true;
@@ -1077,6 +1079,8 @@ static void sdap_cli_auth_done(struct tevent_req *subreq) { struct tevent_req *req = tevent_req_callback_data(subreq, struct tevent_req);
- struct sdap_cli_connect_state *state = tevent_req_data(req,
struct sdap_cli_connect_state);@@ -1091,6 +1095,11 @@ static void sdap_cli_auth_done(struct tevent_req *subreq) return; }
- /* Reconnection succeeded
* Run any post-connection routines*/- be_run_online_cb(state->be);
- tevent_req_done(req);
}
diff --git a/src/providers/proxy.c b/src/providers/proxy.c index 09a2555c5c509ee837019c69ad2c2ddd9f9d01d5..d3ec7ef35361300fb6e7d708e1f91d9b230eeb4a 100644 --- a/src/providers/proxy.c +++ b/src/providers/proxy.c @@ -296,6 +296,16 @@ static void proxy_pam_handler_cache_done(struct tevent_req *subreq) static void proxy_reply(struct be_req *req, int dp_err, int error, const char *errstr) {
- if (!req->be_ctx->offstat.offline) {
/* This action took place online.* Fire any online callbacks if necessary.* Note: we're checking the offline value directly,* because if the activity took a long time to* complete, calling be_is_offline() might report false* incorrectly.*/be_run_online_cb(req->be_ctx);- } return req->fn(req, dp_err, error, errstr);
}
-- 1.7.0.1
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel
On 05/04/2010 01:27 PM, Sumit Bose wrote:
On Mon, May 03, 2010 at 03:41:18PM -0400, Stephen Gallagher wrote:
Rebased to current sssd-1-2.
ACK
Pushed to master and sssd-1-2.
sssd-devel@lists.fedorahosted.org
-
Simo Sorce -
Stephen Gallagher -
Stephen Gallagher -
Sumit Bose