[PATCH] IPA: add callback to reset subdomain timeouts

[Sumit Bose]

Hi,

these two patches are the SSSD part to fix
https://fedorahosted.org/sssd/ticket/2030 . To reset the timeouts I
introduced a new callback type which is always executed if SIGUSR2 (go
online immediately) is received in contrast to the default online
callback which are only executed if the backend was offline before.

If the patch are accepted I will send corresponding ones to FreeIPA to
send the signal if 'ipa trust-add' is run as root.

bye,
Sumit

[Jakub Hrozek]

On Mon, Oct 21, 2013 at 02:54:24PM +0200, Sumit Bose wrote:
>  Hi,
>  
>  these two patches are the SSSD part to fix
>  https://fedorahosted.org/sssd/ticket/2030 . To reset the timeouts I
>  introduced a new callback type which is always executed if SIGUSR2 (go
>  online immediately) is received in contrast to the default online
>  callback which are only executed if the backend was offline before.
>  
>  If the patch are accepted I will send corresponding ones to FreeIPA to
>  send the signal if 'ipa trust-add' is run as root.
>  
>  bye,
>  Sumit 
Hi,

the patch works fine and makes sense. One question:

>  +int be_add_unconditional_online_cb(TALLOC_CTX *mem_ctx, struct
> be_ctx *ctx,
>  +                                   be_callback_t cb, void *pvt,
>  +                                   struct be_cb **unconditional_online_cb)
>  +{
>  +    int ret;
>  +
>  +    ret = be_add_cb(mem_ctx, ctx, cb, pvt, &ctx->unconditional_online_cb_list,
>  +                    unconditional_online_cb);
>  +    if (ret != EOK) {
>  +        DEBUG(SSSDBG_OP_FAILURE, ("be_add_cb failed.\n"));
>  +        return ret;
>  +    }
>  +
>  +    /* Make sure we run the callback for the first
>  +     * connection after startup.
>  +     */
>  +    ctx->run_online_cb = true; 
Is this flag needed? The way I read the callback code, run_online_cb is
only evaluated in be_run_online_cb() that operates on online_cb_list,
not unconditional_online_cb_list.

>  +
>  +    return EOK;
>  +}
>  + 
>  From 671559f98bdd3b79468548e39a72373cc4e6fc04 Mon Sep 17 00:00:00
> 2001
>  From: Sumit Bose <sbose(a)redhat.com&gt;
>  Date: Mon, 21 Oct 2013 14:02:57 +0200
>  Subject: [PATCH 2/2] IPA: add callback to reset subdomain timeouts
>  
>  Fixes https://fedorahosted.org/sssd/ticket/2030 
ACK

[Sumit Bose]

On Tue, Oct 22, 2013 at 10:15:27AM +0200, Jakub Hrozek wrote:
>  On Mon, Oct 21, 2013 at 02:54:24PM +0200, Sumit Bose wrote:
>  > Hi,
>  > 
>  > these two patches are the SSSD part to fix
>  > https://fedorahosted.org/sssd/ticket/2030 . To reset the timeouts I
>  > introduced a new callback type which is always executed if SIGUSR2 (go
>  > online immediately) is received in contrast to the default online
>  > callback which are only executed if the backend was offline before.
>  > 
>  > If the patch are accepted I will send corresponding ones to FreeIPA to
>  > send the signal if 'ipa trust-add' is run as root.
>  > 
>  > bye,
>  > Sumit
>  
>  Hi,
>  
>  the patch works fine and makes sense. One question:
>  
>  > +int be_add_unconditional_online_cb(TALLOC_CTX *mem_ctx, struct be_ctx *ctx,
>  > +                                   be_callback_t cb, void *pvt,
>  > +                                   struct be_cb **unconditional_online_cb)
>  > +{
>  > +    int ret;
>  > +
>  > +    ret = be_add_cb(mem_ctx, ctx, cb, pvt,
> &ctx->unconditional_online_cb_list,
>  > +                    unconditional_online_cb);
>  > +    if (ret != EOK) {
>  > +        DEBUG(SSSDBG_OP_FAILURE, ("be_add_cb failed.\n"));
>  > +        return ret;
>  > +    }
>  > +
>  > +    /* Make sure we run the callback for the first
>  > +     * connection after startup.
>  > +     */
>  > +    ctx->run_online_cb = true;
>  
>  Is this flag needed? The way I read the callback code, run_online_cb is
>  only evaluated in be_run_online_cb() that operates on online_cb_list,
>  not unconditional_online_cb_list. 
You are right, I was misguided by the comment from  be_add_online_cb()
because I want the unconditional callbacks to be run at startup as well.

I'll send a new patch.

bye,
Sumit
>  
>  > +
>  > +    return EOK;
>  > +}
>  > +
>  
>  > From 671559f98bdd3b79468548e39a72373cc4e6fc04 Mon Sep 17 00:00:00 2001
>  > From: Sumit Bose <sbose(a)redhat.com&gt;
>  > Date: Mon, 21 Oct 2013 14:02:57 +0200
>  > Subject: [PATCH 2/2] IPA: add callback to reset subdomain timeouts
>  > 
>  > Fixes https://fedorahosted.org/sssd/ticket/2030
>  
>  ACK
>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Sumit Bose]

On Tue, Oct 22, 2013 at 10:58:57AM +0200, Sumit Bose wrote:
>  On Tue, Oct 22, 2013 at 10:15:27AM +0200, Jakub Hrozek wrote:
>  > On Mon, Oct 21, 2013 at 02:54:24PM +0200, Sumit Bose wrote:
>  > > Hi,
>  > > 
>  > > these two patches are the SSSD part to fix
>  > > https://fedorahosted.org/sssd/ticket/2030 . To reset the timeouts I
>  > > introduced a new callback type which is always executed if SIGUSR2 (go
>  > > online immediately) is received in contrast to the default online
>  > > callback which are only executed if the backend was offline before.
>  > > 
>  > > If the patch are accepted I will send corresponding ones to FreeIPA to
>  > > send the signal if 'ipa trust-add' is run as root.
>  > > 
>  > > bye,
>  > > Sumit
>  > 
>  > Hi,
>  > 
>  > the patch works fine and makes sense. One question:
>  > 
>  > > +int be_add_unconditional_online_cb(TALLOC_CTX *mem_ctx, struct be_ctx *ctx,
>  > > +                                   be_callback_t cb, void *pvt,
>  > > +                                   struct be_cb **unconditional_online_cb)
>  > > +{
>  > > +    int ret;
>  > > +
>  > > +    ret = be_add_cb(mem_ctx, ctx, cb, pvt,
> &ctx->unconditional_online_cb_list,
>  > > +                    unconditional_online_cb);
>  > > +    if (ret != EOK) {
>  > > +        DEBUG(SSSDBG_OP_FAILURE, ("be_add_cb failed.\n"));
>  > > +        return ret;
>  > > +    }
>  > > +
>  > > +    /* Make sure we run the callback for the first
>  > > +     * connection after startup.
>  > > +     */
>  > > +    ctx->run_online_cb = true;
>  > 
>  > Is this flag needed? The way I read the callback code, run_online_cb is
>  > only evaluated in be_run_online_cb() that operates on online_cb_list,
>  > not unconditional_online_cb_list.
>  
>  You are right, I was misguided by the comment from  be_add_online_cb()
>  because I want the unconditional callbacks to be run at startup as well.
>  
>  I'll send a new patch. 
new version attached.

bye,
Sumit

>  
>  bye,
>  Sumit
>  > 
>  > > +
>  > > +    return EOK;
>  > > +}
>  > > +
>  > 
>  > > From 671559f98bdd3b79468548e39a72373cc4e6fc04 Mon Sep 17 00:00:00 2001
>  > > From: Sumit Bose <sbose(a)redhat.com&gt;
>  > > Date: Mon, 21 Oct 2013 14:02:57 +0200
>  > > Subject: [PATCH 2/2] IPA: add callback to reset subdomain timeouts
>  > > 
>  > > Fixes https://fedorahosted.org/sssd/ticket/2030
>  > 
>  > ACK
>  > _______________________________________________
>  > sssd-devel mailing list
>  > sssd-devel(a)lists.fedorahosted.org
>  > https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Jakub Hrozek]

On Tue, Oct 22, 2013 at 12:51:56PM +0200, Sumit Bose wrote:
>  On Tue, Oct 22, 2013 at 10:58:57AM +0200, Sumit Bose wrote:
>  > On Tue, Oct 22, 2013 at 10:15:27AM +0200, Jakub Hrozek wrote:
>  > > On Mon, Oct 21, 2013 at 02:54:24PM +0200, Sumit Bose wrote:
>  > > > Hi,
>  > > > 
>  > > > these two patches are the SSSD part to fix
>  > > > https://fedorahosted.org/sssd/ticket/2030 . To reset the timeouts I
>  > > > introduced a new callback type which is always executed if SIGUSR2 (go
>  > > > online immediately) is received in contrast to the default online
>  > > > callback which are only executed if the backend was offline before.
>  > > > 
>  > > > If the patch are accepted I will send corresponding ones to FreeIPA to
>  > > > send the signal if 'ipa trust-add' is run as root.
>  > > > 
>  > > > bye,
>  > > > Sumit
>  > > 
>  > > Hi,
>  > > 
>  > > the patch works fine and makes sense. One question:
>  > > 
>  > > > +int be_add_unconditional_online_cb(TALLOC_CTX *mem_ctx, struct be_ctx
> *ctx,
>  > > > +                                   be_callback_t cb, void *pvt,
>  > > > +                                   struct be_cb
> **unconditional_online_cb)
>  > > > +{
>  > > > +    int ret;
>  > > > +
>  > > > +    ret = be_add_cb(mem_ctx, ctx, cb, pvt,
> &ctx->unconditional_online_cb_list,
>  > > > +                    unconditional_online_cb);
>  > > > +    if (ret != EOK) {
>  > > > +        DEBUG(SSSDBG_OP_FAILURE, ("be_add_cb failed.\n"));
>  > > > +        return ret;
>  > > > +    }
>  > > > +
>  > > > +    /* Make sure we run the callback for the first
>  > > > +     * connection after startup.
>  > > > +     */
>  > > > +    ctx->run_online_cb = true;
>  > > 
>  > > Is this flag needed? The way I read the callback code, run_online_cb is
>  > > only evaluated in be_run_online_cb() that operates on online_cb_list,
>  > > not unconditional_online_cb_list.
>  > 
>  > You are right, I was misguided by the comment from  be_add_online_cb()
>  > because I want the unconditional callbacks to be run at startup as well.
>  > 
>  > I'll send a new patch.
>  
>  new version attached.
>  
>  bye,
>  Sumit 
The callbacks are run every time now when SIGUSR2 is received.

ACK

[Jakub Hrozek]

On Tue, Oct 22, 2013 at 05:45:36PM +0200, Jakub Hrozek wrote:
>  On Tue, Oct 22, 2013 at 12:51:56PM +0200, Sumit Bose wrote:
>  > On Tue, Oct 22, 2013 at 10:58:57AM +0200, Sumit Bose wrote:
>  > > On Tue, Oct 22, 2013 at 10:15:27AM +0200, Jakub Hrozek wrote:
>  > > > On Mon, Oct 21, 2013 at 02:54:24PM +0200, Sumit Bose wrote:
>  > > > > Hi,
>  > > > > 
>  > > > > these two patches are the SSSD part to fix
>  > > > > https://fedorahosted.org/sssd/ticket/2030 . To reset the timeouts I
>  > > > > introduced a new callback type which is always executed if SIGUSR2
> (go
>  > > > > online immediately) is received in contrast to the default online
>  > > > > callback which are only executed if the backend was offline before.
>  > > > > 
>  > > > > If the patch are accepted I will send corresponding ones to FreeIPA
> to
>  > > > > send the signal if 'ipa trust-add' is run as root.
>  > > > > 
>  > > > > bye,
>  > > > > Sumit
>  > > > 
>  > > > Hi,
>  > > > 
>  > > > the patch works fine and makes sense. One question:
>  > > > 
>  > > > > +int be_add_unconditional_online_cb(TALLOC_CTX *mem_ctx, struct
> be_ctx *ctx,
>  > > > > +                                   be_callback_t cb, void *pvt,
>  > > > > +                                   struct be_cb
> **unconditional_online_cb)
>  > > > > +{
>  > > > > +    int ret;
>  > > > > +
>  > > > > +    ret = be_add_cb(mem_ctx, ctx, cb, pvt,
> &ctx->unconditional_online_cb_list,
>  > > > > +                    unconditional_online_cb);
>  > > > > +    if (ret != EOK) {
>  > > > > +        DEBUG(SSSDBG_OP_FAILURE, ("be_add_cb
> failed.\n"));
>  > > > > +        return ret;
>  > > > > +    }
>  > > > > +
>  > > > > +    /* Make sure we run the callback for the first
>  > > > > +     * connection after startup.
>  > > > > +     */
>  > > > > +    ctx->run_online_cb = true;
>  > > > 
>  > > > Is this flag needed? The way I read the callback code, run_online_cb is
>  > > > only evaluated in be_run_online_cb() that operates on online_cb_list,
>  > > > not unconditional_online_cb_list.
>  > > 
>  > > You are right, I was misguided by the comment from  be_add_online_cb()
>  > > because I want the unconditional callbacks to be run at startup as well.
>  > > 
>  > > I'll send a new patch.
>  > 
>  > new version attached.
>  > 
>  > bye,
>  > Sumit
>  
>  The callbacks are run every time now when SIGUSR2 is received.
>  
>  ACK 
Pushed to master and sssd-1-11