run_cmd: ret empty string on success;err otherwise

Signed-off-by: Colin McCabe <colin.mccabe@dreamhost.com>

diff --git a/src/common/run_cmd.cc b/src/common/run_cmd.cc
index 72b5eb1d0a73644ee5b9052b667276e6692ef13d..5f5cc3cca32355701481fbfba72e006dad3d49fc 100644 (file)
--- a/src/common/run_cmd.cc
+++ b/src/common/run_cmd.cc
@@ -12,11 +12,10 @@
  *
  */
 
-#include "common/config.h"
-#include "common/debug.h"
 #include "common/errno.h"
 
 #include <errno.h>
+#include <sstream>
 #include <stdarg.h>
 #include <stdlib.h>
 #include <sys/types.h>
@@ -24,11 +23,10 @@
 #include <unistd.h>
 #include <vector>
 
-#define dout_prefix *_dout
+using std::ostringstream;
 
-int run_cmd(const char *cmd, ...)
+std::string run_cmd(const char *cmd, ...)
 {
-  int ret;
   std::vector <const char *> arr;
   va_list ap;
   va_start(ap, cmd);
@@ -40,34 +38,43 @@ int run_cmd(const char *cmd, ...)
   va_end(ap);
   arr.push_back(NULL);
 
-  ret = fork();
-  if (ret == -1) {
+  int fret = fork();
+  if (fret == -1) {
     int err = errno;
-    derr << "run_cmd(" << cmd << "): unable to fork(): " << cpp_strerror(err)
-         << dendl;
-    return -1;
+    ostringstream oss;
+    oss << "run_cmd(" << cmd << "): unable to fork(): " << cpp_strerror(err);
+    return oss.str();
   }
-  else if (ret == 0) {
+  else if (fret == 0) {
     // execvp doesn't modify its arguments, so the const-cast here is safe.
     execvp(cmd, (char * const*)&arr[0]);
     _exit(127);
   }
   int status;
-  while (waitpid(ret, &status, 0) == -1) {
+  while (waitpid(fret, &status, 0) == -1) {
     int err = errno;
     if (err == EINTR)
       continue;
-    derr << "run_cmd(" << cmd << "): waitpid error: "
-        << cpp_strerror(err) << dendl;
-    return -1;
+    ostringstream oss;
+    oss << "run_cmd(" << cmd << "): waitpid error: "
+        << cpp_strerror(err);
+    return oss.str();
   }
   if (WIFEXITED(status)) {
-    return WEXITSTATUS(status);
+    int wexitstatus = WEXITSTATUS(status);
+    if (wexitstatus != 0) {
+      ostringstream oss;
+      oss << "run_cmd(" << cmd << "): exited with status " << wexitstatus;
+      return oss.str();
+    }
+    return "";
   }
   else if (WIFSIGNALED(status)) {
-    derr << "run_cmd(" << cmd << "): terminated by signal" << dendl;
-    return -1;
+    ostringstream oss;
+    oss << "run_cmd(" << cmd << "): terminated by signal";
+    return oss.str();
   }
-  derr << "run_cmd(" << cmd << "): terminated by unknown mechanism" << dendl;
-  return -1;
+  ostringstream oss;
+  oss << "run_cmd(" << cmd << "): terminated by unknown mechanism";
+  return oss.str();
 }

diff --git a/src/common/run_cmd.h b/src/common/run_cmd.h
index 4db46989600f1b4d57ce58cee48c59a92a301ca4..9d82a649994e4f29d65d3fc10f37c597c9378d19 100644 (file)
--- a/src/common/run_cmd.h
+++ b/src/common/run_cmd.h
@@ -15,6 +15,8 @@
 #ifndef CEPH_COMMON_RUN_CMD_H
 #define CEPH_COMMON_RUN_CMD_H
 
+#include <string>
+
 //
 // Fork a command and run it. The shell will not be invoked and shell
 // expansions will not be done.
@@ -24,6 +26,8 @@
 // Example:
 //   run_cmd("rm", "-rf", "foo", NULL)
 //
-int run_cmd(const char *cmd, ...);
+// Returns an empty string on success, and an error string otherwise.
+//
+std::string run_cmd(const char *cmd, ...);
 
 #endif

diff --git a/src/common/signal.cc b/src/common/signal.cc
index bac8a18946259795f426c215dee9f1418b3b9620..0e659acf0f634dfe0dae327b2b93e65d28c4942a 100644 (file)
--- a/src/common/signal.cc
+++ b/src/common/signal.cc
@@ -26,8 +26,6 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
-#define dout_prefix *_dout
-
 void install_sighandler(int signum, signal_handler_t handler, int flags)
 {
   int ret;

diff --git a/src/mon/MonitorStore.cc b/src/mon/MonitorStore.cc
index fedb10d9e9e479b86428c5052183c9e62a81526b..ba5b5b83a6891e464dbf1ce1c5396c39f7d1ddc1 100644 (file)
--- a/src/mon/MonitorStore.cc
+++ b/src/mon/MonitorStore.cc
@@ -89,18 +89,18 @@ int MonitorStore::umount()
 
 int MonitorStore::mkfs()
 {
-  int ret = run_cmd("rm", "-rf", dir.c_str(), (char*)NULL);
-  if (ret) {
+  std::string ret = run_cmd("rm", "-rf", dir.c_str(), (char*)NULL);
+  if (!ret.empty()) {
     derr << "MonitorStore::mkfs: failed to remove " << dir
         << ": rm returned " << ret << dendl;
-    return ret;
+    return -EIO;
   }
 
   ret = run_cmd("mkdir", "-p", dir.c_str(), (char*)NULL);
-  if (ret) {
+  if (!ret.empty()) {
     derr << "MonitorStore::mkfs: failed to mkdir -p " << dir
         << ": mkdir returned " << ret << dendl;
-    return ret;
+    return -EIO;
   }
 
   dout(0) << "created monfs at " << dir.c_str() << " for "

diff --git a/src/os/FileStore.cc b/src/os/FileStore.cc
index 3f81adde331855096c5e6af2557db4eb076b4d3f..4b7842ec9dddc17e6f31f81225c1f0695d99e472 100644 (file)
--- a/src/os/FileStore.cc
+++ b/src/os/FileStore.cc
@@ -1002,12 +1002,12 @@ int FileStore::wipe_subvol(const char *s)
       continue;
     ostringstream oss;
     oss << old_dir.str().c_str() << "/" << de->d_name;
-    int ret = run_cmd("rm", "-rf", oss.str().c_str(), (char*)NULL);
-    if (ret) {
+    std::string ret = run_cmd("rm", "-rf", oss.str().c_str(), (char*)NULL);
+    if (!ret.empty()) {
       derr << "FileStore::wipe_subvol: failed to remove " << oss.str() << ": "
           << "error " << ret << dendl;
       ::closedir(dir);
-      return ret;
+      return -EIO;
     }
   }
   ::closedir(dir);
@@ -1025,10 +1025,11 @@ int FileStore::mkfs()
 
   if (!g_conf->filestore_dev.empty()) {
     dout(0) << "mounting" << dendl;
-    ret = run_cmd("mount", g_conf->filestore_dev.c_str(), (char*)NULL);
-    if (ret) {
+    std::string mret = run_cmd("mount", g_conf->filestore_dev.c_str(), (char*)NULL);
+    if (!mret.empty()) {
       derr << "FileStore::mkfs: failed to mount g_conf->filestore_dev "
-          << "'" << g_conf->filestore_dev << "'. Error code " << ret << dendl;
+          << "'" << g_conf->filestore_dev << "'. " << mret << dendl;
+      ret = -EIO;
       goto out;
     }
   }

diff --git a/src/test/run_cmd.cc b/src/test/run_cmd.cc
index f110350e7d3baf554fe81e0a439e15d9f274cc58..f562e93ccc215c7f8130b5d8f083273058c80dd9 100644 (file)
--- a/src/test/run_cmd.cc
+++ b/src/test/run_cmd.cc
@@ -14,13 +14,13 @@ TEST(RunCommand, StringSimple)
   ASSERT_GE(fd, 0);
   ::close(fd);
 
-  int ret = run_cmd("touch", temp_file_name, (char*)NULL);
-  ASSERT_EQ(ret, 0);
+  std::string ret = run_cmd("touch", temp_file_name, (char*)NULL);
+  ASSERT_EQ(ret, "");
 
   ASSERT_EQ(access(temp_file_name, R_OK), 0);
 
   ret = run_cmd("rm", "-f", temp_file_name, (char*)NULL);
-  ASSERT_EQ(ret, 0);
+  ASSERT_EQ(ret, "");
 
   ASSERT_NE(access(temp_file_name, R_OK), 0);
 }