[SSSD] [PATCH] DNS site support 2nd wave - IPA plugin

Jakub Hrozek jhrozek at redhat.com
Wed Apr 10 13:33:58 UTC 2013


On Wed, Apr 10, 2013 at 02:28:39PM +0200, Pavel Březina wrote:
> On 04/10/2013 02:13 PM, Jakub Hrozek wrote:
> >On Tue, Apr 09, 2013 at 02:36:09PM +0200, Pavel Březina wrote:
> >>On 04/08/2013 12:28 PM, Pavel Březina wrote:
> >>>This has to be applied atop the first way.
> >>>
> >>>Feel free to ping me for testing environment.
> >>
> >>Addressed some review issues raised in other thread.
> >>
> >
> >Maybe you'd change the patch to address the compilation
> >issue I raised in the other thread, but the code looks good to me and
> >works OK.
> >
> >I only have concern about the man page and the default.
> >
> >                  <varlistentry>
> >+                    <term>ipa_enable_dns_sites (boolean)</term>
> >+                    <listitem>
> >+                        <para>
> >+                            Enables/disables DNS sites - location based
> >+                            service discovery.
> >
> >I think just enables is OK. A boolean can enable and disable by definition :)
> >
> >+                        </para>
> >+                        <para>
> >+                            If true and _srv_ is present in ipa_server,
> >+                            SSSD will search for SRV records in
> >+                            (1) _location.hostname.example.com, (2) example.com.
> >+                            If (1) exists, it is expected to contain primary
> >+                            servers and (2) is treated as backup servers. If (1)
> >+                            does not exist, (2) is treated as primary servers.
> >
> >Can you run this paragraph by some native speaker? Maybe we could reword it to
> >say something like:
> >
> >"If true and service discovery (see Service Discovery paragraph at the
> >bottom of the man page) is enabled, then the SSSD will first attempt
> >location based discovery using a query that contains
> >"_location.hostname.example.com" and then fall back to traditional SRV
> >discovery. If the location based discovery succeeds, the IPA servers located
> >with the location based discovery are treated as primary servers and the IPA
> >servers located using the traditional SRV discovery are used as back up
> >servers".
> >
> >That was just a suggestion, but the original paragraph was not clear to me.
> >
> >+                        </para>
> >+                        <para>
> >+                            Default: true
> >
> >I think the default should be False until the IPA server supports this
> >discovery by defualt, too, otherwise there is a wasted round-trip for every IPA
> >client.
> >
> >+                        </para>
> >+                    </listitem>
> >+                </varlistentry>
> >
> 
> Thanks. Done.
> 

Ack



More information about the sssd-devel mailing list