watchfrr, lib: cleanup & delay detaching

This cleans up watchfrr to be more "normal" like the other daemons in
terms of what it does in main(), i.e. using the full frr_*() call set.

Also, this changes the startup behaviour on watchfrr to stay attached on
the daemon's parent process until startup is really complete.  This
should allow removing the "watchfrr.started" hack at some point.

Signed-off-by: David Lamparter <equinox@diac24.net>

diff --git a/lib/libfrr.c b/lib/libfrr.c
index 2bce4766d3d44af059c2b5068a65b18fdeb611ab..252132201d56b8588bf628da3a826d1a2ddbe69c 100644 (file)
--- a/lib/libfrr.c
+++ b/lib/libfrr.c
@@ -54,6 +54,7 @@ static char pidfile_default[512];
 static char vtypath_default[256];
 
 bool debug_memstats_at_exit = 0;
+static bool nodetach_term, nodetach_daemon;
 
 static char comb_optstr[256];
 static struct option comb_lo[64];
@@ -281,6 +282,8 @@ void frr_preinit(struct frr_daemon_info *daemon, int argc, char **argv)
                opt_extend(&os_zclient);
        if (!(di->flags & FRR_NO_TCPVTY))
                opt_extend(&os_vty);
+       if (di->flags & FRR_DETACH_LATER)
+               nodetach_daemon = true;
 
        snprintf(config_default, sizeof(config_default), "%s/%s.conf",
                 frr_sysconfdir, di->name);
@@ -767,13 +770,16 @@ void frr_config_fork(void)
 {
        hook_call(frr_late_init, master);
 
-       /* Don't start execution if we are in dry-run mode */
-       if (di->dryrun) {
-               frr_config_read_in(NULL);
-               exit(0);
-       }
+       if (!(di->flags & FRR_NO_CFG_PID_DRY)) {
+               /* Don't start execution if we are in dry-run mode */
+               if (di->dryrun) {
+                       frr_config_read_in(NULL);
+                       exit(0);
+               }
 
-       thread_add_event(master, frr_config_read_in, NULL, 0, &di->read_in);
+               thread_add_event(master, frr_config_read_in, NULL, 0,
+                                &di->read_in);
+       }
 
        if (di->daemon_mode || di->terminal)
                frr_daemonize();
@@ -783,7 +789,7 @@ void frr_config_fork(void)
        pid_output(di->pid_file);
 }
 
-void frr_vty_serv(void)
+static void frr_vty_serv(void)
 {
        /* allow explicit override of vty_path in the future
         * (not currently set anywhere) */
@@ -810,14 +816,22 @@ void frr_vty_serv(void)
        vty_serv_sock(di->vty_addr, di->vty_port, di->vty_path);
 }
 
+static void frr_check_detach(void)
+{
+       if (nodetach_term || nodetach_daemon)
+               return;
+
+       if (daemon_ctl_sock != -1)
+               close(daemon_ctl_sock);
+       daemon_ctl_sock = -1;
+}
+
 static void frr_terminal_close(int isexit)
 {
        int nullfd;
 
-       if (daemon_ctl_sock != -1) {
-               close(daemon_ctl_sock);
-               daemon_ctl_sock = -1;
-       }
+       nodetach_term = false;
+       frr_check_detach();
 
        if (!di->daemon_mode || isexit) {
                printf("\n%s exiting\n", di->name);
@@ -881,6 +895,12 @@ out:
        return 0;
 }
 
+void frr_detach(void)
+{
+       nodetach_daemon = false;
+       frr_check_detach();
+}
+
 void frr_run(struct thread_master *master)
 {
        char instanceinfo[64] = "";
@@ -895,6 +915,8 @@ void frr_run(struct thread_master *master)
                    instanceinfo, di->vty_port, di->startinfo);
 
        if (di->terminal) {
+               nodetach_term = true;
+
                vty_stdio(frr_terminal_close);
                if (daemon_ctl_sock != -1) {
                        set_nonblocking(daemon_ctl_sock);
@@ -914,9 +936,7 @@ void frr_run(struct thread_master *master)
                        close(nullfd);
                }
 
-               if (daemon_ctl_sock != -1)
-                       close(daemon_ctl_sock);
-               daemon_ctl_sock = -1;
+               frr_check_detach();
        }
 
        /* end fixed stderr startup logging */

diff --git a/lib/libfrr.h b/lib/libfrr.h
index d2552799069ce9233d048c7f2a46f8383e9d3f64..db58ff92be9a8f9b15bf6181996802106a79908f 100644 (file)
--- a/lib/libfrr.h
+++ b/lib/libfrr.h
@@ -29,11 +29,21 @@
 #include "module.h"
 #include "hook.h"
 
+/* The following options disable specific command line options that
+ * are not applicable for a particular daemon.
+ */
 #define FRR_NO_PRIVSEP         (1 << 0)
 #define FRR_NO_TCPVTY          (1 << 1)
 #define FRR_LIMITED_CLI                (1 << 2)
-#define FRR_NO_CFG_PID_DRY             (1 << 3)
+#define FRR_NO_CFG_PID_DRY     (1 << 3)
 #define FRR_NO_ZCLIENT         (1 << 4)
+/* If FRR_DETACH_LATER is used, the daemon will keep its parent running
+ * until frr_detach() is called.  Normally "somedaemon -d" returns once the
+ * main event loop is reached in the daemon;  use this for extra startup bits.
+ *
+ * Does nothing if -d isn't used.
+ */
+#define FRR_DETACH_LATER       (1 << 5)
 
 struct frr_daemon_info {
        unsigned flags;
@@ -102,10 +112,8 @@ extern struct thread_master *frr_init(void);
 DECLARE_HOOK(frr_late_init, (struct thread_master * tm), (tm))
 extern void frr_config_fork(void);
 
-extern void frr_vty_serv(void);
-
-/* note: contains call to frr_vty_serv() */
 extern void frr_run(struct thread_master *master);
+extern void frr_detach(void);
 
 extern bool frr_zclient_addr(struct sockaddr_storage *sa, socklen_t *sa_len,
                             const char *path);

diff --git a/watchfrr/watchfrr.c b/watchfrr/watchfrr.c
index ec922490e2af60db4709476303f0c051cfa9a700..e32bf3359bb580e454163c38fe7f7d87912d7ce9 100644 (file)
--- a/watchfrr/watchfrr.c
+++ b/watchfrr/watchfrr.c
@@ -55,9 +55,11 @@
 
 #define PING_TOKEN     "PING"
 
+DEFINE_MGROUP(WATCHFRR, "watchfrr")
+DEFINE_MTYPE_STATIC(WATCHFRR, WATCHFRR_DAEMON, "watchfrr daemon entry")
+
 /* Needs to be global, referenced somewhere inside libfrr. */
 struct thread_master *master;
-static char pidfile_default[256];
 
 static bool watch_only = false;
 
@@ -230,7 +232,7 @@ Otherwise, the interval is doubled (but capped at the -M value).\n\n",
                name of the daemon should be substituted.\n\
     --dry      Do not start or restart anything, just log.\n\
 -p, --pid-file Set process identifier file name\n\
-               (default is %s).\n\
+               (default is %s/watchfrr.pid).\n\
 -b, --blank-string\n\
                When the supplied argument string is found in any of the\n\
                various shell command arguments (-r, -s, or -k), replace\n\
@@ -240,7 +242,7 @@ Otherwise, the interval is doubled (but capped at the -M value).\n\n",
 -h, --help     Display this help and exit\n",
                frr_vtydir, DEFAULT_LOGLEVEL, LOG_EMERG, LOG_DEBUG, LOG_DEBUG,
                DEFAULT_MIN_RESTART, DEFAULT_MAX_RESTART, DEFAULT_PERIOD,
-               DEFAULT_TIMEOUT, DEFAULT_RESTART_TIMEOUT, pidfile_default);
+               DEFAULT_TIMEOUT, DEFAULT_RESTART_TIMEOUT, frr_vtydir);
 }
 
 static pid_t run_background(char *shell_cmd)
@@ -609,12 +611,13 @@ static void daemon_send_ready(void)
        if (!sent && gs.numdown == 0) {
                FILE *fp;
 
+               zlog_notice("all daemons up, doing startup-complete notify");
+               frr_detach();
+
                fp = fopen(DAEMON_VTY_DIR "/watchfrr.started", "w");
                if (fp)
                        fclose(fp);
 #if defined HAVE_SYSTEMD
-               zlog_notice(
-                       "Watchfrr: Notifying Systemd we are up and running");
                systemd_send_started(master, 0);
 #endif
                sent = 1;
@@ -961,6 +964,53 @@ static char *translate_blanks(const char *cmd, const char *blankstr)
        return res;
 }
 
+static void watchfrr_init(int argc, char **argv)
+{
+       const char *special = "zebra";
+       int i;
+       struct daemon *dmn, **add = &gs.daemons;
+       char alldaemons[512] = "", *p = alldaemons;
+
+       for (i = optind; i < argc; i++) {
+               dmn = XCALLOC(MTYPE_WATCHFRR_DAEMON, sizeof(*dmn));
+
+               dmn->name = dmn->restart.name = argv[i];
+               dmn->state = DAEMON_INIT;
+               gs.numdaemons++;
+               gs.numdown++;
+               dmn->fd = -1;
+               dmn->t_wakeup = NULL;
+               thread_add_timer_msec(master, wakeup_init, dmn,
+                                     100 + (random() % 900),
+                                     &dmn->t_wakeup);
+               dmn->restart.interval = gs.min_restart_interval;
+               *add = dmn;
+               add = &dmn->next;
+
+               if (!strcmp(dmn->name, special))
+                       gs.special = dmn;
+       }
+
+       if (!gs.daemons) {
+               fprintf(stderr,
+                       "Must specify one or more daemons to monitor.\n\n");
+               frr_help_exit(1);
+       }
+       if (!watch_only && !gs.special) {
+               fprintf(stderr, "\"%s\" daemon must be in daemon lists\n\n",
+                       special);
+               frr_help_exit(1);
+       }
+
+       for (dmn = gs.daemons; dmn; dmn = dmn->next) {
+               snprintf(p, alldaemons + sizeof(alldaemons) - p, "%s%s",
+                        (p == alldaemons) ? "" : " ", dmn->name);
+               p += strlen(p);
+       }
+       zlog_notice("%s %s watching [%s]%s", progname, FRR_VERSION, alldaemons,
+                   watch_only ? ", monitor mode" : "");
+}
+
 struct zebra_privs_t watchfrr_privs = {
 #ifdef VTY_GROUP
        .vty_group = VTY_GROUP,
@@ -984,7 +1034,8 @@ static struct quagga_signal_t watchfrr_signals[] = {
 
 FRR_DAEMON_INFO(watchfrr, WATCHFRR,
                .flags = FRR_NO_PRIVSEP | FRR_NO_TCPVTY | FRR_LIMITED_CLI
-                        | FRR_NO_CFG_PID_DRY | FRR_NO_ZCLIENT,
+                        | FRR_NO_CFG_PID_DRY | FRR_NO_ZCLIENT
+                        | FRR_DETACH_LATER,
 
                .printhelp = printhelp,
                .copyright = "Copyright 2004 Andrew J. Schorr",
@@ -999,13 +1050,8 @@ FRR_DAEMON_INFO(watchfrr, WATCHFRR,
 int main(int argc, char **argv)
 {
        int opt;
-       const char *pidfile = pidfile_default;
-       const char *special = "zebra";
        const char *blankstr = NULL;
 
-       snprintf(pidfile_default, sizeof(pidfile_default), "%s/watchfrr.pid",
-                frr_vtydir);
-
        frr_preinit(&watchfrr_di, argc, argv);
        progname = watchfrr_di.progname;
 
@@ -1087,7 +1133,7 @@ int main(int argc, char **argv)
                        gs.period = 1000 * period;
                } break;
                case 'p':
-                       pidfile = optarg;
+                       watchfrr_di.pid_file = optarg;
                        break;
                case 'r':
                        if (!valid_command(optarg)) {
@@ -1167,99 +1213,18 @@ int main(int argc, char **argv)
 
        master = frr_init();
        watchfrr_error_init();
+       watchfrr_init(argc, argv);
+       watchfrr_vty_init();
+
+       frr_config_fork();
 
        zlog_set_level(ZLOG_DEST_MONITOR, ZLOG_DISABLED);
-       if (watchfrr_di.daemon_mode) {
+       if (watchfrr_di.daemon_mode)
                zlog_set_level(ZLOG_DEST_SYSLOG, MIN(gs.loglevel, LOG_DEBUG));
-               if (daemon(0, 0) < 0) {
-                       fprintf(stderr, "Watchfrr daemon failed: %s",
-                               strerror(errno));
-                       exit(1);
-               }
-       } else
+       else
                zlog_set_level(ZLOG_DEST_STDOUT, MIN(gs.loglevel, LOG_DEBUG));
 
-       watchfrr_vty_init();
-
-       frr_vty_serv();
-
-       {
-               int i;
-               struct daemon *tail = NULL;
-
-               for (i = optind; i < argc; i++) {
-                       struct daemon *dmn;
-
-                       if (!(dmn = (struct daemon *)calloc(1, sizeof(*dmn)))) {
-                               fprintf(stderr, "calloc(1,%u) failed: %s\n",
-                                       (unsigned int)sizeof(*dmn),
-                                       safe_strerror(errno));
-                               return 1;
-                       }
-                       dmn->name = dmn->restart.name = argv[i];
-                       dmn->state = DAEMON_INIT;
-                       gs.numdaemons++;
-                       gs.numdown++;
-                       dmn->fd = -1;
-                       dmn->t_wakeup = NULL;
-                       thread_add_timer_msec(master, wakeup_init, dmn,
-                                             100 + (random() % 900),
-                                             &dmn->t_wakeup);
-                       dmn->restart.interval = gs.min_restart_interval;
-                       if (tail)
-                               tail->next = dmn;
-                       else
-                               gs.daemons = dmn;
-                       tail = dmn;
-
-                       if (!strcmp(dmn->name, special))
-                               gs.special = dmn;
-               }
-       }
-       if (!gs.daemons) {
-               flog_err(EC_WATCHFRR_UNEXPECTED_DAEMONS,
-                         "Must specify one or more daemons to monitor.");
-               frr_help_exit(1);
-       }
-       if (!watch_only && !gs.special) {
-               flog_err(EC_WATCHFRR_UNEXPECTED_DAEMONS,
-                         "\"%s\" daemon must be in daemon lists", special);
-               frr_help_exit(1);
-       }
-
-       /* Make sure we're not already running. */
-       pid_output(pidfile);
-
-       /* Announce which daemons are being monitored. */
-       {
-               struct daemon *dmn;
-               size_t len = 0;
-
-               for (dmn = gs.daemons; dmn; dmn = dmn->next)
-                       len += strlen(dmn->name) + 1;
-
-               {
-                       char buf[len + 1];
-                       char *p = buf;
-
-                       for (dmn = gs.daemons; dmn; dmn = dmn->next) {
-                               if (p != buf)
-                                       *p++ = ' ';
-                               strcpy(p, dmn->name);
-                               p += strlen(p);
-                       }
-                       zlog_notice("%s %s watching [%s]%s", progname,
-                                   FRR_VERSION, buf,
-                                   watch_only ? ", monitor mode" : "");
-               }
-       }
-
-       {
-               struct thread thread;
-
-               while (thread_fetch(master, &thread))
-                       thread_call(&thread);
-       }
+       frr_run(master);
 
        systemd_send_stopping();
        /* Not reached. */

diff --git a/watchfrr/watchfrr.h b/watchfrr/watchfrr.h
index 1a1c19056fa2ae35ba08bd18238b1a9ac90aba92..ee16846a1dc9029244e44b85a5d3bb6d3fce43c7 100644 (file)
--- a/watchfrr/watchfrr.h
+++ b/watchfrr/watchfrr.h
@@ -21,6 +21,10 @@
 #ifndef FRR_WATCHFRR_H
 #define FRR_WATCHFRR_H
 
+#include "lib/memory.h"
+
+DECLARE_MGROUP(WATCHFRR)
+
 extern void watchfrr_vty_init(void);
 
 extern pid_t integrated_write_pid;