[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [master] Introduces rsylogd to anaconda (part of #524980)



Generally, I like it, but I have few small objections/questions:

- what happens to the installation, if rsyslogd fails to start (for example during fork)?
- what happens if somebody passes wrong remote logging args? Will rsyslogd still start?

And please make the comments more descriptive, we do not touch this code that much to remember everything and it is helpful to have the info handy.

Martin

----- "Ales Kozumplik" <akozumpl redhat com> wrote:

> A proper syslog daemon allows for remote logging that includes
> installed
> system's syslog.
> 
> Removes the init.c code that simulated syslog activity until now.
> ---
>  loader/Makefile.am   |    3 +-
>  loader/init.c        |  217
> ++++++++++++++------------------------------------
>  scripts/mk-images    |   47 +++++++++++
>  scripts/upd-instroot |    4 +-
>  4 files changed, 113 insertions(+), 158 deletions(-)
> 
> diff --git a/loader/Makefile.am b/loader/Makefile.am
> index e5d1b7f..e90822f 100644
> --- a/loader/Makefile.am
> +++ b/loader/Makefile.am
> @@ -53,7 +53,8 @@ loader_SOURCES     = loader.c copy.c log.c
> moduleinfo.c loadermisc.c \
>                       method.c cdinstall.c hdinstall.c nfsinstall.c \
>                       urlinstall.c net.c urls.c telnet.c telnetd.c
>  
> -init_CFLAGS        = $(COMMON_CFLAGS)
> +init_CFLAGS        = $(COMMON_CFLAGS) $(GLIB_CFLAGS)
> +init_LDADD	   = $(GLIB_LIBS)
>  init_SOURCES       = init.c undomounts.c shutdown.c copy.c
>  
>  shutdown_CFLAGS    = $(COMMON_CFLAGS) -DAS_SHUTDOWN=1
> diff --git a/loader/init.c b/loader/init.c
> index 021f2ab..648cc22 100644
> --- a/loader/init.c
> +++ b/loader/init.c
> @@ -26,6 +26,7 @@
>  #ifndef SOCK_STREAM
>  # define SOCK_STREAM 1
>  #endif 
> +#define klogctl syslog
>  #else
>  #include <ctype.h>
>  #include <dirent.h>
> @@ -53,13 +54,13 @@
>  #include <linux/vt.h>
>  #include <termios.h>
>  #include <libgen.h>
> +#include <glib.h>
>  
>  #include "init.h"
>  #include "copy.h"
>  #include "devt.h"
>  #include "devices.h"
>  
> -#define syslog klogctl
>  #endif
>  
>  #include <asm/types.h>
> @@ -113,6 +114,7 @@ char * env[] = {
>  
>  void shutDown(int doKill, reboot_action rebootAction);
>  static int getKillPolicy(void);
> +static void getSyslog(char*);
>  struct termios ts;
>  
>  static int expected_exit = 0;
> @@ -138,164 +140,35 @@ static void fatal_error(int usePerror) {
>  #endif
>  }
>  
> -static int logChunk(int len, char *inbuf, char *outbuf) {
> -    int inctr, outctr;
> -
> -    for (inctr = 0, outctr = 0; inctr < len; inctr++) {
> -        /* If the character is a NULL that's immediately followed by
> a open
> -         * bracket, we've found the beginning of a new kernel
> message.  Put in
> -         * a line separator.
> -         */
> -        if (inbuf[inctr] == '\0' && inctr+1 < len && inbuf[inctr+1]
> == '<') {
> -            outbuf[outctr] = '\n';
> -            outctr++;
> -        }
> -
> -        /* Or, if we see a NULL right before the end of the chunk,
> that's also
> -         * a good place to add a separator.
> -         */
> -        else if (inbuf[inctr] == '\0' && inctr+1 == len) {
> -            outbuf[outctr] = '\n';
> -            outctr++;
> -        }
> -
> -        /* Otherwise, simply output the character as long as it's not
> NULL. */
> -        else if (inbuf[inctr] != '\0') {
> -            outbuf[outctr] = inbuf[inctr];
> -            outctr++;
> -        }
> -    }
> -
> -    return outctr;
> -}
> -
> -static void doklog(char * fn) {
> -    fd_set readset, unixs;
> -    int in, out, i;
> -    int log;
> -    socklen_t s;
> -    int sock = -1;
> -    struct sockaddr_un sockaddr;
> -    char inbuf[1024], outbuf[1024];
> -    int readfd;
> +/* sets up and launches syslog */
> +static void startSyslog(void) {
> +    int conf_fd;
>      int ret;
> -
> -    in = open("/proc/kmsg", O_RDONLY,0);
> -    if (in < 0) {
> -        /* FIXME: was perror */
> -        printstr("open /proc/kmsg");
> -        return;
> -    }
> -
> -    out = open(fn, O_WRONLY, 0);
> -    if (out < 0) 
> -        printf("couldn't open %s for syslog -- still using
> /tmp/syslog\n", fn);
> -
> -    log = open("/tmp/syslog", O_WRONLY | O_CREAT, 0644);
> -    if (log < 0) {
> -        /* FIXME: was perror */
> -        printstr("error opening /tmp/syslog");
> -        sleep(5);
> -	
> -        close(in);
> -        return;
> +    char addr[128];
> +    char forwardtcp[] = "*.* @@";
> +
> +    getSyslog(addr);
> +    if (strlen(addr) > 0) {
> +        conf_fd = open("/etc/rsyslog.conf", O_WRONLY|O_APPEND);
> +        if (conf_fd < 0) {
> +            printf("error opening /etc/rsyslog.conf: %d\n", errno);
> +            printf("syslog forwarding will not be enabled\n");
> +            sleep(5);
> +        } else {
> +            ret = write(conf_fd, forwardtcp, strlen(forwardtcp));
> +            ret = write(conf_fd, addr, strlen(addr));
> +            ret = write(conf_fd, "\n", 1);
> +            close(conf_fd);
> +        }
>      }
>  
> -    /* if we get this far, we should be in good shape */
> -
> -    if (fork()) {
> -        /* parent */
> -        close(in);
> -        close(out);
> -        close(log);
> -        return;
> -    }
> -    close(0); 
> -    close(1);
> -    close(2);
> -
> -    dup2(1, log);
> -
> -#if defined(USE_LOGDEV)
> -    /* now open the syslog socket */
> -    sockaddr.sun_family = AF_UNIX;
> -    strcpy(sockaddr.sun_path, "/dev/log");
> -    sock = socket(AF_UNIX, SOCK_STREAM, 0);
> -    if (sock < 0) {
> -        printf("error creating socket: %d\n", errno);
> -        sleep(5);
> -    }
> -    printstr("got socket\n");
> -    if (bind(sock, (struct sockaddr *) &sockaddr,
> sizeof(sockaddr.sun_family) + 
> -			strlen(sockaddr.sun_path))) {
> -        printf("bind error: %d\n", errno);
> -        sleep(5);
> -    }
> -    printstr("bound socket\n");
> -    chmod("/dev/log", 0666);
> -    if (listen(sock, 5)) {
> -        printf("listen error: %d\n", errno);
> -        sleep(5);
> +    if (fork() == 0) {
> +        /* rsyslog is going to take care of things, so disable
> console logging */
> +        klogctl(8, NULL, 1);
> +        execl("/sbin/rsyslogd", "/sbin/rsyslogd", "-c4", NULL);
> +        /* happens on error */
> +        doExit(1);
>      }
> -#endif
> -
> -    syslog(8, NULL, 1);
> -
> -    FD_ZERO(&unixs);
> -    while (1) {
> -        memcpy(&readset, &unixs, sizeof(unixs));
> -
> -        if (sock >= 0)
> -            FD_SET(sock, &readset);
> -
> -        FD_SET(in, &readset);
> -
> -        i = select(20, &readset, NULL, NULL, NULL);
> -        if (i <= 0) continue;
> -
> -        if (FD_ISSET(in, &readset)) {
> -            i = read(in, inbuf, sizeof(inbuf));
> -            if (i > 0) {
> -                int loggedLen = logChunk(i, inbuf, outbuf);
> -
> -                if (out >= 0)
> -                    ret = write(out, outbuf, loggedLen);
> -                ret = write(log, outbuf, loggedLen);
> -            }
> -        } 
> -
> -        for (readfd = 0; readfd < 20; ++readfd) {
> -            if (FD_ISSET(readfd, &readset) && FD_ISSET(readfd,
> &unixs)) {
> -                i = read(readfd, inbuf, sizeof(inbuf));
> -                if (i > 0) {
> -                    int loggedLen = logChunk(i, inbuf, outbuf);
> -
> -                    if (out >= 0)
> -                        ret = write(out, outbuf, loggedLen);
> -
> -                    ret = write(log, outbuf, loggedLen);
> -                } else if (i == 0) {
> -                    /* socket closed */
> -                    close(readfd);
> -                    FD_CLR(readfd, &unixs);
> -                }
> -            }
> -        }
> -
> -        if (sock >= 0 && FD_ISSET(sock, &readset)) {
> -            s = sizeof(sockaddr);
> -            readfd = accept(sock, (struct sockaddr *) &sockaddr,
> &s);
> -            if (readfd < 0) {
> -                if (out >= 0)
> -                    ret = write(out, "error in accept\n", 16);
> -                ret = write(log, "error in accept\n", 16);
> -                close(sock);
> -                sock = -1;
> -            } else {
> -                FD_SET(readfd, &unixs);
> -            }
> -        }
> -    }    
>  }
>  
>  static int setupTerminal(int fd) {
> @@ -440,6 +313,38 @@ static int getKillPolicy(void) {
>      return 1;
>  }
>  
> +/* Looks through /proc/cmdline for remote syslog paramters. */
> +static void getSyslog(char *addr) {
> +    int fd;
> +    int len;
> +    char buf[1024];
> +
> +    /* assume nothing gets found */
> +    addr[0] = '\0';
> +    if ((fd = open("/proc/cmdline", O_RDONLY,0)) <= 0) {
> +        return;
> +    }
> +    len = read(fd, buf, sizeof(buf) - 1);
> +    close(fd);
> +    buf[len] = '\0';
> +    
> +    int i;
> +    int argc;
> +    char** argv;
> +    GError* err;
> +    if (!g_shell_parse_argv(buf, &argc, &argv, &err )) {
> +        g_error_free(err);
> +        return;
> +    }
> +    for (i = 0; i < argc; ++i) {
> +        if (!strncmp(argv[i], "syslog=", 7)) {
> +            strcpy(addr, argv[i] + 7);
> +            break;
> +        }
> +    }
> +    g_strfreev(argv);
> +}
> +
>  static int getInitPid(void) {
>      int fd = 0, pid = -1, ret;
>      char * buf = calloc(1, 10);
> @@ -762,7 +667,7 @@ int main(int argc, char **argv) {
>  
>      /* Now we have some /tmp space set up, and /etc and /dev point
> to
>         it. We should be in pretty good shape. */
> -    doklog("/dev/tty4");
> +    startSyslog();
>  
>      /* write out a pid file */
>      if ((fd = open("/var/run/init.pid", O_WRONLY|O_CREAT, 0644)) > 0)
> {
> diff --git a/scripts/mk-images b/scripts/mk-images
> index c3b1686..970e6cb 100755
> --- a/scripts/mk-images
> +++ b/scripts/mk-images
> @@ -556,6 +556,7 @@ makeinitrd() {
>      mkdir -p $MBD_DIR/tmp
>      mkdir -p $MBD_DIR/usr/libexec
>      mkdir -p $MBD_DIR/usr/$LIBDIR/NetworkManager
> +    mkdir -p $MBD_DIR/usr/$LIBDIR/rsyslog
>      mkdir -p $MBD_DIR/usr/share/dbus-1/system-services
>      mkdir -p $MBD_DIR/var/lib/dbus
>      mkdir -p $MBD_DIR/var/lib/dhclient
> @@ -760,6 +761,52 @@ makeinitrd() {
>      instbin $IMGPATH /usr/sbin/mdadm $MBD_DIR /sbin/mdadm
>      instbin $IMGPATH /usr/sbin/mdmon $MBD_DIR /sbin/mdmon
>  
> +    # rsyslog
> +    instbin $IMGPATH /usr/sbin/rsyslogd $MBD_DIR /sbin/rsyslogd
> +    ( cd $IMGPATH/usr/$LIBDIR/rsyslog
> +      for f in *.so; do
> +          instbin $IMGPATH /usr/$LIBDIR/rsyslog/$f $MBD_DIR
> /usr/$LIBDIR/rsyslog/$f
> +      done
> +    )
> +    # \EOF has a quote in the first character on purpose; see man
> bash on here documents
> +    cat > $MBD_DIR/etc/rsyslog.conf <<\EOF
> +#### MODULES ####
> +$ModLoad imuxsock.so     # provides support for local system logging
> +$ModLoad imklog.so       # provides kernel logging support
> +
> +#### GLOBAL DIRECTIVES ####
> +# Use default timestamp format
> +$ActionFileDefaultTemplate RSYSLOG_TraditionalFileFormat
> +
> +#### TEMPLATES ####
> +
> +$template anaconda_tty4, "%syslogseverity-text:::uppercase%
> %programname%:%msg%\n"
> +$template anaconda_syslog,
> "%timestamp:8:$:date-rfc3164%,%timestamp:1:3:date-subseconds%
> %syslogseverity-text:::uppercase% %programname%:%msg%\n"
> +
> +#### RULES ####
> +*.*;\
> +authpriv.none           /tmp/syslog;anaconda_syslog
> +                        & /dev/tty4;anaconda_tty4
> +
> +# ### begin forwarding rule ###
> +# The statement between the begin ... end define a SINGLE forwarding
> +# rule. They belong together, do NOT split them. If you create
> multiple
> +# forwarding rules, duplicate the whole block!
> +#
> +# An on-disk queue is created for this action. If the remote host is
> +# down, messages are spooled to disk and sent when it is up again.
> +$WorkDirectory /tmp/spool       # where to place spool files
> +$ActionQueueFileName rsyslog_fw # unique name prefix for spool files
> +$ActionQueueMaxDiskSpace 1m     # space limit (use as much as
> possible)
> +$ActionQueueSaveOnShutdown off  # do not save messages to disk on
> shutdown
> +$ActionQueueType LinkedList     # run asynchronously
> +$ActionResumeRetryCount -1      # infinite retries if host is down
> +# remote host is: name/ip:port, e.g. 192.168.0.1:514, port optional
> +# ### end of the forwarding rule ###
> +
> +EOF
> +
> +
>      # Misc
>      instbin $IMGPATH /usr/sbin/dmidecode $MBD_DIR /sbin/dmidecode
>      instbin $IMGPATH /usr/bin/egrep $MBD_DIR /sbin/egrep
> diff --git a/scripts/upd-instroot b/scripts/upd-instroot
> index 2a79ebf..79b22bc 100755
> --- a/scripts/upd-instroot
> +++ b/scripts/upd-instroot
> @@ -188,7 +188,7 @@ PACKAGES="GConf2 NetworkManager ORBit2 acl
> anaconda
>      pygtk2-libglade pykickstart pyparted python python-bugzilla
> python-decorator
>      python-libs python-nss python-pyblock python-sqlite python-epdb
>      python-urlgrabber python-volume_key pyxf86config readline
> redhat-artwork
> -    reiserfs-utils rpm rpm-libs rpm-python sed
> selinux-policy-targeted
> +    reiserfs-utils rpm rpm-libs rpm-python rsyslog sed
> selinux-policy-targeted
>      setup slang smc-meera-fonts specspo sqlite synaptics
> system-config-date
>      system-config-keyboard ${brandpkgname}-logos
> ${brandpkgname}-release
>      sysvinit-tools taipeifonts tcp_wrappers tcp_wrappers-libs telnet
> @@ -456,6 +456,7 @@ sbin/reiserfstune
>  sbin/resize_reiserfs
>  sbin/resize2fs
>  sbin/route
> +sbin/rsyslogd
>  sbin/setfiles
>  sbin/sfdisk
>  sbin/silo
> @@ -483,6 +484,7 @@ usr/$LIBDIR/libuser/*
>  usr/$LIBDIR/pango
>  usr/$LIBDIR/python?.?
>  usr/$LIBDIR/rpm/rpmpopt
> +usr/$LIBDIR/rsyslog
>  usr/$LIBDIR/libiscsi.so*
>  usr/$LIBDIR/libsqlite3.so*
>  usr/$LIBDIR/xorg/modules
> -- 
> 1.6.2.5
> 
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list redhat com
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]