[PATCH] Temporarily mark subdomain as inactive instead marking the whole back end offline

[Jakub Hrozek]

Hi,

the attached patches implement https://fedorahosted.org/sssd/ticket/2637

There are two main use-cases:
    1) If AD DCs are not reachable on the IPA server itself, we should
    avoid going offline completely, at least the IPA domain should be
    still reachable.
    2) If SSSD is connected to a non-root AD DC, we still try to contact
    the forest root because only the forest root normally knows all the
    subdomains. But in many setups, the forest root is not reachable
    due to network restrictions.

The full design is described here:
https://fedorahosted.org/sssd/wiki/DesignDocs/OneWayTrusts#Subdomainofflines...

There is one area that I'm not sure about myself -- in the
ad_subdomains.c changes, I always set ignore_mark_offline to true for
the root DC. I think it's safe because in this case the root domain is a
subdomain and we don't want a subdomain to allow marking the be as
offline, but I would like to ask that this change is double-checked.

I mostly checked by pausing AD DC VMs in my test setups and making sure
that the backend stays offline after lookup error, a subsequent lookup
is answered as if we were online on the backend side and that the
inactive status is reset. Also, main domain failures still must mark
sssd as offline.

[Jakub Hrozek]

On Fri, Sep 04, 2015 at 02:35:49PM +0200, Jakub Hrozek wrote:
> From 591d07855b70aacbb4488ba9a54438ee9ded48b5 Mon Sep 17 00:00:00 2001
> From: Jakub Hrozek 
> Date: Fri, 4 Sep 2015 09:27:17 +0200
> Subject: [PATCH 2/8] DP: Provide a way to mark subdomain as disabled and
>  auto-enable it later with offline_timeout
>
> https://fedorahosted.org/sssd/ticket/2637
>
> Adds a new Data Provider function be_mark_dom_offline() that is a
> replacement for be_mark_offline(). When called, the function would
> either set the whole back end offline, just like be_mark_offline or just
> set the subdomain status to inactive.
>
> When a subdomain is inactive, there is a singleton timed task that would
> re-set the subdomin after offline_timeout seconds.
>
> src/providers/data_provider_be.c | 99 ++++++++++++++++++++++++++++++++++++----
>  src/providers/dp_backend.h       |  1 +
>  2 files changed, 91 insertions(+), 9 deletions(-)

One note -- I'm adding a unit test for this patch:
    https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/log/?h=oneway

But I still think it makes sense to send the patches for review while
I'm working on the test..

[Jakub Hrozek]

On Fri, Sep 04, 2015 at 02:40:11PM +0200, Jakub Hrozek wrote:
> On Fri, Sep 04, 2015 at 02:35:49PM +0200, Jakub Hrozek wrote:
> > From 591d07855b70aacbb4488ba9a54438ee9ded48b5 Mon Sep 17 00:00:00 2001
> > From: Jakub Hrozek 
> > Date: Fri, 4 Sep 2015 09:27:17 +0200
> > Subject: [PATCH 2/8] DP: Provide a way to mark subdomain as disabled and
> >  auto-enable it later with offline_timeout
> >
> > https://fedorahosted.org/sssd/ticket/2637
> >
> > Adds a new Data Provider function be_mark_dom_offline() that is a
> > replacement for be_mark_offline(). When called, the function would
> > either set the whole back end offline, just like be_mark_offline or just
> > set the subdomain status to inactive.
> >
> > When a subdomain is inactive, there is a singleton timed task that would
> > re-set the subdomin after offline_timeout seconds.
> >
> > src/providers/data_provider_be.c | 99 ++++++++++++++++++++++++++++++++++++----
> >  src/providers/dp_backend.h       |  1 +
> >  2 files changed, 91 insertions(+), 9 deletions(-)
>
> One note -- I'm adding a unit test for this patch:
>     https://fedorapeople.org/cgit/jhrozek/public_git/sssd.git/log/?h=oneway
>
> But I still think it makes sense to send the patches for review while
> I'm working on the test..

Done, test added and some debug messages were improved, but no big
changes happened with the patches.

[Pavel Březina]

On 09/04/2015 02:35 PM, Jakub Hrozek wrote:
> Hi,
>
> the attached patches implement https://fedorahosted.org/sssd/ticket/2637
>
> There are two main use-cases:
>      1) If AD DCs are not reachable on the IPA server itself, we should
>      avoid going offline completely, at least the IPA domain should be
>      still reachable.
>      2) If SSSD is connected to a non-root AD DC, we still try to contact
>      the forest root because only the forest root normally knows all the
>      subdomains. But in many setups, the forest root is not reachable
>      due to network restrictions.
>
> The full design is described here:
> https://fedorahosted.org/sssd/wiki/DesignDocs/OneWayTrusts#Subdomainofflines...
>
> There is one area that I'm not sure about myself -- in the
> ad_subdomains.c changes, I always set ignore_mark_offline to true for
> the root DC. I think it's safe because in this case the root domain is a
> subdomain and we don't want a subdomain to allow marking the be as
> offline, but I would like to ask that this change is double-checked.

I think it is ok as well.
> I mostly checked by pausing AD DC VMs in my test setups and making sure
> that the backend stays offline after lookup error, a subsequent lookup
> is answered as if we were online on the backend side and that the
> inactive status is reset. Also, main domain failures still must mark
> sssd as offline.

Hi, I'm about to code-ack those patches. But I would like to make more 
clear about the tri-state in the following function.
> +static void be_mark_subdom_offline(struct sss_domain_info *subdom,
>                               struct be_ctx *be_ctx)
>
> +{
> struct timeval tv;
> struct tevent_timer *timeout = NULL;
> int reset_status_timeout;
> reset_status_timeout = get_offline_timeout(be_ctx);
> tv = tevent_timeval_current_ofs(reset_status_timeout, 0);
> switch (subdom->state) {
> case DOM_DISABLED:
>    DEBUG(SSSDBG_MINOR_FAILURE, "Won't touch disabled subdomain\n");
>    return;
> case DOM_INACTIVE:
>    DEBUG(SSSDBG_TRACE_ALL, "Subdomain already disabled\n");

Subdomain is inactive not disabled.
>    return;
> case DOM_ENABLED:
>    DEBUG(SSSDBG_TRACE_LIBS, "Disabling subdomain %s\n", subdom->name);

You are marking it as inactive not as disabled.
>    break;
> }
> timeout = tevent_add_timer(be_ctx->ev, be_ctx, tv,
>                           be_subdom_reset_status, subdom);
> if (timeout == NULL) {
>    DEBUG(SSSDBG_OP_FAILURE, "Cannot create timer\n");
>    return;
> }
> subdom->state = DOM_INACTIVE;

^^
> +}

So are we marking it as disabled, inactive or offline?

Have you considered to rename DOM_INACTIVE -> DOM_OFFLINE maybe? I think 
it more complies to the situation.

Also DOM_ENABLED should be DOM_ACTIVE (according to design document and 
it makes more sense) or DOM_ONLINE (if you decide to rename DOM_INACTIVE 
to DOM_OFFLINE).

I will test the patches tomorrow.

[Pavel Březina]

On 09/17/2015 10:41 PM, Pavel Březina wrote:
> On 09/04/2015 02:35 PM, Jakub Hrozek wrote:
> > Hi,
> >
> > the attached patches implement https://fedorahosted.org/sssd/ticket/2637
> >
> > There are two main use-cases:
> >      1) If AD DCs are not reachable on the IPA server itself, we should
> >      avoid going offline completely, at least the IPA domain should be
> >      still reachable.
> >      2) If SSSD is connected to a non-root AD DC, we still try to contact
> >      the forest root because only the forest root normally knows all the
> >      subdomains. But in many setups, the forest root is not reachable
> >      due to network restrictions.
> >
> > The full design is described here:
> > https://fedorahosted.org/sssd/wiki/DesignDocs/OneWayTrusts#Subdomainofflines...
> >
> > There is one area that I'm not sure about myself -- in the
> > ad_subdomains.c changes, I always set ignore_mark_offline to true for
> > the root DC. I think it's safe because in this case the root domain is a
> > subdomain and we don't want a subdomain to allow marking the be as
> > offline, but I would like to ask that this change is double-checked.
>
> I think it is ok as well.
> > I mostly checked by pausing AD DC VMs in my test setups and making sure
> > that the backend stays offline after lookup error, a subsequent lookup
> > is answered as if we were online on the backend side and that the
> > inactive status is reset. Also, main domain failures still must mark
> > sssd as offline.
>
> Hi, I'm about to code-ack those patches. But I would like to make more
> clear about the tri-state in the following function.
> > +static void be_mark_subdom_offline(struct sss_domain_info *subdom,
> >                               struct be_ctx *be_ctx)
> >
> > +{
> > struct timeval tv;
> > struct tevent_timer *timeout = NULL;
> > int reset_status_timeout;
> > reset_status_timeout = get_offline_timeout(be_ctx);
> > tv = tevent_timeval_current_ofs(reset_status_timeout, 0);
> > switch (subdom->state) {
> > case DOM_DISABLED:
> >    DEBUG(SSSDBG_MINOR_FAILURE, "Won't touch disabled subdomain\n");
> >    return;
> > case DOM_INACTIVE:
> >    DEBUG(SSSDBG_TRACE_ALL, "Subdomain already disabled\n");
>
> Subdomain is inactive not disabled.
> >    return;
> > case DOM_ENABLED:
> >    DEBUG(SSSDBG_TRACE_LIBS, "Disabling subdomain %s\n",
> >
> > subdom->name);
>
> You are marking it as inactive not as disabled.
> >    break;
> > }
> > timeout = tevent_add_timer(be_ctx->ev, be_ctx, tv,
> >                           be_subdom_reset_status, subdom);
> > if (timeout == NULL) {
> >    DEBUG(SSSDBG_OP_FAILURE, "Cannot create timer\n");
> >    return;
> > }
> > subdom->state = DOM_INACTIVE;
>                     ^^
>
> > +}
>
> So are we marking it as disabled, inactive or offline?
>
> Have you considered to rename DOM_INACTIVE -> DOM_OFFLINE maybe? I think
> it more complies to the situation.
>
> Also DOM_ENABLED should be DOM_ACTIVE (according to design document and
> it makes more sense) or DOM_ONLINE (if you decide to rename DOM_INACTIVE
> to DOM_OFFLINE).
>
> I will test the patches tomorrow.

I read the patch set with unit test, of course.

[Jakub Hrozek]

On Thu, Sep 17, 2015 at 10:41:07PM +0200, Pavel Březina wrote:
> On 09/04/2015 02:35 PM, Jakub Hrozek wrote:
> > Hi,
> >
> > the attached patches implement https://fedorahosted.org/sssd/ticket/2637
> >
> > There are two main use-cases:
> >     1) If AD DCs are not reachable on the IPA server itself, we should
> >     avoid going offline completely, at least the IPA domain should be
> >     still reachable.
> >     2) If SSSD is connected to a non-root AD DC, we still try to contact
> >     the forest root because only the forest root normally knows all the
> >     subdomains. But in many setups, the forest root is not reachable
> >     due to network restrictions.
> >
> > The full design is described here:
> > https://fedorahosted.org/sssd/wiki/DesignDocs/OneWayTrusts#Subdomainofflines...
> >
> > There is one area that I'm not sure about myself -- in the
> > ad_subdomains.c changes, I always set ignore_mark_offline to true for
> > the root DC. I think it's safe because in this case the root domain is a
> > subdomain and we don't want a subdomain to allow marking the be as
> > offline, but I would like to ask that this change is double-checked.
>
> I think it is ok as well.
> > I mostly checked by pausing AD DC VMs in my test setups and making sure
> > that the backend stays offline after lookup error, a subsequent lookup
> > is answered as if we were online on the backend side and that the
> > inactive status is reset. Also, main domain failures still must mark
> > sssd as offline.
>
> Hi, I'm about to code-ack those patches. But I would like to make more clear
> about the tri-state in the following function.
> > +static void be_mark_subdom_offline(struct sss_domain_info *subdom,
> >                               struct be_ctx *be_ctx)
> >
> > +{
> > struct timeval tv;
> > struct tevent_timer *timeout = NULL;
> > int reset_status_timeout;
> > reset_status_timeout = get_offline_timeout(be_ctx);
> > tv = tevent_timeval_current_ofs(reset_status_timeout, 0);
> > switch (subdom->state) {
> > case DOM_DISABLED:
> >    DEBUG(SSSDBG_MINOR_FAILURE, "Won't touch disabled subdomain\n");
> >    return;
> > case DOM_INACTIVE:
> >    DEBUG(SSSDBG_TRACE_ALL, "Subdomain already disabled\n");
>
> Subdomain is inactive not disabled.
> >    return;
> > case DOM_ENABLED:
> >    DEBUG(SSSDBG_TRACE_LIBS, "Disabling subdomain %s\n", subdom->name);
>
> You are marking it as inactive not as disabled.
> >    break;
> > }
> > timeout = tevent_add_timer(be_ctx->ev, be_ctx, tv,
> >                           be_subdom_reset_status, subdom);
> > if (timeout == NULL) {
> >    DEBUG(SSSDBG_OP_FAILURE, "Cannot create timer\n");
> >    return;
> > }
> > subdom->state = DOM_INACTIVE;
>                    ^^
>
> > +}
>
> So are we marking it as disabled, inactive or offline?
>
> Have you considered to rename DOM_INACTIVE -> DOM_OFFLINE maybe? I think it
> more complies to the situation.
>
> Also DOM_ENABLED should be DOM_ACTIVE (according to design document and it
> makes more sense) or DOM_ONLINE (if you decide to rename DOM_INACTIVE to
> DOM_OFFLINE).
>
> I will test the patches tomorrow.

The only reason I didn't name the constants as online/offline is that I
still think this is a bit of a hack and we should have a proper failover
support in the future.

But it's also only an internal change, so I'm fine changing the patches
as well. (I can do that in-tree and then resubmit if there are more
changes required).

[Jakub Hrozek]

On Fri, Sep 18, 2015 at 11:04:36AM +0200, Jakub Hrozek wrote:
> On Thu, Sep 17, 2015 at 10:41:07PM +0200, Pavel Březina wrote:
> > On 09/04/2015 02:35 PM, Jakub Hrozek wrote:
> > > Hi,
> > >
> > > the attached patches implement https://fedorahosted.org/sssd/ticket/2637
> > >
> > > There are two main use-cases:
> > >     1) If AD DCs are not reachable on the IPA server itself, we should
> > >     avoid going offline completely, at least the IPA domain should be
> > >     still reachable.
> > >     2) If SSSD is connected to a non-root AD DC, we still try to contact
> > >     the forest root because only the forest root normally knows all the
> > >     subdomains. But in many setups, the forest root is not reachable
> > >     due to network restrictions.
> > >
> > > The full design is described here:
> > > https://fedorahosted.org/sssd/wiki/DesignDocs/OneWayTrusts#Subdomainofflines...
> > >
> > > There is one area that I'm not sure about myself -- in the
> > > ad_subdomains.c changes, I always set ignore_mark_offline to true for
> > > the root DC. I think it's safe because in this case the root domain is a
> > > subdomain and we don't want a subdomain to allow marking the be as
> > > offline, but I would like to ask that this change is double-checked.
> >
> > I think it is ok as well.
> > > I mostly checked by pausing AD DC VMs in my test setups and making sure
> > > that the backend stays offline after lookup error, a subsequent lookup
> > > is answered as if we were online on the backend side and that the
> > > inactive status is reset. Also, main domain failures still must mark
> > > sssd as offline.
> >
> > Hi, I'm about to code-ack those patches. But I would like to make more clear
> > about the tri-state in the following function.
> > > +static void be_mark_subdom_offline(struct sss_domain_info *subdom,
> > >                               struct be_ctx *be_ctx)
> > >
> > > +{
> > > struct timeval tv;
> > > struct tevent_timer *timeout = NULL;
> > > int reset_status_timeout;
> > > reset_status_timeout = get_offline_timeout(be_ctx);
> > > tv = tevent_timeval_current_ofs(reset_status_timeout, 0);
> > > switch (subdom->state) {
> > > case DOM_DISABLED:
> > >    DEBUG(SSSDBG_MINOR_FAILURE, "Won't touch disabled subdomain\n");
> > >    return;
> > > case DOM_INACTIVE:
> > >    DEBUG(SSSDBG_TRACE_ALL, "Subdomain already disabled\n");
> >
> > Subdomain is inactive not disabled.
> > >    return;
> > > case DOM_ENABLED:
> > >    DEBUG(SSSDBG_TRACE_LIBS, "Disabling subdomain %s\n", subdom->name);
> >
> > You are marking it as inactive not as disabled.
> > >    break;
> > > }
> > > timeout = tevent_add_timer(be_ctx->ev, be_ctx, tv,
> > >                           be_subdom_reset_status, subdom);
> > > if (timeout == NULL) {
> > >    DEBUG(SSSDBG_OP_FAILURE, "Cannot create timer\n");
> > >    return;
> > > }
> > > subdom->state = DOM_INACTIVE;
> >                    ^^
> >
> > > +}
> >
> > So are we marking it as disabled, inactive or offline?
> >
> > Have you considered to rename DOM_INACTIVE -> DOM_OFFLINE maybe? I think it
> > more complies to the situation.
> >
> > Also DOM_ENABLED should be DOM_ACTIVE (according to design document and it
> > makes more sense) or DOM_ONLINE (if you decide to rename DOM_INACTIVE to
> > DOM_OFFLINE).
> >
> > I will test the patches tomorrow.
>
> The only reason I didn't name the constants as online/offline is that I
> still think this is a bit of a hack and we should have a proper failover
> support in the future.
>
> But it's also only an internal change, so I'm fine changing the patches
> as well. (I can do that in-tree and then resubmit if there are more
> changes required).

I changed the states to be active, disabled and inactive and reworded
the debug messages.

No logic changes were done in the patchset.

[Pavel Březina]

On 09/18/2015 03:03 PM, Jakub Hrozek wrote:
> On Fri, Sep 18, 2015 at 11:04:36AM +0200, Jakub Hrozek wrote:
> > On Thu, Sep 17, 2015 at 10:41:07PM +0200, Pavel Březina wrote:
> > > On 09/04/2015 02:35 PM, Jakub Hrozek wrote:
> > > > Hi,
> > > >
> > > > the attached patches implement https://fedorahosted.org/sssd/ticket/2637
> > > >
> > > > There are two main use-cases:
> > > >      1) If AD DCs are not reachable on the IPA server itself, we should
> > > >      avoid going offline completely, at least the IPA domain should be
> > > >      still reachable.
> > > >      2) If SSSD is connected to a non-root AD DC, we still try to contact
> > > >      the forest root because only the forest root normally knows all the
> > > >      subdomains. But in many setups, the forest root is not reachable
> > > >      due to network restrictions.
> > > >
> > > > The full design is described here:
> > > > https://fedorahosted.org/sssd/wiki/DesignDocs/OneWayTrusts#Subdomainofflines...
> > > >
> > > > There is one area that I'm not sure about myself -- in the
> > > > ad_subdomains.c changes, I always set ignore_mark_offline to true for
> > > > the root DC. I think it's safe because in this case the root domain is a
> > > > subdomain and we don't want a subdomain to allow marking the be as
> > > > offline, but I would like to ask that this change is double-checked.
> > >
> > > I think it is ok as well.
> > > > I mostly checked by pausing AD DC VMs in my test setups and making sure
> > > > that the backend stays offline after lookup error, a subsequent lookup
> > > > is answered as if we were online on the backend side and that the
> > > > inactive status is reset. Also, main domain failures still must mark
> > > > sssd as offline.
> > >
> > > Hi, I'm about to code-ack those patches. But I would like to make more clear
> > > about the tri-state in the following function.
> > > > +static void be_mark_subdom_offline(struct sss_domain_info *subdom,
> > > >                               struct be_ctx *be_ctx)
> > > >
> > > > +{
> > > > struct timeval tv;
> > > > struct tevent_timer *timeout = NULL;
> > > > int reset_status_timeout;
> > > > reset_status_timeout = get_offline_timeout(be_ctx);
> > > > tv = tevent_timeval_current_ofs(reset_status_timeout, 0);
> > > > switch (subdom->state) {
> > > > case DOM_DISABLED:
> > > >    DEBUG(SSSDBG_MINOR_FAILURE, "Won't touch disabled subdomain\n");
> > > >    return;
> > > > case DOM_INACTIVE:
> > > >    DEBUG(SSSDBG_TRACE_ALL, "Subdomain already disabled\n");
> > >
> > > Subdomain is inactive not disabled.
> > > >    return;
> > > > case DOM_ENABLED:
> > > >    DEBUG(SSSDBG_TRACE_LIBS, "Disabling subdomain %s\n", subdom->name);
> > >
> > > You are marking it as inactive not as disabled.
> > > >    break;
> > > > }
> > > > timeout = tevent_add_timer(be_ctx->ev, be_ctx, tv,
> > > >                           be_subdom_reset_status, subdom);
> > > > if (timeout == NULL) {
> > > >    DEBUG(SSSDBG_OP_FAILURE, "Cannot create timer\n");
> > > >    return;
> > > > }
> > > > subdom->state = DOM_INACTIVE;
> > >                     ^^
> > >
> > > > +}
> > >
> > > So are we marking it as disabled, inactive or offline?
> > >
> > > Have you considered to rename DOM_INACTIVE -> DOM_OFFLINE maybe? I think it
> > > more complies to the situation.
> > >
> > > Also DOM_ENABLED should be DOM_ACTIVE (according to design document and it
> > > makes more sense) or DOM_ONLINE (if you decide to rename DOM_INACTIVE to
> > > DOM_OFFLINE).
> > >
> > > I will test the patches tomorrow.
> >
> > The only reason I didn't name the constants as online/offline is that I
> > still think this is a bit of a hack and we should have a proper failover
> > support in the future.
> >
> > But it's also only an internal change, so I'm fine changing the patches
> > as well. (I can do that in-tree and then resubmit if there are more
> > changes required).
>
> I changed the states to be active, disabled and inactive and reworded
> the debug messages.
>
> No logic changes were done in the patchset.

Ack.

[Jakub Hrozek]

On Fri, Sep 18, 2015 at 03:11:28PM +0200, Pavel Březina wrote:
> On 09/18/2015 03:03 PM, Jakub Hrozek wrote:
> > On Fri, Sep 18, 2015 at 11:04:36AM +0200, Jakub Hrozek wrote:
> > > On Thu, Sep 17, 2015 at 10:41:07PM +0200, Pavel Březina wrote:
> > > > On 09/04/2015 02:35 PM, Jakub Hrozek wrote:
> > > > > Hi,
> > > > >
> > > > > the attached patches implement https://fedorahosted.org/sssd/ticket/2637
> > > > >
> > > > > There are two main use-cases:
> > > > >     1) If AD DCs are not reachable on the IPA server itself, we should
> > > > >     avoid going offline completely, at least the IPA domain should be
> > > > >     still reachable.
> > > > >     2) If SSSD is connected to a non-root AD DC, we still try to contact
> > > > >     the forest root because only the forest root normally knows all the
> > > > >     subdomains. But in many setups, the forest root is not reachable
> > > > >     due to network restrictions.
> > > > >
> > > > > The full design is described here:
> > > > > https://fedorahosted.org/sssd/wiki/DesignDocs/OneWayTrusts#Subdomainofflines...
> > > > >
> > > > > There is one area that I'm not sure about myself -- in the
> > > > > ad_subdomains.c changes, I always set ignore_mark_offline to true for
> > > > > the root DC. I think it's safe because in this case the root domain is a
> > > > > subdomain and we don't want a subdomain to allow marking the be as
> > > > > offline, but I would like to ask that this change is double-checked.
> > > >
> > > > I think it is ok as well.
> > > > > I mostly checked by pausing AD DC VMs in my test setups and making sure
> > > > > that the backend stays offline after lookup error, a subsequent lookup
> > > > > is answered as if we were online on the backend side and that the
> > > > > inactive status is reset. Also, main domain failures still must mark
> > > > > sssd as offline.
> > > >
> > > > Hi, I'm about to code-ack those patches. But I would like to make more clear
> > > > about the tri-state in the following function.
> > > > > +static void be_mark_subdom_offline(struct sss_domain_info *subdom,
> > > > >                               struct be_ctx *be_ctx)
> > > > >
> > > > > +{
> > > > > struct timeval tv;
> > > > > struct tevent_timer *timeout = NULL;
> > > > > int reset_status_timeout;
> > > > > reset_status_timeout = get_offline_timeout(be_ctx);
> > > > > tv = tevent_timeval_current_ofs(reset_status_timeout, 0);
> > > > > switch (subdom->state) {
> > > > > case DOM_DISABLED:
> > > > >    DEBUG(SSSDBG_MINOR_FAILURE, "Won't touch disabled subdomain\n");
> > > > >    return;
> > > > > case DOM_INACTIVE:
> > > > >    DEBUG(SSSDBG_TRACE_ALL, "Subdomain already disabled\n");
> > > >
> > > > Subdomain is inactive not disabled.
> > > > >    return;
> > > > > case DOM_ENABLED:
> > > > >    DEBUG(SSSDBG_TRACE_LIBS, "Disabling subdomain %s\n", subdom->name);
> > > >
> > > > You are marking it as inactive not as disabled.
> > > > >    break;
> > > > > }
> > > > > timeout = tevent_add_timer(be_ctx->ev, be_ctx, tv,
> > > > >                           be_subdom_reset_status, subdom);
> > > > > if (timeout == NULL) {
> > > > >    DEBUG(SSSDBG_OP_FAILURE, "Cannot create timer\n");
> > > > >    return;
> > > > > }
> > > > > subdom->state = DOM_INACTIVE;
> > > >                    ^^
> > > >
> > > > > +}
> > > >
> > > > So are we marking it as disabled, inactive or offline?
> > > >
> > > > Have you considered to rename DOM_INACTIVE -> DOM_OFFLINE maybe? I think it
> > > > more complies to the situation.
> > > >
> > > > Also DOM_ENABLED should be DOM_ACTIVE (according to design document and it
> > > > makes more sense) or DOM_ONLINE (if you decide to rename DOM_INACTIVE to
> > > > DOM_OFFLINE).
> > > >
> > > > I will test the patches tomorrow.
> > >
> > > The only reason I didn't name the constants as online/offline is that I
> > > still think this is a bit of a hack and we should have a proper failover
> > > support in the future.
> > >
> > > But it's also only an internal change, so I'm fine changing the patches
> > > as well. (I can do that in-tree and then resubmit if there are more
> > > changes required).
> >
> > I changed the states to be active, disabled and inactive and reworded
> > the debug messages.
> >
> > No logic changes were done in the patchset.
>
> Ack.

Thanks a lot for the review, I pushed all the patches to master:
 * 20162352030d1c577bb69d44e967d2c5839e5c0e
 * ece345a74cec793e6d970a4955beb3d4a05935b3
 * 64d4b1e5fd4a3c99ef8d8fef6ad0db52c5152c1c
 * dd0a21738e1b71940bba11134734b5999e9fd8e9
 * 7fc8692d49cdaa0368072f196433c07b475da679
 * 0561d532cf76b035b73cfed929a6896071dac407
 * 99c5f2f6ba0af6ce52be0d82ec2794bacc215742
 * b5825c74b6bf7a99ae2172392dbecb51179013a6