[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    AW: [PATCH 7/8] mdev: add SIGHUP handler to reload configuration
From:       Walter Harms <wharms () bfs ! de>
Date:       2020-11-23 11:30:16
Message-ID: 8767e1f9869c4227b1be9b82aec684f3 () bfs ! de
[Download RAW message or body]

Maybe it is bloat, NTL it es expected behavior as other unix daemons
do it this way. IMHO this could be a made into an optional feature.


________________________________________
Von: busybox <busybox-bounces@busybox.net> im Auftrag von Denys Vlasenko <vda.linux@googlemail.com>
Gesendet: Sonntag, 22. November 2020 16:24:38
An: Jan Klötzke
Cc: busybox
Betreff: Re: [PATCH 7/8] mdev: add SIGHUP handler to reload configuration

This is bloat. There is no rule every daemon should understand SIGHUP.

On Mon, Dec 16, 2019 at 10:57 PM Jan Klötzke <jan@kloetzke.net> wrote:
>
> Like a well behaved daemon the reception of SIGHUP triggers a reload of
> /etc/mdev.conf. The file is parsed immediately to catch any errors early
> and to cache the current version.
>
> Signed-off-by: Jan Klötzke <jan@kloetzke.net>
> ---
>  util-linux/mdev.c | 106 +++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 86 insertions(+), 20 deletions(-)
>
> diff --git a/util-linux/mdev.c b/util-linux/mdev.c
> index 671221af5..81c4336af 100644
> --- a/util-linux/mdev.c
> +++ b/util-linux/mdev.c
> @@ -314,6 +314,10 @@ struct globals {
>  #endif
>         struct rule cur_rule;
>         char timestr[sizeof("HH:MM:SS.123456")];
> +#if ENABLE_FEATURE_MDEV_DAEMON
> +       int netlink_fd;
> +       struct fd_pair signal_pipe;
> +#endif
>  } FIX_ALIASING;
>  #define G (*(struct globals*)bb_common_bufsiz1)
>  #define INIT_G() do { \
> @@ -1160,6 +1164,15 @@ static void initial_scan(char *temp)
>
>  #if ENABLE_FEATURE_MDEV_DAEMON
>
> +static void daemon_sighandler(int sig)
> +{
> +       int olderrno = errno;
> +       unsigned char ch = sig; /* use char, avoid dealing with partial writes */
> +       if (write(G.signal_pipe.wr, &ch, 1) != 1)
> +               bb_simple_perror_msg("can't send signal");
> +       errno = olderrno;
> +}
> +
>  /*
>   * The kernel (as of v5.4) will pass up to 32 environment variables with a
>   * total of 2kiB on each event. On top of that the action string and device
> @@ -1177,10 +1190,8 @@ static void initial_scan(char *temp)
>  # define RCVBUF (128 * 1024 * 1024)
>  # define MAX_ENV 32
>
> -static int daemon_init(char *temp)
> +static void daemon_init(char *temp)
>  {
> -       int fd;
> -
>         /* Subscribe for UEVENT kernel messages */
>         /* Without a sufficiently big RCVBUF, a ton of simultaneous events
>          * can trigger ENOBUFS on read, which is unrecoverable.
> @@ -1188,7 +1199,9 @@ static int daemon_init(char *temp)
>          *      mdev -d
>          *      find /sys -name uevent -exec sh -c 'echo add >"{}"' ';'
>          */
> -       fd = create_and_bind_to_netlink(NETLINK_KOBJECT_UEVENT, 1 << 0, RCVBUF);
> +       G.netlink_fd = create_and_bind_to_netlink(NETLINK_KOBJECT_UEVENT,
> +                       1 << 0, RCVBUF);
> +       ndelay_on(G.netlink_fd);
>
>         /*
>          * Make inital scan after the uevent socket is alive and
> @@ -1196,29 +1209,53 @@ static int daemon_init(char *temp)
>          * in daemon mode.
>          */
>         initial_scan(temp);
> +}
> +
> +static void daemon_handle_sighup(void)
> +{
> +       unsigned char sig;
> +
> +       while (safe_read(G.signal_pipe.rd, &sig, 1) == 1);
> +
> +#if ENABLE_FEATURE_MDEV_CONF
> +       // reset parser state
> +       G.filename = "/etc/mdev.conf";
> +       if (G.parser) {
> +               config_close(G.parser);
> +               G.parser = NULL;
> +       }
> +       G.rule_idx = 0;
>
> -       return fd;
> +       // parse whole file to flag errors immediately
> +       dbg1("SIGHUP: reload '%s'", G.filename);
> +       do {
> +               next_rule();
> +       } while (G.parser);
> +#endif
>  }
>
> -static void daemon_loop(char *temp, int fd)
> +static void daemon_handle_events(char *temp)
>  {
> -       for (;;) {
> -               char netbuf[BUFFER_SIZE];
> -               char *env[MAX_ENV];
> -               char *s, *end;
> -               ssize_t len;
> -               int idx;
> +       char netbuf[BUFFER_SIZE];
> +       char *env[MAX_ENV];
> +       char *s, *end;
> +       ssize_t len;
> +       int idx;
>
> -               len = safe_read(fd, netbuf, sizeof(netbuf) - 1);
> +       for (;;) {
> +               len = safe_read(G.netlink_fd, netbuf, sizeof(netbuf) - 1);
>                 if (len < 0) {
> -                       if (errno == ENOBUFS) {
> +                       if (errno == EAGAIN || errno == EWOULDBLOCK) {
> +                               // done reading events
> +                               break;
> +                       } else if (errno == ENOBUFS) {
>                                 /*
>                                  * We ran out of socket receive buffer space.
>                                  * Start from scratch.
>                                  */
>                                 dbg1s("uevent overrun! Rescanning...");
> -                               close(fd);
> -                               fd = daemon_init(temp);
> +                               close(G.netlink_fd);
> +                               daemon_init(temp);
>                                 continue;
>                         }
>                         bb_simple_perror_msg_and_die("read");
> @@ -1242,6 +1279,37 @@ static void daemon_loop(char *temp, int fd)
>                         bb_unsetenv(env[--idx]);
>         }
>  }
> +
> +static void daemon_loop(char *temp)
> +{
> +       xpiped_pair(G.signal_pipe);
> +       close_on_exec_on(G.signal_pipe.rd);
> +       close_on_exec_on(G.signal_pipe.wr);
> +       ndelay_on(G.signal_pipe.rd);
> +       ndelay_on(G.signal_pipe.wr);
> +       bb_signals((1 << SIGHUP), daemon_sighandler);
> +
> +       for (;;) {
> +               struct pollfd pfds[2];
> +               int ret;
> +
> +               pfds[0].fd = G.netlink_fd;
> +               pfds[0].events = POLLIN;
> +               pfds[1].fd = G.signal_pipe.rd;
> +               pfds[1].events = POLLIN;
> +               ret = poll(pfds, 2, -1);
> +               if (ret <= 0) {
> +                       if (ret < 0 && errno != EINTR)
> +                               bb_simple_perror_msg_and_die("poll");
> +                       continue;
> +               }
> +
> +               if (pfds[0].revents)
> +                       daemon_handle_events(temp);
> +               if (pfds[1].revents)
> +                       daemon_handle_sighup();
> +       }
> +}
>  #endif
>
>  int mdev_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
> @@ -1297,12 +1365,10 @@ int mdev_main(int argc UNUSED_PARAM, char **argv)
>                  * after initial scan so that caller can be sure everything
>                  * is up-to-date when mdev process returns.
>                  */
> -               int fd = daemon_init(temp);
> -
> +               daemon_init(temp);
>                 if (!(opt & MDEV_OPT_FOREGROUND))
>                         bb_daemonize_or_rexec(0, argv);
> -
> -               daemon_loop(temp, fd);
> +               daemon_loop(temp);
>         }
>  #endif
>         if (opt & MDEV_OPT_SCAN) {
> --
> 2.20.1
>
> _______________________________________________
> busybox mailing list
> busybox@busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]