Whamcloud - gitweb
LU-10508 utils: use callvpe() in lustre_rsync 69/30869/4
authorJohn L. Hammond <john.hammond@intel.com>
Mon, 15 Jan 2018 16:20:00 +0000 (10:20 -0600)
committerOleg Drokin <oleg.drokin@intel.com>
Fri, 9 Feb 2018 05:58:20 +0000 (05:58 +0000)
Add callvpe() to lustre/utils as a replacement for system() in cases
where the command string includes arbitrary pathnames. In
lustre_rsync, replace calls to system() with calls to callvpe().

Test-Parameters: trivial testlist=lustre-rsync-test
Signed-off-by: John L. Hammond <john.hammond@intel.com>
Change-Id: Id5ae7e25e14346a1293497c2caa221513d0ee9f3
Reviewed-on: https://review.whamcloud.com/30869
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Jian Yu <jian.yu@intel.com>
Reviewed-by: Dmitry Eremin <dmitry.eremin@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>
lustre/utils/Makefile.am
lustre/utils/callvpe.c [new file with mode: 0644]
lustre/utils/callvpe.h [new file with mode: 0644]
lustre/utils/lustre_rsync.c

index bdeafd9..43ca575 100644 (file)
@@ -63,7 +63,7 @@ lfs_SOURCES = lfs.c lfs_project.c lfs_project.h
 lfs_LDADD := liblustreapi.la -lz
 lfs_DEPENDENCIES := liblustreapi.la
 
-lustre_rsync_SOURCES = lustre_rsync.c lustre_rsync.h
+lustre_rsync_SOURCES = lustre_rsync.c lustre_rsync.h callvpe.c callvpe.h
 lustre_rsync_LDADD :=  liblustreapi.la $(PTHREAD_LIBS)
 lustre_rsync_DEPENDENCIES := liblustreapi.la
 
diff --git a/lustre/utils/callvpe.c b/lustre/utils/callvpe.c
new file mode 100644 (file)
index 0000000..5c98a05
--- /dev/null
@@ -0,0 +1,116 @@
+/*
+ * LGPL HEADER START
+ *
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * LGPL HEADER END
+ *
+ * Copyright (c) 2018, Intel Corporation.
+ */
+
+#include <signal.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include "callvpe.h"
+
+/**
+ * callvpe() - execute a file with given arguments and environment.
+ * \param file[in] name or path of file to be executed.
+ * \param args[in] arguments to file.
+ * \param envp[in] execution environment.
+ * \return -1 on failure (for example if fork() failed).
+ * \return process return status on success.
+ *
+ * callvpe() is intended as a safer replacement for system(). It
+ * executes the file specified and returns after it has completed. As
+ * with system during execution of the command, SIGCHLD will be
+ * blocked, and SIGINT and SIGQUIT will be ignored. The main
+ * difference between system(cmd) and callvpe(file, args, envp) is
+ * that system() calls exec("/bin/sh" "-c" "cmd") whereas callvpe()
+ * bypasses the shell and passes the args given directly to execvpe().
+ *
+ * Rather than:
+ *
+ *      snprintf(cmd, sizeof(cmd), "rm -rf %s\n", path);
+ *      rc = system(cmd);
+ *
+ * instead use:
+ *
+ *      char *args[] = { "rm", "-rf", "--", path, NULL };
+ *      extern char **environ;
+ *      rc = callvpe("/bin/rm", args, environ);
+ *
+ * Note that since callvpe() does not use the shell, IO redirection
+ * and pipelines (cmd > /dev/null, cmd 2>&1, cmd1 | cmd2, ...) are not
+ * supported.
+ */
+int callvpe(const char *file, char *const args[], char *const envp[])
+{
+       struct sigaction sa = {
+               .sa_handler = SIG_IGN,
+       };
+       struct sigaction sa_int_saved;
+       struct sigaction sa_quit_saved;
+       sigset_t sigset_saved;
+       pid_t pid;
+       pid_t pid2;
+       int status;
+       int rc;
+
+       sigemptyset(&sa.sa_mask);
+
+       rc = sigaction(SIGINT, &sa, &sa_int_saved);
+       if (rc < 0)
+               return rc;
+
+       rc = sigaction(SIGQUIT, &sa, &sa_quit_saved);
+       if (rc < 0)
+               goto out_sa_int;
+
+       sigaddset(&sa.sa_mask, SIGCHLD);
+       rc = sigprocmask(SIG_BLOCK, &sa.sa_mask, &sigset_saved);
+       if (rc < 0)
+               goto out_sa_quit;
+
+       pid = fork();
+       if (pid < 0) {
+               rc = -1;
+               goto out_sigset;
+       }
+
+       if (pid == 0) {
+               execvpe(file, args, envp);
+               _exit(127);
+       }
+
+       pid2 = waitpid(pid, &status, 0);
+       if (pid2 < 0) {
+               rc = -1;
+               goto out_sigset;
+       }
+
+       rc = status;
+
+out_sigset:
+       sigprocmask(SIG_SETMASK, &sigset_saved, NULL);
+out_sa_quit:
+       sigaction(SIGQUIT, &sa_quit_saved, NULL);
+out_sa_int:
+       sigaction(SIGINT, &sa_int_saved, NULL);
+
+       return rc;
+}
diff --git a/lustre/utils/callvpe.h b/lustre/utils/callvpe.h
new file mode 100644 (file)
index 0000000..d220dc0
--- /dev/null
@@ -0,0 +1,29 @@
+/*
+ * LGPL HEADER START
+ *
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as
+ * published by the Free Software Foundation; either version 2.1 of the
+ * License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * LGPL HEADER END
+ *
+ * Copyright (c) 2018, Intel Corporation.
+ */
+
+#ifndef _CALLVPE_H_
+#define _CALLVPE_H_
+
+int callvpe(const char *file, char *const args[], char *const envp[]);
+
+#endif
index 109a389..ed906ac 100644 (file)
 #include <libcfs/util/string.h>
 #include <lustre/lustreapi.h>
 #include "lustre_rsync.h"
+#include "callvpe.h"
 
 #define REPLICATE_STATUS_VER 1
 #define CLEAR_INTERVAL 100
@@ -160,7 +161,6 @@ struct lr_info {
         char savedpath[PATH_MAX + 1];
         char link[PATH_MAX + 1];
         char linktmp[PATH_MAX + 1];
-       char cmd[PATH_MAX * 10];
         int bufsize;
         char *buf;
 
@@ -289,22 +289,28 @@ int lr_rsync_data(struct lr_info *info)
 
         if (st_src.st_mtime != st_dest.st_mtime ||
             st_src.st_size != st_dest.st_size) {
-                /* XXX spawning off an rsync for every data sync and
-                 * waiting synchronously is bad for performance.
-                 * librsync could possibly used here. But it does not
-                 * seem to be of production grade. Multi-threaded
-                 * replication is also to be considered.
-                 */
-                int status;
-
-               if (snprintf(info->cmd, sizeof(info->cmd), "%s --inplace %s %s",
-                            rsync, info->src, info->dest) >= sizeof(info->cmd)) {
-                       rc = -E2BIG;
-                       goto err;
-               }
-               lr_debug(DTRACE, "\t%s %s\n", info->cmd, info->tfid);
-               status = system(info->cmd);
-                if (status == -1) {
+               /* XXX spawning off an rsync for every data sync and
+                * waiting synchronously is bad for performance.
+                * librsync could possibly used here. But it does not
+                * seem to be of production grade. Multi-threaded
+                * replication is also to be considered.
+                */
+               char *args[] = {
+                       rsync,
+                       "--inplace",
+                       "--",
+                       info->src,
+                       info->dest,
+                       NULL,
+               };
+               extern char **environ;
+               int status;
+
+               lr_debug(DTRACE, "\t%s %s %s %s %s %s\n", args[0], args[1],
+                        args[2], args[3], args[4], info->tfid);
+
+               status = callvpe(rsync, args, environ);
+               if (status < 0) {
                         rc = -errno;
                 } else if (WIFEXITED(status)) {
                         status = WEXITSTATUS(status);
@@ -326,7 +332,7 @@ int lr_rsync_data(struct lr_info *info)
                 lr_debug(DTRACE, "Not syncing %s and %s %s\n", info->src,
                          info->dest, info->tfid);
         }
-err:
+
         return rc;
 }
 
@@ -747,15 +753,26 @@ int lr_rmfile(struct lr_info *info)
 /* Recursively remove directory and its contents */
 int lr_rm_recursive(struct lr_info *info)
 {
+       char *args[] = {
+               "rm",
+               "-rf",
+               "--",
+               info->dest,
+               NULL,
+       };
+       extern char **environ;
+       int status;
        int rc;
 
-       snprintf(info->cmd, sizeof(info->cmd), "rm -rf %s",
-                info->dest);
-       rc = system(info->cmd);
-        if (rc == -1)
-                rc = -errno;
+       status = callvpe("/bin/rm", args, environ);
+       if (status < 0)
+               rc = -errno;
+       else if (WIFEXITED(status))
+               rc = WEXITSTATUS(status) == 0 ? 0 : -EINVAL;
+       else
+               rc = -EINTR;
 
-        return rc;
+       return rc;
 }
 
 /* Remove a file under SPECIAL_DIR with its tfid as its name. */