Merge "Do not check all CF configs if instance names are specified"
diff --git a/delete/delete.py b/delete/delete.py
index 055fb4b..02260fd 100644
--- a/delete/delete.py
+++ b/delete/delete.py
@@ -250,8 +250,8 @@
     remote_instances = list(set(instances) - set(local_instances))
     if local_instances:
         utils.PrintColorString("Deleting local instances")
-        delete_report = DeleteInstances(cfg, list_instances.FilterInstancesByNames(
-            list_instances.GetLocalInstances(), local_instances))
+        delete_report = DeleteInstances(
+            cfg, list_instances.GetLocalInstancesByNames(local_instances))
     if remote_instances:
         delete_report = DeleteRemoteInstances(cfg,
                                               remote_instances,
diff --git a/delete/delete_test.py b/delete/delete_test.py
index cf17905..f8273ca 100644
--- a/delete/delete_test.py
+++ b/delete/delete_test.py
@@ -127,8 +127,7 @@
         cfg = mock.Mock()
         # Test delete local instances.
         instances = ["local-instance-1", "local-instance-2"]
-        self.Patch(list_instances, "FilterInstancesByNames", return_value="")
-        self.Patch(list_instances, "GetLocalInstances", return_value=[])
+        self.Patch(list_instances, "GetLocalInstancesByNames", return_value=[])
         delete.DeleteInstanceByNames(cfg, instances)
         mock_delete_local_ins.assert_called()
 
diff --git a/internal/constants.py b/internal/constants.py
index b507734..0d412a0 100755
--- a/internal/constants.py
+++ b/internal/constants.py
@@ -165,7 +165,6 @@
 INS_KEY_IS_LOCAL = "remote"
 INS_KEY_ZONE = "zone"
 INS_STATUS_RUNNING = "RUNNING"
-LOCAL_INS_NAME = "local-instance"
 ENV_CUTTLEFISH_CONFIG_FILE = "CUTTLEFISH_CONFIG_FILE"
 ENV_CUTTLEFISH_INSTANCE = "CUTTLEFISH_INSTANCE"
 ENV_CVD_HOME = "HOME"
diff --git a/list/instance.py b/list/instance.py
index 2cc51ca..10063c4 100644
--- a/list/instance.py
+++ b/list/instance.py
@@ -50,6 +50,8 @@
 _ACLOUD_CVD_TEMP = os.path.join(tempfile.gettempdir(), "acloud_cvd_temp")
 _CVD_RUNTIME_FOLDER_NAME = "cuttlefish_runtime"
 _CVD_STATUS_BIN = "cvd_status"
+_LOCAL_INSTANCE_NAME_FORMAT = "local-instance-%(id)d"
+_LOCAL_INSTANCE_NAME_PATTERN = re.compile(r"^local-instance-(?P<id>\d+)$")
 _MSG_UNABLE_TO_CALCULATE = "Unable to calculate"
 _NO_ANDROID_ENV = "android source not available"
 _RE_GROUP_ADB = "local_adb_port"
@@ -77,8 +79,11 @@
     Return:
         String, path of cf runtime config.
     """
-    return os.path.join(os.path.expanduser("~"), _CVD_RUNTIME_FOLDER_NAME,
-                        constants.CUTTLEFISH_CONFIG_FILE)
+    cfg_path = os.path.join(os.path.expanduser("~"), _CVD_RUNTIME_FOLDER_NAME,
+                            constants.CUTTLEFISH_CONFIG_FILE)
+    if os.path.isfile(cfg_path):
+        return cfg_path
+    return None
 
 
 def GetLocalInstanceName(local_instance_id):
@@ -90,7 +95,23 @@
     Return:
         String, the instance name.
     """
-    return "%s-%d" % (constants.LOCAL_INS_NAME, local_instance_id)
+    return _LOCAL_INSTANCE_NAME_FORMAT % {"id": local_instance_id}
+
+
+def GetLocalInstanceIdByName(name):
+    """Get local cuttlefish instance id by name.
+
+    Args:
+        name: String of instance name.
+
+    Return:
+        The instance id as an integer if the name is in valid format.
+        None if the name does not represent a local cuttlefish instance.
+    """
+    match = _LOCAL_INSTANCE_NAME_PATTERN.match(name)
+    if match:
+        return int(match.group("id"))
+    return None
 
 
 def GetLocalInstanceConfig(local_instance_id):
@@ -110,27 +131,27 @@
 
 
 def GetAllLocalInstanceConfigs():
-    """Get the list of instance config.
+    """Get all cuttlefish runtime configs from the known locations.
 
     Return:
-        List of instance config path.
+        List of tuples. Each tuple consists of an instance id and a config
+        path.
     """
-    cfg_list = []
+    id_cfg_pairs = []
     # Check if any instance config is under home folder.
     cfg_path = GetDefaultCuttlefishConfig()
-    if os.path.isfile(cfg_path):
-        cfg_list.append(cfg_path)
+    if cfg_path:
+        id_cfg_pairs.append((1, cfg_path))
 
     # Check if any instance config is under acloud cvd temp folder.
     if os.path.exists(_ACLOUD_CVD_TEMP):
         for ins_name in os.listdir(_ACLOUD_CVD_TEMP):
-            cfg_path = os.path.join(_ACLOUD_CVD_TEMP,
-                                    ins_name,
-                                    _CVD_RUNTIME_FOLDER_NAME,
-                                    constants.CUTTLEFISH_CONFIG_FILE)
-            if os.path.isfile(cfg_path):
-                cfg_list.append(cfg_path)
-    return cfg_list
+            ins_id = GetLocalInstanceIdByName(ins_name)
+            if ins_id is not None:
+                cfg_path = GetLocalInstanceConfig(ins_id)
+                if os.path.isfile(cfg_path):
+                    id_cfg_pairs.append((ins_id, cfg_path))
+    return id_cfg_pairs
 
 
 def GetLocalInstanceHomeDir(local_instance_id):
diff --git a/list/instance_test.py b/list/instance_test.py
index f51cb57..f5d2575 100644
--- a/list/instance_test.py
+++ b/list/instance_test.py
@@ -18,7 +18,6 @@
 import collections
 import datetime
 import subprocess
-
 import unittest
 from six import b
 
@@ -77,12 +76,12 @@
                    return_value=cf_config)
         local_instance = instance.LocalInstance(cf_config)
 
-        self.assertEqual(constants.LOCAL_INS_NAME + "-2", local_instance.name)
+        self.assertEqual("local-instance-2", local_instance.name)
         self.assertEqual(True, local_instance.islocal)
         self.assertEqual("1080x1920 (480)", local_instance.display)
         expected_full_name = ("device serial: 127.0.0.1:%s (%s) elapsed time: %s"
                               % ("6521",
-                                 constants.LOCAL_INS_NAME + "-2",
+                                 "local-instance-2",
                                  "None"))
         self.assertEqual(expected_full_name, local_instance.fullname)
         self.assertEqual(6521, local_instance.adb_port)
diff --git a/list/list.py b/list/list.py
index c60bcb6..42487f9 100644
--- a/list/list.py
+++ b/list/list.py
@@ -127,28 +127,44 @@
     return _SortInstancesForDisplay(_ProcessInstances(all_instances))
 
 
-def _GetLocalCuttlefishInstances():
+def _GetLocalCuttlefishInstances(id_cfg_pairs):
     """Look for local cuttelfish instances.
 
     Gather local instances information from cuttlefish runtime config.
 
+    Args:
+        id_cfg_pairs: List of tuples. Each tuple consists of an instance id and
+                      a config path.
+
     Returns:
         instance_list: List of local instances.
     """
     local_instance_list = []
-    for cf_runtime_config_path in instance.GetAllLocalInstanceConfigs():
-        ins = instance.LocalInstance(cf_runtime_config_path)
-        if ins.CvdStatus():
-            local_instance_list.append(ins)
-        else:
-            logger.info("cvd runtime config found but instance is not active:%s"
-                        , cf_runtime_config_path)
+    for ins_id, cfg_path in id_cfg_pairs:
+        ins_lock = instance.GetLocalInstanceLock(ins_id)
+        if not ins_lock.Lock():
+            logger.warning("Cuttlefish Instance %d is locked by another "
+                           "process.", ins_id)
+            continue
+        try:
+            if not os.path.isfile(cfg_path):
+                continue
+            ins = instance.LocalInstance(cfg_path)
+            if ins.CvdStatus():
+                local_instance_list.append(ins)
+            else:
+                logger.info("Cvd runtime config is found at %s but instance "
+                            "%d is not active.", cfg_path, ins_id)
+        finally:
+            ins_lock.Unlock()
     return local_instance_list
 
 
 def GetActiveCVD(local_instance_id):
     """Check if the local AVD with specific instance id is running
 
+    This function does not lock the instance.
+
     Args:
         local_instance_id: Integer of instance id.
 
@@ -161,7 +177,7 @@
         if ins.CvdStatus():
             return ins
     cfg_path = instance.GetDefaultCuttlefishConfig()
-    if local_instance_id == 1 and os.path.isfile(cfg_path):
+    if local_instance_id == 1 and cfg_path:
         ins = instance.LocalInstance(cfg_path)
         if ins.CvdStatus():
             return ins
@@ -178,7 +194,8 @@
     if not utils.IsSupportedPlatform():
         return []
 
-    return (_GetLocalCuttlefishInstances() +
+    id_cfg_pairs = instance.GetAllLocalInstanceConfigs()
+    return (_GetLocalCuttlefishInstances(id_cfg_pairs) +
             instance.LocalGoldfishInstance.GetExistingInstances())
 
 
@@ -258,7 +275,7 @@
     return instances_list[0]
 
 
-def FilterInstancesByNames(instances, names):
+def _FilterInstancesByNames(instances, names):
     """Find instances by names.
 
     Args:
@@ -286,6 +303,37 @@
     return found_instances
 
 
+def GetLocalInstancesByNames(names):
+    """Get local cuttlefish and goldfish instances by names.
+
+    Args:
+        names: Collection of instance names.
+
+    Returns:
+        List consisting of LocalInstance and LocalGoldfishInstance objects.
+
+    Raises:
+        errors.NoInstancesFound: No instances found.
+    """
+    id_cfg_pairs = []
+    for name in names:
+        ins_id = instance.GetLocalInstanceIdByName(name)
+        if ins_id is None:
+            continue
+        cfg_path = instance.GetLocalInstanceConfig(ins_id)
+        if cfg_path:
+            id_cfg_pairs.append((ins_id, cfg_path))
+        if ins_id == 1:
+            cfg_path = instance.GetDefaultCuttlefishConfig()
+            if cfg_path:
+                id_cfg_pairs.append((ins_id, cfg_path))
+
+    return _FilterInstancesByNames(
+        _GetLocalCuttlefishInstances(id_cfg_pairs) +
+        instance.LocalGoldfishInstance.GetExistingInstances(),
+        names)
+
+
 def GetInstancesFromInstanceNames(cfg, instance_names):
     """Get instances from instance names.
 
@@ -301,7 +349,9 @@
     Raises:
         errors.NoInstancesFound: No instances found.
     """
-    return FilterInstancesByNames(GetInstances(cfg), instance_names)
+    return _FilterInstancesByNames(
+        GetLocalInstancesByNames(instance_names) + GetRemoteInstances(cfg),
+        instance_names)
 
 
 def FilterInstancesByAdbPort(instances, adb_port):
diff --git a/list/list_test.py b/list/list_test.py
index a4b466c..c6d04bd 100644
--- a/list/list_test.py
+++ b/list/list_test.py
@@ -25,7 +25,7 @@
 from acloud.list import instance
 
 
-class InstanceObject(object):
+class InstanceObject:
     """Mock to store data of instance."""
 
     def __init__(self, name):
@@ -44,8 +44,10 @@
         alive_instance2 = InstanceObject("alive_instance2")
         alive_local_instance = InstanceObject("alive_local_instance")
 
-        instance_alive = [alive_instance1, alive_instance2, alive_local_instance]
-        self.Patch(list_instance, "GetInstances", return_value=instance_alive)
+        self.Patch(list_instance, "GetLocalInstancesByNames",
+                   return_value=[alive_local_instance])
+        self.Patch(list_instance, "GetRemoteInstances",
+                   return_value=[alive_instance1, alive_instance2])
         instances_list = list_instance.GetInstancesFromInstanceNames(cfg, instance_names)
         instances_name_in_list = [instance_object.name for instance_object in instances_list]
         self.assertEqual(instances_name_in_list.sort(), instance_names.sort())
@@ -58,7 +60,7 @@
         # test get instance from instance name error with invalid input.
         instance_names = ["miss2_local_instance", "alive_instance1"]
         miss_instance_names = ["miss2_local_instance"]
-        self.assertRaisesRegexp(
+        self.assertRaisesRegex(
             errors.NoInstancesFound,
             "Did not find the following instances: %s" % ' '.join(miss_instance_names),
             list_instance.GetInstancesFromInstanceNames,
@@ -88,6 +90,31 @@
         expected_instance = "cf_instance2"
         self.assertEqual(list_instance.ChooseOneRemoteInstance(cfg), expected_instance)
 
+    def testGetLocalInstancesByNames(self):
+        """test GetLocalInstancesByNames."""
+        self.Patch(
+            instance, "GetLocalInstanceIdByName",
+            side_effect=lambda name: 1 if name == "local-instance-1" else None)
+        self.Patch(instance, "GetLocalInstanceConfig",
+                   return_value="path1")
+        self.Patch(instance, "GetDefaultCuttlefishConfig",
+                   return_value="path2")
+        mock_ins = mock.Mock()
+        mock_ins.name = "local-instance-1"
+        mock_get_cf = self.Patch(list_instance,
+                                 "_GetLocalCuttlefishInstances",
+                                 return_value=[mock_ins])
+        self.Patch(instance.LocalGoldfishInstance, "GetExistingInstances",
+                   return_value=[])
+
+        ins_list = list_instance.GetLocalInstancesByNames(["local-instance-1"])
+        self.assertEqual(1, len(ins_list))
+        mock_get_cf.assert_called_with([(1, "path1"), (1, "path2")])
+
+        with self.assertRaises(errors.NoInstancesFound):
+            ins_list = list_instance.GetLocalInstancesByNames(
+                ["local-instance-1", "local-instance-6"])
+
     # pylint: disable=attribute-defined-outside-init
     def testFilterInstancesByAdbPort(self):
         """test FilterInstancesByAdbPort."""
@@ -107,22 +134,42 @@
     def testGetLocalCuttlefishInstances(self):
         """test _GetLocalCuttlefishInstances."""
         # Test getting two instance case
-        self.Patch(instance, "GetAllLocalInstanceConfigs",
-                   return_value=["fake_path1", "fake_path2"])
-        self.Patch(instance, "GetLocalInstanceRuntimeDir")
+        id_cfg_pairs = [(1, "fake_path1"), (2, "fake_path2")]
+        mock_isfile = self.Patch(list_instance.os.path, "isfile",
+                                 return_value=True)
+
+        mock_lock = mock.Mock()
+        mock_lock.Lock.return_value = True
+        self.Patch(instance, "GetLocalInstanceLock", return_value=mock_lock)
 
         local_ins = mock.MagicMock()
         local_ins.CvdStatus.return_value = True
         self.Patch(instance, "LocalInstance", return_value=local_ins)
 
-        ins_list = list_instance._GetLocalCuttlefishInstances()
+        ins_list = list_instance._GetLocalCuttlefishInstances(id_cfg_pairs)
         self.assertEqual(2, len(ins_list))
+        mock_isfile.assert_called()
+        local_ins.CvdStatus.assert_called()
+        self.assertEqual(2, mock_lock.Lock.call_count)
+        self.assertEqual(2, mock_lock.Unlock.call_count)
 
-        local_ins = mock.MagicMock()
-        local_ins.CvdStatus.return_value = False
-        self.Patch(instance, "LocalInstance", return_value=local_ins)
-        ins_list = list_instance._GetLocalCuttlefishInstances()
+        local_ins.CvdStatus.reset_mock()
+        mock_lock.Lock.reset_mock()
+        mock_lock.Lock.return_value = False
+        mock_lock.Unlock.reset_mock()
+        ins_list = list_instance._GetLocalCuttlefishInstances(id_cfg_pairs)
         self.assertEqual(0, len(ins_list))
+        local_ins.CvdStatus.assert_not_called()
+        self.assertEqual(2, mock_lock.Lock.call_count)
+        mock_lock.Unlock.assert_not_called()
+
+        mock_lock.Lock.reset_mock()
+        mock_lock.Lock.return_value = True
+        local_ins.CvdStatus.return_value = False
+        ins_list = list_instance._GetLocalCuttlefishInstances(id_cfg_pairs)
+        self.assertEqual(0, len(ins_list))
+        self.assertEqual(2, mock_lock.Lock.call_count)
+        self.assertEqual(2, mock_lock.Unlock.call_count)
 
     # pylint: disable=no-member
     def testPrintInstancesDetails(self):