Fix current problem with machine locks.

Currently if crosperf can't access the default locks directory, it
assumes that it is safe to proceed without locks and it uses machines
without checking locks.  This script does two things:  It causes
crosperf to fail if it can't access a locks directory; and it allows
the user to specify an alternate locks directory.  As a by-product it
also adds a boolean prompting function to utils/misc.py

BUG=None
TEST=Tested with default directory accessible & inaccessible; tested using
local locks directory.

Change-Id: I6f24f772986813bf089f5f4e40335a15e8ee4837
Reviewed-on: https://chrome-internal-review.googlesource.com/214129
Reviewed-by: Luis Lozano <llozano@chromium.org>
Commit-Queue: Caroline Tice <cmtice@google.com>
Tested-by: Caroline Tice <cmtice@google.com>

4 files changed, 41 insertions, 19 deletions

diff --git a/crosperf/experiment.py b/crosperf/experiment.py
index d56603ab..e34c84f8 100644
--- a/crosperf/experiment.py
+++ b/crosperf/experiment.py
@@ -24,7 +24,7 @@ class Experiment(object):
   def __init__(self, name, remote, working_directory,
                chromeos_root, cache_conditions, labels, benchmarks,
                experiment_file, email_to, acquire_timeout, log_dir,
-               log_level, share_cache, results_directory):
+               log_level, share_cache, results_directory, locks_directory):
     self.name = name
     self.working_directory = working_directory
     self.remote = remote
@@ -60,7 +60,7 @@ class Experiment(object):
                                                 log_level)
     else:
       self.machine_manager = MachineManager(chromeos_root, acquire_timeout,
-                                            log_level)
+                                            log_level, locks_directory)
     self.l = logger.GetLogger(log_dir)
 
     for machine in remote:
diff --git a/crosperf/experiment_factory.py b/crosperf/experiment_factory.py
index a03bbcc8..eb4aa7ea 100644
--- a/crosperf/experiment_factory.py
+++ b/crosperf/experiment_factory.py
@@ -112,6 +112,7 @@ class ExperimentFactory(object):
     config.AddConfig("no_email", global_settings.GetField("no_email"))
     share_cache = global_settings.GetField("share_cache")
     results_dir = global_settings.GetField("results_dir")
+    locks_dir = global_settings.GetField("locks_dir")
     chrome_src = global_settings.GetField("chrome_src")
     show_all_results = global_settings.GetField("show_all_results")
     log_level = global_settings.GetField("logging_level")
@@ -220,7 +221,7 @@ class ExperimentFactory(object):
                             experiment_file.Canonicalize(),
                             email, acquire_timeout, log_dir, log_level,
                             share_cache,
-                            results_dir)
+                            results_dir, locks_dir)
 
     return experiment
 
diff --git a/crosperf/machine_manager.py b/crosperf/machine_manager.py
index ad214a27..78438839 100644
--- a/crosperf/machine_manager.py
+++ b/crosperf/machine_manager.py
@@ -17,6 +17,7 @@ import time
 
 from utils import command_executer
 from utils import logger
+from utils import misc
 from utils.file_utils import FileUtils
 
 from image_checksummer import ImageChecksummer
@@ -26,6 +27,9 @@ CHECKSUM_FILE = "/usr/local/osimage_checksum_file"
 class NonMatchingMachines(Exception):
   pass
 
+class MissingLocksDirectory(Exception):
+    """Raised when cannot find/access the machine locks directory."""
+
 class CrosMachine(object):
   def __init__(self, name, chromeos_root, log_level, cmd_exec=None):
     self.name = name
@@ -168,8 +172,8 @@ class CrosMachine(object):
 
 
 class MachineManager(object):
-  def __init__(self, chromeos_root, acquire_timeout, log_level, cmd_exec=None,
-               lgr=None):
+  def __init__(self, chromeos_root, acquire_timeout, log_level, locks_dir,
+               cmd_exec=None, lgr=None):
     self._lock = threading.RLock()
     self._all_machines = []
     self._machines = []
@@ -180,14 +184,20 @@ class MachineManager(object):
     self.machine_checksum_string = {}
     self.acquire_timeout = acquire_timeout
     self.log_level = log_level
+    self.locks_dir = locks_dir
     self.ce = cmd_exec or command_executer.GetCommandExecuter(
         log_level=self.log_level)
     self.logger = lgr or logger.GetLogger()
 
-    if os.path.isdir(lock_machine.Machine.LOCKS_DIR):
-      self.no_lock = False
-    else:
-      self.no_lock = True
+    if self.locks_dir != lock_machine.Machine.LOCKS_DIR:
+      msg = ("WARNING:  If you use your own locks directory, there is no"
+             " guarantee that someone else might not hold a lock on the same"
+             " machine in a different locks directory.")
+      self.logger.LogOutput(msg)
+
+    if not os.path.isdir(self.locks_dir):
+      raise MissingLocksDirectory ("Cannot access locks directory: %s"
+                                   % self.locks_dir)
     self._initialized_machines = []
     self.chromeos_root = chromeos_root
 
@@ -265,10 +275,9 @@ class MachineManager(object):
       for m in self._machines:
         if m.name == cros_machine.name:
           return
-      if self.no_lock:
-        locked = True
-      else:
-        locked = lock_machine.Machine(cros_machine.name).Lock(True, sys.argv[0])
+      locked = lock_machine.Machine(cros_machine.name,
+                                    self.locks_dir).Lock(True,
+                                                         sys.argv[0])
       if locked:
         self._machines.append(cros_machine)
         command = "cat %s" % CHECKSUM_FILE
@@ -304,7 +313,9 @@ class MachineManager(object):
     with self._lock:
       self._machines = [m for m in self._machines
                         if m.name != machine_name]
-      res = lock_machine.Machine(machine_name).Unlock(True)
+      res = lock_machine.Machine(machine_name,
+                                 self.locks_dir).Unlock(True)
+
       if not res:
         self.logger.LogError("Could not unlock machine: '%s'."
                              % m.name)
@@ -413,11 +424,11 @@ class MachineManager(object):
     with self._lock:
       # Unlock all machines.
       for m in self._machines:
-        if not self.no_lock:
-          res = lock_machine.Machine(m.name).Unlock(True)
-          if not res:
-            self.logger.LogError("Could not unlock machine: '%s'."
-                                 % m.name)
+        res = lock_machine.Machine(m.name, self.locks_dir).Unlock(True)
+
+        if not res:
+          self.logger.LogError("Could not unlock machine: '%s'."
+                               % m.name)
 
   def __str__(self):
     with self._lock:
diff --git a/crosperf/settings_factory.py b/crosperf/settings_factory.py
index d86dc2bc..835748a3 100644
--- a/crosperf/settings_factory.py
+++ b/crosperf/settings_factory.py
@@ -6,6 +6,8 @@
 
 """Setting files for global, benchmark and labels."""
 
+import lock_machine
+
 from field import BooleanField
 from field import FloatField
 from field import IntegerField
@@ -133,6 +135,14 @@ class GlobalSettings(Settings):
                             " se[arated bu a \",\""))
     self.AddField(TextField("results_dir", default="",
                             description="The results dir"))
+    default_locks_dir = lock_machine.Machine.LOCKS_DIR
+    self.AddField(TextField("locks_dir", default=default_locks_dir,
+                            description="An alternate directory to use for "
+                            "storing/checking machine locks.\n"
+                            "WARNING:  If you use your own locks directory, "
+                            "there is no guarantee that someone else might not "
+                            "hold a lock on the same machine in a different "
+                            "locks directory."))
     self.AddField(TextField("chrome_src",
                             description="The path to the source of chrome. "
                             "This is used to run telemetry benchmarks. "