* [ANN] cmogstored 1.6.0 - a mogstored alternative
@ 2016-08-31 16:46 7% Eric Wong
0 siblings, 0 replies; 3+ results
From: Eric Wong @ 2016-08-31 16:46 UTC (permalink / raw)
To: mogile, cmogstored-public
cmogstored is an alternative implementation of the "mogstored" storage
component of MogileFS. cmogstored is implemented in C and does not use
Perl at runtime. cmogstored is the only component you need to install
on a MogileFS storage node.
cmogstored 1.6.0 - minor fixes on allocation errors
There are minor robustness fixes on handling errors when
allocating memory or spawn failures on otherwise-hosed systems.
These bugfixes will not affect real users unless the system
is already hosed or in badly overtaxed, so there's no real
need to upgrade.
There are minor portability improvements and I now test under
FreeBSD 10.x.
The iostat test cases are relaxed a bit to account for
virtualized devices (as iostat is less useful with modern
17 changes since 1.5.0 (Nov 2015):
Rakefile: add missing <div> for Atom feed
test/pwrite-wrap: remove unused variable and comment
test/pwrite_wrap: squelch unnecessary output
test/pwrite_wrap: reduce space overhead required
update copyrights for 2016
build-aux/txt2pre: drop CGI.pm requirement
stdin is always redirected to /dev/null
minor vfork/fork safety fixes
process: try to handle OOM gracefully
http_put: gracefully handle path allocation errors
iostat_process: declare environ extern
test/mgmt: relax checks for iostat mapping
gnulib copyright update for 2016
upgrade: avoid syslog call if execve fails
rely on gnulib for environ portability
INSTALL: update latest Debian stable version to 8.x
README: stop mentioning cgit
http://bogomips.org/cmogstored/files/cmogstored-1.6.0.tar.gz
SHA-1: b35767444a9a993ee2c25c27192de7152a041682
SHA-256: f50d3449f3cdf8e6b67a77e42c6fc2055a7708090a52a7bebd601e3827e8a22f
* homepage: http://bogomips.org/cmogstored/README
* git clone git://bogomips.org/cmogstored.git
* git clone http://bogomips.org/cmogstored.git
* gitweb: http://repo.or.cz/w/cmogstored.git
* list: cmogstored-public@bogomips.org (subscription optional)
* archives: http://bogomips.org/cmogstored-public/
* nntp://news.public-inbox.org/inbox.comp.file-systems.mogilefs.cmogstored
^ permalink raw reply [relevance 7%]
* [PATCH 2/2] minor vfork/fork safety fixes
@ 2016-06-01 8:28 6% ` Eric Wong
0 siblings, 0 replies; 3+ results
From: Eric Wong @ 2016-06-01 8:28 UTC (permalink / raw)
To: cmogstored-public
In case "/bin/sh" or "/dev/null" becomes unavailable during the
lifetime of cmogstored, we will no longer crash when attempting
to (re)start iostat. However, your system is probably hosed
anyways if "/bin/sh" or "/dev/null" become unavailable.
This also fixes a bug where we would leak the iostat pipe if
either fork/vfork fails.
We also close an innocuous race condition where the child
might toggle flags in the parent process and trigger an
extra wakeup.
Finally, we use sigprocmask in the child in case pthread_sigmask
does not not work on some systems after forking. This is likely
only a cosmetic change.
---
cloexec_from.c | 4 +--
iostat_process.c | 100 +++++++++++++++++++++++++++++++++----------------------
svc.c | 6 ++--
util.h | 2 +-
4 files changed, 67 insertions(+), 45 deletions(-)
diff --git a/cloexec_from.c b/cloexec_from.c
index 444e665..8aa6a92 100644
--- a/cloexec_from.c
+++ b/cloexec_from.c
@@ -5,11 +5,11 @@
#include "cmogstored.h"
/*
- * this function is only called in a forked child (for iostat)
+ * this function is only called in a vforked child (for iostat)
* if O_CLOEXEC/SOCK_CLOEXEC is unsupported, or if mog_cloexec_detect()
* detects those flags are broken.
*/
-void mog_cloexec_from(int lowfd)
+void mog_cloexec_from(int lowfd) /* vfork-safe */
{
int fd;
int last_good = lowfd;
diff --git a/iostat_process.c b/iostat_process.c
index d8766e0..9f98077 100644
--- a/iostat_process.c
+++ b/iostat_process.c
@@ -14,6 +14,12 @@ static time_t iostat_last_fail;
static struct mog_iostat *iostat;
static time_t iostat_fail_timeout = 10;
+enum mog_exerr {
+ MOG_EXERR_DUP2 = 3,
+ MOG_EXERR_SIGPROCMASK = 4,
+ MOG_EXERR_EXECVE = 5
+};
+
static void iostat_atexit(void)
{
if (iostat_pid > 0)
@@ -40,8 +46,7 @@ static int iostat_pipe_init(int *fds)
return 0;
}
-/* only called in the child process */
-static const char * exec_cmd(const char *cmd)
+static const char *exec_cmd(const char *cmd)
{
time_t last_fail = time(NULL) - iostat_last_fail;
time_t delay = iostat_fail_timeout - last_fail;
@@ -55,7 +60,7 @@ static const char * exec_cmd(const char *cmd)
return xasprintf("sleep %d; exec %s", (int)delay, cmd);
}
-static void dup2_or_die(int oldfd, int newfd, const char *errdesc)
+static int dup2_retry(int oldfd, int newfd) /* vfork-safe */
{
int rc;
@@ -63,57 +68,62 @@ static void dup2_or_die(int oldfd, int newfd, const char *errdesc)
rc = dup2(oldfd, newfd);
while (rc < 0 && (errno == EINTR || errno == EBUSY));
- if (rc < 0) {
- syslog(LOG_CRIT, "dup2(%s) failed: %m", errdesc);
- _exit(1);
- }
+ return rc;
}
-static void preexec_redirect(int out_fd)
+static void execve_iostat(int out_fd, const char *cmd) /* vfork-safe */
{
- int null_fd;
-
- dup2_or_die(out_fd, STDOUT_FILENO, "iostat_pipe[1],STDOUT");
- mog_close(out_fd);
-
- null_fd = open("/dev/null", O_RDONLY);
- if (null_fd < 0) {
- syslog(LOG_CRIT, "open(/dev/null) failed: %m");
- abort();
- }
- dup2_or_die(null_fd, STDIN_FILENO, "/dev/null,STDIN");
- mog_close(null_fd);
-
- /* don't touch stderr */
+ int i;
+ union {
+ char *argv[4];
+ char const *in[4];
+ } u;
+
+ u.in[0] = "/bin/sh";
+ u.in[1] = "-c";
+ u.in[2] = cmd;
+ u.in[3] = 0;
+
+ if (dup2_retry(out_fd, STDOUT_FILENO) < 0)
+ _exit(MOG_EXERR_DUP2);
+ if (!mog_cloexec_atomic)
+ mog_cloexec_from(STDERR_FILENO + 1);
+
+ /* ignore errors, not much we can do about missing signals */
+ for (i = 1; i < NSIG; i++)
+ (void)signal(i, SIG_DFL);
+
+ if (sigprocmask(SIG_SETMASK, &mog_emptyset, NULL) != 0)
+ _exit(MOG_EXERR_SIGPROCMASK);
+
+ execve("/bin/sh", u.argv, environ);
+ _exit(MOG_EXERR_EXECVE);
}
static pid_t iostat_fork_exec(int out_fd)
{
/* rely on /bin/sh to parse iostat command-line args */
const char *cmd = getenv("MOG_IOSTAT_CMD");
+ int cs;
+
if (!cmd)
cmd = "iostat -dx 1 30";
cmd = exec_cmd(cmd);
-
+ CHECK(int, 0, pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs));
iostat_pid = mog_fork_for_exec();
- if (iostat_pid < 0) {
+ if (iostat_pid < 0)
syslog(LOG_ERR, "fork() for iostat failed: %m");
- } else if (iostat_pid > 0) {
+ else if (iostat_pid == 0) /* child */
+ execve_iostat(out_fd, cmd);
+
+ /* parent */
+ CHECK(int, 0, pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, 0));
+ if (iostat_pid > 0)
mog_process_register(iostat_pid, MOG_PROC_IOSTAT);
- mog_close(out_fd);
- } else {
- /* child */
- preexec_redirect(out_fd);
- if (! mog_cloexec_atomic)
- mog_cloexec_from(STDERR_FILENO + 1);
-
- mog_intr_enable();
- execl("/bin/sh", "sh", "-c", cmd, (char *)NULL);
- syslog(LOG_CRIT, "execl(%s) failed: %m", cmd);
- _exit(1);
- }
+ mog_close(out_fd);
mog_free(cmd);
+
return iostat_pid;
}
@@ -122,8 +132,20 @@ bool mog_iostat_respawn(int oldstatus)
int fds[2];
struct mog_fd *mfd;
- if (WIFEXITED(oldstatus) && WEXITSTATUS(oldstatus) == 0) {
- /* syslog(LOG_DEBUG, "iostat done, restarting"); */
+ if (WIFEXITED(oldstatus)) {
+ int ex = WEXITSTATUS(oldstatus);
+ const char *fn = 0;
+
+ switch (ex) {
+ case 0: break;
+ case MOG_EXERR_DUP2: fn = "dup2"; break;
+ case MOG_EXERR_SIGPROCMASK: fn = "sigprocmask"; break;
+ case MOG_EXERR_EXECVE: fn = "execve"; break;
+ default: fn = "(unknown)";
+ }
+ if (fn)
+ syslog(LOG_ERR, "iostat exited due to %s failure", fn);
+ /* else syslog(LOG_DEBUG, "iostat done, restarting"); */
} else {
iostat_last_fail = time(NULL);
syslog(LOG_WARNING,
diff --git a/svc.c b/svc.c
index a0c2921..43bc356 100644
--- a/svc.c
+++ b/svc.c
@@ -166,14 +166,14 @@ size_t mog_svc_each(Hash_processor processor, void *data)
return rv;
}
-static bool cloexec_disable(struct mog_fd *mfd)
+static bool cloexec_disable(struct mog_fd *mfd) /* vfork-safe */
{
if (mfd)
CHECK(int, 0, mog_set_cloexec(mfd->fd, false));
return true;
}
-static bool svc_cloexec_off_i(void *svcptr, void *unused)
+static bool svc_cloexec_off_i(void *svcptr, void *unused) /* vfork-safe */
{
struct mog_svc *svc = svcptr;
@@ -186,7 +186,7 @@ static bool svc_cloexec_off_i(void *svcptr, void *unused)
* Only call this from a freshly forked upgrade child process.
* This holds no locks to avoid potential deadlocks in post-fork mutexes
*/
-void mog_svc_upgrade_prepare(void)
+void mog_svc_upgrade_prepare(void) /* vfork-safe */
{
(void)hash_do_for_each(by_docroot, svc_cloexec_off_i, NULL);
}
diff --git a/util.h b/util.h
index dda0da5..526eda3 100644
--- a/util.h
+++ b/util.h
@@ -63,7 +63,7 @@ static inline int mog_set_nonblocking(int fd, const bool value)
* ever get defined, we wouldn't be using them in the first place without
* updating this code... (no way they'd be on by default).
*/
-static inline int mog_set_cloexec(int fd, const bool set)
+static inline int mog_set_cloexec(int fd, const bool set) /* vfork-safe */
{
return fcntl(fd, F_SETFD, set ? FD_CLOEXEC : 0);
}
^ permalink raw reply related [relevance 6%]
* [PATCH 0/2] vfork safety fixes
@ 2016-06-01 8:27 6% Eric Wong
0 siblings, 1 reply; 3+ results
From: Eric Wong @ 2016-06-01 8:27 UTC (permalink / raw)
To: cmogstored-public
I doubt these affect any current use cases and I haven't heard
any reports, but better safe than sorry. Hopefully this makes
things easier for anybody who wants to audit the code (please
do so!).
Eric Wong (2):
stdin is always redirected to /dev/null
minor vfork/fork safety fixes
cloexec_from.c | 4 +--
cmogstored.c | 23 +++++++------
iostat_process.c | 100 +++++++++++++++++++++++++++++++++----------------------
process.c | 14 ++++++--
svc.c | 6 ++--
util.h | 2 +-
6 files changed, 91 insertions(+), 58 deletions(-)
^ permalink raw reply [relevance 6%]
Results 1-3 of 3 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2016-06-01 8:27 6% [PATCH 0/2] vfork safety fixes Eric Wong
2016-06-01 8:28 ` [PATCH 1/2] stdin is always redirected to /dev/null Eric Wong
2016-06-01 8:28 6% ` [PATCH 2/2] minor vfork/fork safety fixes Eric Wong
2016-08-31 16:46 7% [ANN] cmogstored 1.6.0 - a mogstored alternative Eric Wong
Code repositories for project(s) associated with this public inbox
https://yhbt.net/cmogstored.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).