]> git.apps.os.sepia.ceph.com Git - ceph.git/commit
global: do not start two daemons with a single pid-file (part 2) 7671/head
authorLoic Dachary <ldachary@redhat.com>
Mon, 1 Feb 2016 12:32:13 +0000 (19:32 +0700)
committerKefu Chai <kchai@redhat.com>
Fri, 26 Feb 2016 02:49:52 +0000 (10:49 +0800)
commitc63baebbb9bb5e6564955e920d0f658e5a3092af
treeadb5b3e078cf2e590aa8d8f97fe815e12076819f
parentcf433bac5b7bc6bdbb6fac56c420288544c79ef5
global: do not start two daemons with a single pid-file (part 2)

Fixes the following bugs:

* the fd is open(O_WRONLY) and cannot be read from, safe_read
  always fails and never removes the pid file.

* pidfile_open(g_conf) is close(STDOUT_FILENO) and there is a risk that
  pidfile_open gets STDOUT_FILENO only to have it closed and redirected
  to /dev/null.

* Before writing the file, ftruncate it so that overriding a file
  containing the pid 1234 with the pid 89 does not end up being
  a file with 8934.

* Before reading the file, lseek back to offset 0 otherwise it
  will read nothing.

* tests_pidfile was missing an argument when failing
  TEST_without_pidfile and killed all process with ceph in their name,
  leading to chaos and no useful error message.

* lstat(fd) cannot possibly return a result different from the one
  obtained right after the file was open, stat(path) must be used
  instead.

In addition to fixing the bugs above, refactor the pidfile.cc
implementation to:

* be systematic about error reporting (using cerr for when removing
  the pidfile because derr is not available at this point and derr
  when creating the pidfile).

* replace pidfile_open / pidfile_write with just pidfile_write since
  there never is a case when they are not used together.

More test cases are added to test_pidfile to verify the bugs above are
fixed.

http://tracker.ceph.com/issues/13422 Fixes: #13422

Signed-off-by: Loic Dachary <loic@dachary.org>
(cherry picked from commit 9828d49d6f3ccfc78d496153d263ea39b1722d4b)

Conflicts:
src/global/global_init.cc
      - the `flag` argument of `global_init_prefork()` is not used, so
        it was removed in master. but the cleanup commit was not
        cherry-picked to hammer, thus the conflict. we can just keep it
        around in hammer to minimize the code churn, although it may
        stand in the way of future backports.)
      - s/nullptr/NULL/ as hammer does not support c++11.
src/global/global_init.cc
src/global/pidfile.cc
src/global/pidfile.h