Looks good,
Ethan
On Fri, Jan 27, 2012 at 12:50, Ben Pfaff <b...@nicira.com> wrote:
> This eliminates a kluge that was duplicated in three different daemons.
>
> Signed-off-by: Ben Pfaff <b...@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@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
