On 08/27/2016 06:54 PM, Justin Stephenson wrote:
Hello,

The attached patches resolve https://fedorahosted.org/sssd/ticket/3142

However, I am having difficult with the man page addition to
'src/man/sssd.conf.5.xml' for this new option. I have stared at the open
and close xml tags(for far too long) and it looks correct but when I
build sssd I never see the sssd.conf man page inclusion. Could anyone
tell me what I am missing here?

If you feel there is better wording for the description please let me know.

Kind regards,
Justin Stephenson

Hello Justin,

CI passed:
http://sssd-ci.duckdns.org/logs/job/52/72/summary.html

I have one little comment about coding style. See below.


0001-MONITOR-Remove-disable-netlink-command-line-option.patch


From 0552c199dd37c7e280304b9bc92ff44a8a1a6d57 Mon Sep 17 00:00:00 2001
From: Justin Stephenson <jstep...@redhat.com>
Date: Fri, 26 Aug 2016 15:15:32 -0400
Subject: [PATCH 1/2]     MONITOR: Remove --disable-netlink command-line option

    Removing monitor command-line option, to be superceded by
    sssd.conf option
---

ACK


0002-MONITOR-Add-disable_netlink-option.patch


From c52c0c1a520cdf8509baaaaac00fa3c7bec0dd73 Mon Sep 17 00:00:00 2001
From: Justin Stephenson <jstep...@redhat.com>
Date: Fri, 26 Aug 2016 17:43:25 -0400
Subject: [PATCH 2/2]     MONITOR: Add disable_netlink option

    Adding a new monitor boolean option to disable netlink support.
    This will give users more control over sssd state changes without
    having to modify systemd unit files.

    Resolves:
    https://fedorahosted.org/sssd/ticket/3142
---
[...]

     /* Set up the environment variable for the Kerberos Replay Cache */
@@ -2471,14 +2472,28 @@ static int monitor_process_init(struct mt_ctx *ctx,
         return ret;
     }

-    ret = setup_netlink(ctx, ctx->ev, network_status_change_cb,
-                        ctx, &ctx->nlctx);
+    ret = confdb_get_bool(ctx->cdb,
+                          CONFDB_MONITOR_CONF_ENTRY,
+                          CONFDB_MONITOR_DISABLE_NETLINK,
+                          false, &disable_netlink);
+
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE,
-              "Cannot set up listening for network notifications\n");
+            "Failed to read disable_netlink from confdb: [%d] %s\n",
                 ^ --- this is right indentation
+            ret, sss_strerror(ret));
                 ^ --- this is right indentation

Please, fix this little nitpicking.

I am not native speaker, I am not able check
text in man page. (I guess you are.)

The first patch ACKed, the second needs
little work.

Regards

--
Petr^4 Čech
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

Reply via email to