Separate the destination directory to /data/misc/perfprofd.

Remove reading of aux-config.

Bug: 19483574

(cherry picked from commit 58bade36c738ed96137cfcc8d15f0505f085b5ae)

Change-Id: I019540197561d13d99640a8ff7042c20b58d9b9f

3 files changed, 51 insertions, 162 deletions

diff --git a/perfprofd/perfprofd.conf b/perfprofd/perfprofd.conf
index 482beea1..696c3de5 100644
--- a/perfprofd/perfprofd.conf
+++ b/perfprofd/perfprofd.conf
@@ -5,7 +5,11 @@
 #
 # Destination directory for profiles
 #
-destination_directory=/data/data/com.google.android.gms/files
+destination_directory=/data/misc/perfprofd
+#
+# Config directory for perfprofd
+#
+config_directory=/data/data/com.google.android.gms/files
 #
 # Sampling period (for perf -c option)
 #
diff --git a/perfprofd/perfprofdcore.cc b/perfprofd/perfprofdcore.cc
index f77fa578..58773c7e 100644
--- a/perfprofd/perfprofdcore.cc
+++ b/perfprofd/perfprofdcore.cc
@@ -63,10 +63,8 @@ typedef enum {
   // All systems go for profile collection.
   DO_COLLECT_PROFILE,
 
-  // The destination directory selected in the conf file doesn't exist. Most
-  // likely this is due to a missing or out-of-date version of the uploading
-  // service in GMS core.
-  DONT_PROFILE_MISSING_DESTINATION_DIR,
+  // The selected configuration directory doesn't exist.
+  DONT_PROFILE_MISSING_CONFIG_DIR,
 
   // Destination directory does not contain the semaphore file that
   // the perf profile uploading service creates when it determines
@@ -102,7 +100,8 @@ static unsigned short random_seed[3];
 //
 // Config file path. May be overridden with -c command line option
 //
-static const char *config_file_path = NULL;
+static const char *config_file_path =
+    "/data/data/com.google.android.gms/files/perfprofd.conf";
 
 //
 // Set by SIGHUP signal handler
@@ -125,7 +124,7 @@ class ConfigReader {
   std::string getStringValue(const char *key) const;
 
   // read the specified config file, applying any settings it contains
-  void readFile(const char *configFilePath);
+  void readFile();
 
  private:
   void addUnsignedEntry(const char *key,
@@ -174,8 +173,10 @@ void ConfigReader::addDefaultEntries()
 
   // Destination directory (where to write profiles). This location
   // chosen since it is accessible to the uploader service.
-  addStringEntry("destination_directory",
-                 "/data/data/com.google.android.gms/files");
+  addStringEntry("destination_directory", "/data/misc/perfprofd");
+
+  // Config directory (where to read configs).
+  addStringEntry("config_directory", "/data/data/com.google.android.gms/files");
 
   // Full path to 'perf' executable.
   addStringEntry("perf_path", "/system/xbin/simpleperf");
@@ -320,9 +321,9 @@ static bool isblank(const std::string &line)
   return true;
 }
 
-void ConfigReader::readFile(const char *configFilePath)
+void ConfigReader::readFile()
 {
-  FILE *fp = fopen(configFilePath, "r");
+  FILE *fp = fopen(config_file_path, "r");
   if (!fp) {
     W_ALOGE("unable to open configuration file %s", config_file_path);
     return;
@@ -397,8 +398,8 @@ const char *ckprofile_result_to_string(CKPROFILE_RESULT result)
   switch (result) {
     case DO_COLLECT_PROFILE:
       return "DO_COLLECT_PROFILE";
-    case DONT_PROFILE_MISSING_DESTINATION_DIR:
-      return "missing destination directory";
+    case DONT_PROFILE_MISSING_CONFIG_DIR:
+      return "missing config directory";
     case DONT_PROFILE_MISSING_SEMAPHORE:
       return "missing semaphore file";
     case DONT_PROFILE_MISSING_PERF_EXECUTABLE:
@@ -434,30 +435,6 @@ const char *profile_result_to_string(PROFILE_RESULT result)
 }
 
 //
-// The daemon does a read of the main config file on startup, however
-// if the destination directory also contains a config file, then we
-// read parameters from that as well. This provides a mechanism for
-// changing/controlling the behavior of the daemon via the settings
-// established in the uploader service (which may be easier to update
-// than the daemon).
-//
-static void read_aux_config(ConfigReader &config)
-{
-  std::string destConfig(config.getStringValue("destination_directory"));
-  destConfig += "/perfprofd.conf";
-  FILE *fp = fopen(destConfig.c_str(), "r");
-  if (fp) {
-    fclose(fp);
-    bool trace_config_read =
-        (config.getUnsignedValue("trace_config_read") != 0);
-    if (trace_config_read) {
-      W_ALOGI("reading auxiliary config file %s", destConfig.c_str());
-    }
-    config.readFile(destConfig.c_str());
-  }
-}
-
-//
 // Check to see whether we should perform a profile collection
 //
 static CKPROFILE_RESULT check_profiling_enabled(ConfigReader &config)
@@ -471,41 +448,29 @@ static CKPROFILE_RESULT check_profiling_enabled(ConfigReader &config)
   }
 
   //
-  // Check for the existence of the destination directory
+  // Check for existence of semaphore file in config directory
   //
-  std::string destdir = config.getStringValue("destination_directory");
-  DIR* dir = opendir(destdir.c_str());
-  if (!dir) {
-    W_ALOGW("unable to open destination directory %s: (%s)",
-            destdir.c_str(), strerror(errno));
-    return DONT_PROFILE_MISSING_DESTINATION_DIR;
+  if (access(config.getStringValue("config_directory").c_str(), F_OK) == -1) {
+    W_ALOGW("unable to open config directory %s: (%s)",
+            config.getStringValue("config_directory").c_str(), strerror(errno));
+    return DONT_PROFILE_MISSING_CONFIG_DIR;
   }
 
-  // Reread aux config file -- it may have changed
-  read_aux_config(config);
+
+  // Check for existence of semaphore file
+  std::string semaphore_filepath = config.getStringValue("config_directory")
+                                   + "/" + SEMAPHORE_FILENAME;
+  if (access(semaphore_filepath.c_str(), F_OK) == -1) {
+    return DONT_PROFILE_MISSING_SEMAPHORE;
+  }
 
   // Check for existence of simpleperf/perf executable
   std::string pp = config.getStringValue("perf_path");
   if (access(pp.c_str(), R_OK|X_OK) == -1) {
     W_ALOGW("unable to access/execute %s", pp.c_str());
-    closedir(dir);
     return DONT_PROFILE_MISSING_PERF_EXECUTABLE;
   }
 
-  // Check for existence of semaphore file
-  unsigned found = 0;
-  struct dirent* e;
-  while ((e = readdir(dir)) != 0) {
-    if (!strcmp(e->d_name, SEMAPHORE_FILENAME)) {
-      found = 1;
-      break;
-    }
-  }
-  closedir(dir);
-  if (!found) {
-    return DONT_PROFILE_MISSING_SEMAPHORE;
-  }
-
   //
   // We are good to go
   //
@@ -782,9 +747,7 @@ static void set_seed(ConfigReader &config)
 //
 static void init(ConfigReader &config)
 {
-  if (config_file_path != NULL) {
-    config.readFile(config_file_path);
-  }
+  config.readFile();
   set_seed(config);
 
   char propBuf[PROPERTY_VALUE_MAX];
@@ -814,7 +777,6 @@ int perfprofd_main(int argc, char** argv)
 
   parse_args(argc, argv);
   init(config);
-  read_aux_config(config);
 
   // Early exit if we're not supposed to run on this build flavor
   if (is_debug_build != 1 &&
@@ -837,11 +799,7 @@ int perfprofd_main(int argc, char** argv)
 
     // Reread config file if someone sent a SIGHUP
     if (please_reread_config_file) {
-      if (config_file_path) {
-        config.readFile(config_file_path);
-      } else {
-        read_aux_config(config);
-      }
+      config.readFile();
       please_reread_config_file = 0;
     }
 
diff --git a/perfprofd/tests/perfprofd_test.cc b/perfprofd/tests/perfprofd_test.cc
index b70dd86e..72667324 100644
--- a/perfprofd/tests/perfprofd_test.cc
+++ b/perfprofd/tests/perfprofd_test.cc
@@ -70,7 +70,6 @@ class PerfProfdTest : public testing::Test {
 
   virtual void TearDown() {
     mock_perfprofdutils_finish();
-    remove_dest_dir();
   }
 
   void noclean() {
@@ -96,11 +95,6 @@ class PerfProfdTest : public testing::Test {
     system(cmd.c_str());
   }
 
-  void remove_dest_dir() {
-    setup_dirs();
-    ASSERT_FALSE(dest_dir == "");
-  }
-
   void setup_dirs()
   {
     if (test_dir == "") {
@@ -147,10 +141,8 @@ class PerfProfdRunner {
  public:
   PerfProfdRunner()
       : config_path_(test_dir)
-      , aux_config_path_(dest_dir)
   {
     config_path_ += "/" CONFIGFILE;
-    aux_config_path_ += "/" CONFIGFILE;
   }
 
   ~PerfProfdRunner()
@@ -163,22 +155,16 @@ class PerfProfdRunner {
     config_text_ += "\n";
   }
 
-  void addToAuxConfig(const std::string &line)
-  {
-    aux_config_text_ += line;
-    aux_config_text_ += "\n";
-  }
-
   void remove_semaphore_file()
   {
-    std::string semaphore(dest_dir);
+    std::string semaphore(test_dir);
     semaphore += "/" SEMAPHORE_FILENAME;
     unlink(semaphore.c_str());
   }
 
   void create_semaphore_file()
   {
-    std::string semaphore(dest_dir);
+    std::string semaphore(test_dir);
     semaphore += "/" SEMAPHORE_FILENAME;
     close(open(semaphore.c_str(), O_WRONLY|O_CREAT));
   }
@@ -189,10 +175,6 @@ class PerfProfdRunner {
     argv[2] = config_path_.c_str();
 
     writeConfigFile(config_path_, config_text_);
-    if (aux_config_text_.length()) {
-      writeConfigFile(aux_config_path_, aux_config_text_);
-    }
-
 
     // execute daemon main
     return perfprofd_main(3, (char **) argv);
@@ -201,8 +183,6 @@ class PerfProfdRunner {
  private:
   std::string config_path_;
   std::string config_text_;
-  std::string aux_config_path_;
-  std::string aux_config_text_;
 
   void writeConfigFile(const std::string &config_path,
                        const std::string &config_text)
@@ -300,14 +280,14 @@ TEST_F(PerfProfdTest, MissingGMS)
   //
   // AWP requires cooperation between the daemon and the GMS core
   // piece. If we're running on a device that has an old or damaged
-  // version of GMS core, then the directory we're interested in may
-  // not be there. This test insures that the daemon does the right
-  // thing in this case.
+  // version of GMS core, then the config directory we're interested in
+  // may not be there. This test insures that the daemon does the
+  // right thing in this case.
   //
   PerfProfdRunner runner;
   runner.addToConfig("only_debug_build=0");
   runner.addToConfig("trace_config_read=1");
-  runner.addToConfig("destination_directory=/does/not/exist");
+  runner.addToConfig("config_directory=/does/not/exist");
   runner.addToConfig("main_loop_iterations=1");
   runner.addToConfig("use_fixed_seed=1");
   runner.addToConfig("collection_interval=100");
@@ -320,26 +300,17 @@ TEST_F(PerfProfdTest, MissingGMS)
 
   // Verify log contents
   const std::string expected = RAW_RESULT(
-      I: starting Android Wide Profiling daemon
-      I: config file path set to /data/nativetest/perfprofd_test/perfprofd.conf
-      I: option destination_directory set to /does/not/exist
-      I: option main_loop_iterations set to 1
-      I: option use_fixed_seed set to 1
-      I: option collection_interval set to 100
-      I: random seed set to 1
       I: sleep 90 seconds
-      W: unable to open destination directory /does/not/exist: (No such file or directory)
-      I: profile collection skipped (missing destination directory)
-      I: sleep 10 seconds
-      I: finishing Android Wide Profiling daemon
-                                          );\
+      W: unable to open config directory /does/not/exist: (No such file or directory)
+      I: profile collection skipped (missing config directory)
+                                          );
 
   // check to make sure entire log matches
-  bool compareEntireLog = true;
   compareLogMessages(mock_perfprofdutils_getlogged(),
-                     expected, "MissingGMS", compareEntireLog);
+                     expected, "MissingGMS");
 }
 
+
 TEST_F(PerfProfdTest, MissingOptInSemaphoreFile)
 {
   //
@@ -351,6 +322,8 @@ TEST_F(PerfProfdTest, MissingOptInSemaphoreFile)
   //
   PerfProfdRunner runner;
   runner.addToConfig("only_debug_build=0");
+  std::string cfparam("config_directory="); cfparam += test_dir;
+  runner.addToConfig(cfparam);
   std::string ddparam("destination_directory="); ddparam += dest_dir;
   runner.addToConfig(ddparam);
   runner.addToConfig("main_loop_iterations=1");
@@ -385,6 +358,8 @@ TEST_F(PerfProfdTest, MissingPerfExecutable)
   PerfProfdRunner runner;
   runner.addToConfig("only_debug_build=0");
   runner.addToConfig("trace_config_read=1");
+  std::string cfparam("config_directory="); cfparam += test_dir;
+  runner.addToConfig(cfparam);
   std::string ddparam("destination_directory="); ddparam += dest_dir;
   runner.addToConfig(ddparam);
   runner.addToConfig("main_loop_iterations=1");
@@ -420,6 +395,8 @@ TEST_F(PerfProfdTest, BadPerfRun)
   //
   PerfProfdRunner runner;
   runner.addToConfig("only_debug_build=0");
+  std::string cfparam("config_directory="); cfparam += test_dir;
+  runner.addToConfig(cfparam);
   std::string ddparam("destination_directory="); ddparam += dest_dir;
   runner.addToConfig(ddparam);
   runner.addToConfig("main_loop_iterations=1");
@@ -485,58 +462,6 @@ TEST_F(PerfProfdTest, ConfigFileParsing)
                      expected, "ConfigFileParsing");
 }
 
-TEST_F(PerfProfdTest, AuxiliaryConfigFile)
-{
-  //
-  // We want to be able to tweak profile collection parameters (sample
-  // duration, etc) using changes to gservices. To carry this out, the
-  // GMS core upload service writes out an perfprofd.conf config file when
-  // it starts up. This test verifies that we can read this file.
-  //
-
-  // Minimal settings in main config file
-  PerfProfdRunner runner;
-  runner.addToConfig("only_debug_build=0");
-  runner.addToConfig("trace_config_read=1");
-  runner.addToConfig("use_fixed_seed=1");
-  std::string ddparam("destination_directory="); ddparam += dest_dir;
-  runner.addToConfig(ddparam);
-
-  // Remaining settings in aux config file
-  runner.addToAuxConfig("main_loop_iterations=1");
-  runner.addToAuxConfig("collection_interval=100");
-  runner.addToAuxConfig("perf_path=/system/bin/true");
-  runner.addToAuxConfig("stack_profile=1");
-  runner.addToAuxConfig("sampling_period=9999");
-  runner.addToAuxConfig("sample_duration=333");
-
-  runner.remove_semaphore_file();
-
-  // Kick off daemon
-  int daemon_main_return_code = runner.invoke();
-
-  // Check return code from daemon
-  EXPECT_EQ(0, daemon_main_return_code);
-
-  // Verify log contents
-  const std::string expected = RAW_RESULT(
-      I: reading auxiliary config file /data/nativetest/perfprofd_test/tmp/perfprofd.conf
-      I: option main_loop_iterations set to 1
-      I: option collection_interval set to 100
-      I: option perf_path set to /system/bin/true
-      I: option stack_profile set to 1
-      I: option sampling_period set to 9999
-      I: option sample_duration set to 333
-      I: sleep 90 seconds
-      I: reading auxiliary config file /data/nativetest/perfprofd_test/tmp/perfprofd.conf
-      I: option main_loop_iterations set to 1
-                                          );
-
-  // check to make sure log excerpt matches
-  compareLogMessages(mock_perfprofdutils_getlogged(),
-                     expected, "AuxiliaryConfigFile");
-}
-
 TEST_F(PerfProfdTest, BasicRunWithCannedPerf)
 {
   //
@@ -616,6 +541,8 @@ TEST_F(PerfProfdTest, BasicRunWithLivePerf)
   runner.addToConfig("only_debug_build=0");
   std::string ddparam("destination_directory="); ddparam += dest_dir;
   runner.addToConfig(ddparam);
+  std::string cfparam("config_directory="); cfparam += test_dir;
+  runner.addToConfig(cfparam);
   runner.addToConfig("main_loop_iterations=1");
   runner.addToConfig("use_fixed_seed=12345678");
   runner.addToConfig("collection_interval=9999");