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

Jakub Hrozek jhrozek at redhat.com
Wed Apr 10 12:13:23 UTC 2013


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>




More information about the sssd-devel mailing list