Looks Good.
On Fri, Apr 1, 2011 at 8:38 AM, Justin Petbot <[email protected]> wrote: > 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 >> [email protected] >> http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
