At Wed, 22 Jan 2014 22:35:20 +0900,
Ryusuke Konishi wrote:
>
> After applying commit 8ce4b524a1654ece478f54fce772cc0c16e3c559
> "cleanerd: call _exit(2) twice for ensuring not being a session
> leader", the mount helper program of nilfs2 no longer gets proper
> process ID (pid) of cleaner daemon, and umount.nilfs2 fails to kill
> cleanerd.
>
> This fixes the issue by letting cleanerd print out the pid of spawned
> daemon and letting nilfs_launch_cleanerd() function read it through
> pipe.
>
> Signed-off-by: Ryusuke Konishi <konishi.ryusuke@xxxxxxxxxxxxx>
> Cc: Hitoshi Mitake <mitake.hitoshi@xxxxxxxxxxxxx>
Looks good to me and sorry for my bug!
Reviewed-by: Hitoshi Mitake <mitake.hitoshi@xxxxxxxxxxxxx>
Thanks,
Hitoshi
> ---
> lib/cleaner_exec.c | 84 ++++++++++++++++++++++++++++++++++++++++++----
> man/nilfs_cleanerd.8 | 3 ++
> sbin/cleanerd/cleanerd.c | 3 ++
> 3 files changed, 83 insertions(+), 7 deletions(-)
>
> diff --git a/lib/cleaner_exec.c b/lib/cleaner_exec.c
> index edc55e3..5336c32 100644
> --- a/lib/cleaner_exec.c
> +++ b/lib/cleaner_exec.c
> @@ -114,6 +114,43 @@ static inline int process_is_alive(pid_t pid)
> return (kill(pid, 0) == 0);
> }
>
> +static int receive_pid(int fd, pid_t *ppid)
> +{
> + char buf[100];
> + unsigned long pid;
> + FILE *fp;
> + int ret;
> +
> + fp = fdopen(fd, "r");
> + if (!fp) {
> + nilfs_cleaner_logger(
> + LOG_ERR, _("Error: fdopen failed: %m"));
> + close(fd);
> + goto fail;
> + }
> +
> + /* read process ID of cleanerd through the given fd */
> + while (fgets(buf, sizeof(buf), fp) != NULL) {
> + /*
> + * fgets() is blocked during no child processes
> + * respond, but it will escape returning a NULL value
> + * and terminate the loop when all child processes
> + * close the given pipe (fd) including call of exit().
> + */
> + ret = sscanf(buf, "NILFS_CLEANERD_PID=%lu", &pid);
> + if (ret == 1) {
> + *ppid = pid;
> + fclose(fp); /* this also closes fd */
> + return 0;
> + }
> + }
> + fclose(fp);
> +fail:
> + nilfs_cleaner_logger(
> + LOG_WARNING, _("Warning: cannot get pid of cleanerd"));
> + return -1;
> +}
> +
> int nilfs_launch_cleanerd(const char *device, const char *mntdir,
> unsigned long protperiod, pid_t *ppid)
> {
> @@ -123,6 +160,7 @@ int nilfs_launch_cleanerd(const char *device, const char *mntdir,
> int i = 0;
> int ret;
> char buf[256];
> + int pipes[2];
>
> if (stat(cleanerd, &statbuf) != 0) {
> nilfs_cleaner_logger(LOG_ERR, _("Error: %s not found"),
> @@ -130,8 +168,16 @@ int nilfs_launch_cleanerd(const char *device, const char *mntdir,
> return -1;
> }
>
> + ret = pipe(pipes);
> + if (ret < 0) {
> + nilfs_cleaner_logger(
> + LOG_ERR, _("Error: failed to create pipe: %m"));
> + return -1;
> + }
> +
> ret = fork();
> if (ret == 0) {
> + /* child */
> if (setgid(getgid()) < 0) {
> nilfs_cleaner_logger(
> LOG_ERR,
> @@ -159,16 +205,40 @@ int nilfs_launch_cleanerd(const char *device, const char *mntdir,
> sigdelset(&sigs, SIGSEGV);
> sigprocmask(SIG_UNBLOCK, &sigs, NULL);
>
> + close(pipes[0]);
> + ret = dup2(pipes[1], STDOUT_FILENO);
> + if (ret < 0) {
> + nilfs_cleaner_logger(
> + LOG_ERR,
> + _("Error: failed to duplicate pipe: %m"));
> + exit(1);
> + }
> + close(pipes[1]);
> +
> execv(cleanerd, (char **)dargs);
> exit(1); /* reach only if failed */
> - } else if (ret != -1) {
> - *ppid = ret;
> - return 0; /* cleanerd started */
> + } else if (ret > 0) {
> + /* parent */
> + close(pipes[1]);
> +
> + /*
> + * fork() returns a pid of the child process, but we
> + * cannot use it because cleanerd internally fork()s
> + * and changes pid.
> + */
> + ret = receive_pid(pipes[0], ppid);
> + if (ret < 0)
> + *ppid = 0;
> + /*
> + * always return a success code because cleanerd has
> + * already started.
> + */
> + return 0;
> } else {
> - int errsv = errno;
> - nilfs_cleaner_logger(LOG_ERR,
> - _("Error: could not fork: %s"),
> - strerror(errsv));
> + nilfs_cleaner_logger(
> + LOG_ERR, _("Error: could not fork: %m"));
> + close(pipes[0]);
> + close(pipes[1]);
> }
> return -1;
> }
> diff --git a/man/nilfs_cleanerd.8 b/man/nilfs_cleanerd.8
> index 46caef2..fd1c48a 100644
> --- a/man/nilfs_cleanerd.8
> +++ b/man/nilfs_cleanerd.8
> @@ -24,6 +24,9 @@ users are recommended to invoke \fBnilfs_cleanerd\fP through
> \fBmount.nilfs2\fP(8) or \fBmount\fP(8) and shutdown it through
> \fBumount.nilfs2\fP(8) or \fBumount\fP(8) in order to avoid state
> inconsistencies among administration tools.
> +.PP
> +\fBnilfs_cleanerd\fP displays its process ID (pid) to standard
> +output when it started.
> .SH OPTIONS
> .TP
> \fB\-V\fR, \fB\-\-version\fR
> diff --git a/sbin/cleanerd/cleanerd.c b/sbin/cleanerd/cleanerd.c
> index 86dfcf7..742ab98 100644
> --- a/sbin/cleanerd/cleanerd.c
> +++ b/sbin/cleanerd/cleanerd.c
> @@ -719,6 +719,9 @@ static int daemonize(int nochdir, int noclose)
> if (!nochdir && (chdir(ROOTDIR) < 0))
> return -1;
>
> + printf("NILFS_CLEANERD_PID=%lu\n", (unsigned long)getpid());
> + fflush(stdout);
> +
> if (!noclose) {
> close(0);
> close(1);
> --
> 1.7.9.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
