From 2388a783e25d53ae92090430260fbcfb5d7a8d47 Mon Sep 17 00:00:00 2001 From: Ethan Jackson Date: Wed, 14 Nov 2012 18:34:14 -0800 Subject: [PATCH] daemon: Avoid the link() syscall. make_pidfile() depends on the link() system call to atomically create pidfiles when multiple daemons are started concurrently. However, this system call isn't available on ESX so an alternative strategy is necessary. Fortunately, the approach this patch takes is cleaner than the original code. Signed-off-by: Ethan Jackson --- lib/daemon.c | 70 ++++++++++++++++++++++++++++------------------------ 1 file changed, 38 insertions(+), 32 deletions(-) diff --git a/lib/daemon.c b/lib/daemon.c index 71f6f811..25f2db72 100644 --- a/lib/daemon.c +++ b/lib/daemon.c @@ -175,57 +175,63 @@ make_pidfile(void) int error; /* Create a temporary pidfile. */ - tmpfile = xasprintf("%s.tmp%ld", pidfile, pid); - fatal_signal_add_file_to_unlink(tmpfile); - file = fopen(tmpfile, "w+"); + if (overwrite_pidfile) { + tmpfile = xasprintf("%s.tmp%ld", pidfile, pid); + fatal_signal_add_file_to_unlink(tmpfile); + } else { + /* Everyone shares the same file which will be treated as a lock. To + * avoid some uncomfortable race conditions, we can't set up the fatal + * signal unlink until we've acquired it. */ + tmpfile = xasprintf("%s.tmp", pidfile); + } + + file = fopen(tmpfile, "a+"); if (!file) { VLOG_FATAL("%s: create failed (%s)", tmpfile, strerror(errno)); } + error = lock_pidfile(file, F_SETLK); + if (error) { + /* Looks like we failed to acquire the lock. Note that, if we failed + * for some other reason (and '!overwrite_pidfile'), we will have + * left 'tmpfile' as garbage in the file system. */ + VLOG_FATAL("%s: fcntl(F_SETLK) failed (%s)", tmpfile, strerror(error)); + } + + if (!overwrite_pidfile) { + /* We acquired the lock. Make sure to clean up on exit, and verify + * that we're allowed to create the actual pidfile. */ + fatal_signal_add_file_to_unlink(tmpfile); + check_already_running(); + } + if (fstat(fileno(file), &s) == -1) { VLOG_FATAL("%s: fstat failed (%s)", tmpfile, strerror(errno)); } + if (ftruncate(fileno(file), 0) == -1) { + VLOG_FATAL("%s: truncate failed (%s)", tmpfile, strerror(errno)); + } + fprintf(file, "%ld\n", pid); if (fflush(file) == EOF) { VLOG_FATAL("%s: write failed (%s)", tmpfile, strerror(errno)); } - error = lock_pidfile(file, F_SETLK); - if (error) { - VLOG_FATAL("%s: fcntl(F_SETLK) failed (%s)", tmpfile, strerror(error)); - } + error = rename(tmpfile, pidfile); - /* Rename or link it to the correct name. */ - if (overwrite_pidfile) { - if (rename(tmpfile, pidfile) < 0) { - VLOG_FATAL("failed to rename \"%s\" to \"%s\" (%s)", - tmpfile, pidfile, strerror(errno)); - } - } else { - do { - error = link(tmpfile, pidfile) == -1 ? errno : 0; - if (error == EEXIST) { - check_already_running(); - } - } while (error == EINTR || error == EEXIST); - if (error) { - VLOG_FATAL("failed to link \"%s\" as \"%s\" (%s)", - tmpfile, pidfile, strerror(error)); - } + /* Due to a race, 'tmpfile' may be owned by a different process, so we + * shouldn't delete it on exit. */ + fatal_signal_remove_file_to_unlink(tmpfile); + + if (error < 0) { + VLOG_FATAL("failed to rename \"%s\" to \"%s\" (%s)", + tmpfile, pidfile, strerror(errno)); } /* Ensure that the pidfile will get deleted on exit. */ fatal_signal_add_file_to_unlink(pidfile); - /* Delete the temporary pidfile if it still exists. */ - if (!overwrite_pidfile) { - error = fatal_signal_unlink_file_now(tmpfile); - if (error) { - VLOG_FATAL("%s: unlink failed (%s)", tmpfile, strerror(error)); - } - } - /* Clean up. * * We don't close 'file' because its file descriptor must remain open to -- 2.30.2