[ovs-dev] [PATCH] daemon: New function daemon_save_fd() to preserve fds across detach.

Ben Pfaff blp at nicira.com
Fri Feb 3 00:30:42 UTC 2012


Thank you.  I pushed this to master.

On Wed, Feb 01, 2012 at 05:19:27PM -0800, Ethan Jackson wrote:
> Looks good,
> 
> Ethan
> 
> On Fri, Jan 27, 2012 at 12:50, Ben Pfaff <blp at nicira.com> wrote:
> > This eliminates a kluge that was duplicated in three different daemons.
> >
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > ---
> >  lib/daemon.c          |   36 ++++++++++++++++++++++++++++++------
> >  lib/daemon.h          |    3 ++-
> >  ovsdb/ovsdb-client.c  |    8 ++------
> >  tests/test-netflow.c  |   11 ++---------
> >  utilities/ovs-ofctl.c |   10 ++--------
> >  5 files changed, 38 insertions(+), 30 deletions(-)
> >
> > diff --git a/lib/daemon.c b/lib/daemon.c
> > index 3dd5a1a..8ef5491 100644
> > --- a/lib/daemon.c
> > +++ b/lib/daemon.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
> > + * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira Networks.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -16,6 +16,7 @@
> >
> >  #include <config.h>
> >  #include "daemon.h"
> > +#include <assert.h>
> >  #include <errno.h>
> >  #include <fcntl.h>
> >  #include <signal.h>
> > @@ -61,6 +62,10 @@ static int daemonize_fd = -1;
> >  * it dies due to an error signal? */
> >  static bool monitor;
> >
> > +/* For each of the standard file descriptors, whether to replace it by
> > + * /dev/null (if false) or keep it for the daemon to use (if true). */
> > +static bool save_fds[3];
> > +
> >  static void check_already_running(void);
> >  static int lock_pidfile(FILE *, int command);
> >
> > @@ -142,6 +147,20 @@ daemon_set_monitor(void)
> >     monitor = true;
> >  }
> >
> > +/* A daemon doesn't normally have any use for the file descriptors for stdin,
> > + * stdout, and stderr after it detaches.  To keep these file descriptors from
> > + * e.g. holding an SSH session open, by default detaching replaces each of
> > + * these file descriptors by /dev/null.  But a few daemons expect the user to
> > + * redirect stdout or stderr to a file, in which case it is desirable to keep
> > + * these file descriptors.  This function, therefore, disables replacing 'fd'
> > + * by /dev/null when the daemon detaches. */
> > +void
> > +daemon_save_fd(int fd)
> > +{
> > +    assert(fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO);
> > +    save_fds[fd] = true;
> > +}
> > +
> >  /* If a pidfile has been configured, creates it and stores the running
> >  * process's pid in it.  Ensures that the pidfile will be deleted when the
> >  * process exits. */
> > @@ -402,16 +421,21 @@ monitor_daemon(pid_t daemon_pid)
> >     program_name = saved_program_name;
> >  }
> >
> > -/* Close stdin, stdout, stderr.  If we're started from e.g. an SSH session,
> > - * then this keeps us from holding that session open artificially. */
> > +/* Close standard file descriptors (except any that the client has requested we
> > + * leave open by calling daemon_save_fd()).  If we're started from e.g. an SSH
> > + * session, then this keeps us from holding that session open artificially. */
> >  static void
> >  close_standard_fds(void)
> >  {
> >     int null_fd = get_null_fd();
> >     if (null_fd >= 0) {
> > -        dup2(null_fd, STDIN_FILENO);
> > -        dup2(null_fd, STDOUT_FILENO);
> > -        dup2(null_fd, STDERR_FILENO);
> > +        int fd;
> > +
> > +        for (fd = 0; fd < 3; fd++) {
> > +            if (!save_fds[fd]) {
> > +                dup2(null_fd, fd);
> > +            }
> > +        }
> >     }
> >
> >     /* Disable logging to stderr to avoid wasting CPU time. */
> > diff --git a/lib/daemon.h b/lib/daemon.h
> > index eb38d5d..1b4f988 100644
> > --- a/lib/daemon.h
> > +++ b/lib/daemon.h
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
> > + * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira Networks.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -64,6 +64,7 @@ bool is_chdir_enabled(void);
> >  void set_detach(void);
> >  bool get_detach(void);
> >  void daemon_set_monitor(void);
> > +void daemon_save_fd(int fd);
> >  void daemonize(void);
> >  void daemonize_start(void);
> >  void daemonize_complete(void);
> > diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
> > index d2a9de1..03dd6c0 100644
> > --- a/ovsdb/ovsdb-client.c
> > +++ b/ovsdb/ovsdb-client.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2009, 2010, 2011 Nicira Networks.
> > + * Copyright (c) 2009, 2010, 2011, 2012 Nicira Networks.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -722,12 +722,8 @@ do_monitor(struct jsonrpc *rpc, const char *database,
> >             monitor_print(msg->result, table, &columns, true);
> >             fflush(stdout);
> >             if (get_detach()) {
> > -                /* daemonize() closes the standard file descriptors.  We output
> > -                 * to stdout, so we need to save and restore STDOUT_FILENO. */
> > -                int fd = dup(STDOUT_FILENO);
> > +                daemon_save_fd(STDOUT_FILENO);
> >                 daemonize();
> > -                dup2(fd, STDOUT_FILENO);
> > -                close(fd);
> >             }
> >         } else if (msg->type == JSONRPC_NOTIFY
> >                    && !strcmp(msg->method, "update")) {
> > diff --git a/tests/test-netflow.c b/tests/test-netflow.c
> > index 5a88ada..b85c663 100644
> > --- a/tests/test-netflow.c
> > +++ b/tests/test-netflow.c
> > @@ -1,5 +1,5 @@
> >  /*
> > - * Copyright (c) 2011 Nicira Networks.
> > + * Copyright (c) 2011, 2012 Nicira Networks.
> >  *
> >  * Licensed under the Apache License, Version 2.0 (the "License");
> >  * you may not use this file except in compliance with the License.
> > @@ -190,7 +190,6 @@ main(int argc, char *argv[])
> >     bool exiting = false;
> >     int error;
> >     int sock;
> > -    int fd;
> >     int n;
> >
> >     proctitle_init(argc, argv);
> > @@ -208,10 +207,7 @@ main(int argc, char *argv[])
> >         ovs_fatal(0, "%s: failed to open (%s)", argv[1], strerror(-sock));
> >     }
> >
> > -    /* Daemonization will close stdout but we really want to keep it, so make a
> > -     * copy. */
> > -    fd = dup(STDOUT_FILENO);
> > -
> > +    daemon_save_fd(STDOUT_FILENO);
> >     daemonize_start();
> >
> >     error = unixctl_server_create(NULL, &server);
> > @@ -222,9 +218,6 @@ main(int argc, char *argv[])
> >
> >     daemonize_complete();
> >
> > -    /* Now get stdout back. */
> > -    dup2(fd, STDOUT_FILENO);
> > -
> >     ofpbuf_init(&buf, MAX_RECV);
> >     n = 0;
> >     for (;;) {
> > diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> > index 3d3a7b7..d14575e 100644
> > --- a/utilities/ovs-ofctl.c
> > +++ b/utilities/ovs-ofctl.c
> > @@ -828,12 +828,9 @@ monitor_vconn(struct vconn *vconn)
> >  {
> >     struct unixctl_server *server;
> >     bool exiting = false;
> > -    int error, fd;
> > -
> > -    /* Daemonization will close stderr but we really want to keep it, so make a
> > -     * copy. */
> > -    fd = dup(STDERR_FILENO);
> > +    int error;
> >
> > +    daemon_save_fd(STDERR_FILENO);
> >     daemonize_start();
> >     error = unixctl_server_create(NULL, &server);
> >     if (error) {
> > @@ -842,9 +839,6 @@ monitor_vconn(struct vconn *vconn)
> >     unixctl_command_register("exit", "", 0, 0, ofctl_exit, &exiting);
> >     daemonize_complete();
> >
> > -    /* Now get stderr back. */
> > -    dup2(fd, STDERR_FILENO);
> > -
> >     for (;;) {
> >         struct ofpbuf *b;
> >         int retval;
> > --
> > 1.7.2.5
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list