[ovs-dev] [daemon 09/10] daemon: Integrate checking for an existing pidfile into daemonize_start().
Justin Petbot
jpetbot at gmail.com
Fri Apr 1 15:38:33 UTC 2011
On Thu, 31 Mar 2011, at 4:31:32 PM, Ben Pfaff wrote:
> Until now, it has been the responsibility of an individual daemon to call
> die_if_already_running() at an appropriate time. A long time ago, this
> had to happen *before* daemonizing, because once the process daemonized
> itself there was no way to report failure to the process that originally
> started the daemon. With the introduction of daemonize_start(), this is
> now possible, but we haven't been taking advantage of it.
>
> Therefore, this commit integrates the die_if_already_running() call into
> daemonize_start() and deletes the calls to it from individual daemons.
> ---
> debian/ovs-monitor-ipsec | 4 +---
> lib/daemon.c | 9 +++++----
> lib/daemon.h | 1 -
> ovsdb/ovsdb-server.c | 1 -
> python/ovs/daemon.py | 9 +++++----
> tests/test-daemon.py | 3 +--
> tests/test-jsonrpc.c | 2 --
> tests/test-jsonrpc.py | 4 +---
> utilities/ovs-controller.c | 1 -
> utilities/ovs-openflowd.c | 1 -
> vswitchd/ovs-brcompatd.c | 1 -
> vswitchd/ovs-vswitchd.c | 1 -
> .../usr_share_openvswitch_scripts_ovs-xapi-sync | 2 --
> 13 files changed, 13 insertions(+), 26 deletions(-)
>
> diff --git a/debian/ovs-monitor-ipsec b/debian/ovs-monitor-ipsec
> index 17f3997..febd569 100755
> --- a/debian/ovs-monitor-ipsec
> +++ b/debian/ovs-monitor-ipsec
> @@ -1,5 +1,5 @@
> #!/usr/bin/python
> -# Copyright (c) 2009, 2010 Nicira Networks
> +# Copyright (c) 2009, 2010, 2011 Nicira Networks
> #
> # Licensed under the Apache License, Version 2.0 (the "License");
> # you may not use this file except in compliance with the License.
> @@ -450,8 +450,6 @@ def main(argv):
> "(use --help for help)\n" % ovs.util.PROGRAM_NAME)
> sys.exit(1)
>
> - ovs.daemon.die_if_already_running()
> -
> remote = args[0]
Do you think it's worth it to correct the man page for ovs-dpctl?
> idl = ovs.db.idl.Idl(remote, "Open_vSwitch", monitor_uuid_schema_cb)
>
> diff --git a/lib/daemon.c b/lib/daemon.c
> index 173dabe..a9cada8 100644
> --- a/lib/daemon.c
> +++ b/lib/daemon.c
> @@ -107,9 +107,9 @@ is_chdir_enabled(void)
> return chdir_;
> }
>
> -/* Normally, die_if_already_running() will terminate the program with a message
> - * if a locked pidfile already exists. If this function is called,
> - * die_if_already_running() will merely log a warning. */
> +/* Normally, daemonize() or damonize_start() will terminate the program with a
> + * message if a locked pidfile already exists. If this function is called, an
> + * existing pidfile will be replaced, with a warning. */
I'm not sure how much of a problem this is, but you should change this
variable to !pidfile.
> void
> ignore_existing_pidfile(void)
> {
> @@ -141,7 +141,7 @@ daemon_set_monitor(void)
>
> /* If a locked pidfile exists, issue a warning message and, unless
> * ignore_existing_pidfile() has been called, terminate the program. */
> -void
> +static void
> die_if_already_running(void)
> {
> pid_t pid;
> @@ -449,6 +449,7 @@ daemonize_start(void)
> /* Running in daemon process. */
> }
>
> + die_if_already_running();
> make_pidfile();
>
> /* Make sure that the unixctl commands for vlog get registered in a
> diff --git a/lib/daemon.h b/lib/daemon.h
> index 8f4fe39..4af98f6 100644
> --- a/lib/daemon.h
> +++ b/lib/daemon.h
> @@ -67,7 +67,6 @@ void daemon_set_monitor(void);
> void daemonize(void);
> void daemonize_start(void);
> void daemonize_complete(void);
> -void die_if_already_running(void);
> void ignore_existing_pidfile(void);
> void daemon_usage(void);
> pid_t read_pidfile(const char *name);
> diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
> index 8d44d84..17ccb6e 100644
> --- a/ovsdb/ovsdb-server.c
> +++ b/ovsdb/ovsdb-server.c
> @@ -101,7 +101,6 @@ main(int argc, char *argv[])
> parse_options(argc, argv, &file_name, &remotes, &unixctl_path,
> &run_command);
>
> - die_if_already_running();
> daemonize_start();
>
> error = ovsdb_file_open(file_name, false, &db, &file);
> diff --git a/python/ovs/daemon.py b/python/ovs/daemon.py
> index 4df2371..cfb4178 100644
> --- a/python/ovs/daemon.py
> +++ b/python/ovs/daemon.py
> @@ -89,9 +89,9 @@ def is_chdir_enabled():
> return _chdir
>
> def ignore_existing_pidfile():
> - """Normally, die_if_already_running() will terminate the program with a
Did you mean to fix this statement?
> - message if a locked pidfile already exists. If this function is called,
> - die_if_already_running() will merely log a warning."""
> + """Normally, daemonize() or daemonize_start() will terminate the program
> + with a message if a locked pidfile already exists. If this function is
> + called, an existing pidfile will be replaced, with a warning."""
> global _overwrite_pidfile
> _overwrite_pidfile = True
>
> @@ -111,7 +111,7 @@ def set_monitor():
> global _monitor
> _monitor = True
>
> -def die_if_already_running():
> +def _die_if_already_running():
> """If a locked pidfile exists, issue a warning message and, unless
> ignore_existing_pidfile() has been called, terminate the program."""
> if _pidfile is None:
> @@ -343,6 +343,7 @@ def daemonize_start():
> _monitor_daemon(daemon_pid)
> # Running in daemon process
>
> + _die_if_already_running()
> _make_pidfile()
>
> def daemonize_complete():
> diff --git a/tests/test-daemon.py b/tests/test-daemon.py
> index 386445d..586e0ec 100644
> --- a/tests/test-daemon.py
> +++ b/tests/test-daemon.py
> @@ -1,4 +1,4 @@
> -# Copyright (c) 2010 Nicira Networks.
> +# Copyright (c) 2010, 2011 Nicira Networks.
> #
> # Licensed under the Apache License, Version 2.0 (the "License");
> # you may not use this file except in compliance with the License.
> @@ -45,7 +45,6 @@ def main(argv):
> % (ovs.util.PROGRAM_NAME, key))
> sys.exit(1)
>
> - ovs.daemon.die_if_already_running()
> ovs.daemon.daemonize_start()
> if bail:
> sys.stderr.write("%s: exiting after daemonize_start() as requested\n"
> diff --git a/tests/test-jsonrpc.c b/tests/test-jsonrpc.c
> index 44edda8..12bbc97 100644
> --- a/tests/test-jsonrpc.c
> +++ b/tests/test-jsonrpc.c
> @@ -182,8 +182,6 @@ do_listen(int argc OVS_UNUSED, char *argv[])
> bool done;
> int error;
>
> - die_if_already_running();
> -
> error = jsonrpc_pstream_open(argv[1], &pstream);
> if (error) {
> ovs_fatal(error, "could not listen on \"%s\"", argv[1]);
> diff --git a/tests/test-jsonrpc.py b/tests/test-jsonrpc.py
> index a8bf553..064457e 100644
> --- a/tests/test-jsonrpc.py
> +++ b/tests/test-jsonrpc.py
> @@ -1,4 +1,4 @@
> -# Copyright (c) 2009, 2010 Nicira Networks
> +# Copyright (c) 2009, 2010, 2011 Nicira Networks
> #
> # Licensed under the Apache License, Version 2.0 (the "License");
> # you may not use this file except in compliance with the License.
> @@ -49,8 +49,6 @@ def handle_rpc(rpc, msg):
> return done
>
> def do_listen(name):
> - ovs.daemon.die_if_already_running()
> -
> error, pstream = ovs.stream.PassiveStream.open(name)
> if error:
> sys.stderr.write("could not listen on \"%s\": %s\n"
> diff --git a/utilities/ovs-controller.c b/utilities/ovs-controller.c
> index af9e11f..ae0ea3d 100644
> --- a/utilities/ovs-controller.c
> +++ b/utilities/ovs-controller.c
> @@ -139,7 +139,6 @@ main(int argc, char *argv[])
> ovs_fatal(0, "no active or passive switch connections");
> }
>
> - die_if_already_running();
> daemonize_start();
>
> retval = unixctl_server_create(unixctl_path, &unixctl);
> diff --git a/utilities/ovs-openflowd.c b/utilities/ovs-openflowd.c
> index 1494209..f9d1ee2 100644
> --- a/utilities/ovs-openflowd.c
> +++ b/utilities/ovs-openflowd.c
> @@ -103,7 +103,6 @@ main(int argc, char *argv[])
> parse_options(argc, argv, &s);
> signal(SIGPIPE, SIG_IGN);
>
> - die_if_already_running();
> daemonize_start();
>
> /* Start listening for ovs-appctl requests. */
> diff --git a/vswitchd/ovs-brcompatd.c b/vswitchd/ovs-brcompatd.c
> index 7b341aa..75e5009 100644
> --- a/vswitchd/ovs-brcompatd.c
> +++ b/vswitchd/ovs-brcompatd.c
> @@ -1295,7 +1295,6 @@ main(int argc, char *argv[])
> process_init();
> ovsrec_init();
>
> - die_if_already_running();
> daemonize_start();
>
> retval = unixctl_server_create(NULL, &unixctl);
> diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
> index bae03dd..b9a2461 100644
> --- a/vswitchd/ovs-vswitchd.c
> +++ b/vswitchd/ovs-vswitchd.c
> @@ -74,7 +74,6 @@ main(int argc, char *argv[])
> process_init();
> ovsrec_init();
>
> - die_if_already_running();
> daemonize_start();
>
> retval = unixctl_server_create(NULL, &unixctl);
> diff --git a/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync b/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync
> index 4d82b99..41a0ba8 100755
> --- a/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync
> +++ b/xenserver/usr_share_openvswitch_scripts_ovs-xapi-sync
> @@ -252,8 +252,6 @@ def main(argv):
> "(use --help for help)\n" % ovs.util.PROGRAM_NAME)
> sys.exit(1)
>
> - ovs.daemon.die_if_already_running()
I see you decided to go back in time and address the issue I mentioned
in the previous commit to look less foolish. Extremely clever.
> -
> remote = args[0]
> idl = ovs.db.idl.Idl(remote, "Open_vSwitch", monitor_uuid_schema_cb)
>
> --
> 1.7.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list