Convert SetInstanceParams to concurrency

[ganeti-local] / lib / cmdlib.py

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index 0d6e655..1fa6be7 100644 (file)
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -37,6 +37,7 @@ from ganeti import logger
 from ganeti import utils
 from ganeti import errors
 from ganeti import hypervisor
 from ganeti import utils
 from ganeti import errors
 from ganeti import hypervisor

+from ganeti import locking

 from ganeti import config
 from ganeti import constants
 from ganeti import objects
 from ganeti import config
 from ganeti import constants
 from ganeti import objects

@@ -49,8 +50,8 @@ class LogicalUnit(object):
   """Logical Unit base class.
 
   Subclasses must follow these rules:
   """Logical Unit base class.
 
   Subclasses must follow these rules:

-    - implement CheckPrereq which also fills in the opcode instance
-      with all the fields (even if as None)
+    - implement ExpandNames
+    - implement CheckPrereq

     - implement Exec
     - implement BuildHooksEnv
     - redefine HPATH and HTYPE
     - implement Exec
     - implement BuildHooksEnv
     - redefine HPATH and HTYPE

@@ -81,6 +82,7 @@ class LogicalUnit(object):
     self.cfg = context.cfg
     self.sstore = sstore
     self.context = context
     self.cfg = context.cfg
     self.sstore = sstore
     self.context = context

+    self.needed_locks = None

     self.__ssh = None
 
     for attr_name in self._OP_REQP:
     self.__ssh = None
 
     for attr_name in self._OP_REQP:

@@ -108,6 +110,46 @@ class LogicalUnit(object):
 
   ssh = property(fget=__GetSSH)
 
 
   ssh = property(fget=__GetSSH)
 

+  def ExpandNames(self):
+    """Expand names for this LU.
+
+    This method is called before starting to execute the opcode, and it should
+    update all the parameters of the opcode to their canonical form (e.g. a
+    short node name must be fully expanded after this method has successfully
+    completed). This way locking, hooks, logging, ecc. can work correctly.
+
+    LUs which implement this method must also populate the self.needed_locks
+    member, as a dict with lock levels as keys, and a list of needed lock names
+    as values. Rules:
+      - Use an empty dict if you don't need any lock
+      - If you don't need any lock at a particular level omit that level
+      - Don't put anything for the BGL level
+      - If you want all locks at a level use None as a value
+        (this reflects what LockSet does, and will be replaced before
+        CheckPrereq with the full list of nodes that have been locked)
+
+    Examples:
+    # Acquire all nodes and one instance
+    self.needed_locks = {
+      locking.LEVEL_NODE: None,
+      locking.LEVEL_INSTANCES: ['instance1.example.tld'],
+    }
+    # Acquire just two nodes
+    self.needed_locks = {
+      locking.LEVEL_NODE: ['node1.example.tld', 'node2.example.tld'],
+    }
+    # Acquire no locks
+    self.needed_locks = {} # No, you can't leave it to the default value None
+
+    """
+    # The implementation of this method is mandatory only if the new LU is
+    # concurrent, so that old LUs don't need to be changed all at the same
+    # time.
+    if self.REQ_BGL:
+      self.needed_locks = {} # Exclusive LUs don't need locks.
+    else:
+      raise NotImplementedError
+

   def CheckPrereq(self):
     """Check prerequisites for this LU.
 
   def CheckPrereq(self):
     """Check prerequisites for this LU.
 

@@ -120,9 +162,7 @@ class LogicalUnit(object):
     not fulfilled. Its return value is ignored.
 
     This method should also update all the parameters of the opcode to
     not fulfilled. Its return value is ignored.
 
     This method should also update all the parameters of the opcode to

-    their canonical form; e.g. a short node name must be fully
-    expanded after this method has successfully completed (so that
-    hooks, logging, etc. work correctly).
+    their canonical form if it hasn't been done by ExpandNames before.

 
     """
     raise NotImplementedError
 
     """
     raise NotImplementedError

@@ -177,6 +217,28 @@ class LogicalUnit(object):
     """
     return lu_result
 
     """
     return lu_result
 

+  def _ExpandAndLockInstance(self):
+    """Helper function to expand and lock an instance.
+
+    Many LUs that work on an instance take its name in self.op.instance_name
+    and need to expand it and then declare the expanded name for locking. This
+    function does it, and then updates self.op.instance_name to the expanded
+    name. It also initializes needed_locks as a dict, if this hasn't been done
+    before.
+
+    """
+    if self.needed_locks is None:
+      self.needed_locks = {}
+    else:
+      assert locking.LEVEL_INSTANCE not in self.needed_locks, \
+        "_ExpandAndLockInstance called with instance-level locks set"
+    expanded_name = self.cfg.ExpandInstanceName(self.op.instance_name)
+    if expanded_name is None:
+      raise errors.OpPrereqError("Instance '%s' not known" %
+                                  self.op.instance_name)
+    self.needed_locks[locking.LEVEL_INSTANCE] = expanded_name
+    self.op.instance_name = expanded_name
+

 
 class NoHooksLU(LogicalUnit):
   """Simple LU which runs no hooks.
 
 class NoHooksLU(LogicalUnit):
   """Simple LU which runs no hooks.

@@ -1053,15 +1115,7 @@ def _WaitForSync(cfgw, instance, proc, oneshot=False, unlock=False):
     if done or oneshot:
       break
 
     if done or oneshot:
       break
 

-    if unlock:
-      #utils.Unlock('cmd')
-      pass
-    try:
-      time.sleep(min(60, max_time))
-    finally:
-      if unlock:
-        #utils.Lock('cmd')
-        pass
+    time.sleep(min(60, max_time))

 
   if done:
     proc.LogInfo("Instance %s's disks are in sync." % instance.name)
 
   if done:
     proc.LogInfo("Instance %s's disks are in sync." % instance.name)

@@ -1708,6 +1762,10 @@ class LUQueryClusterInfo(NoHooksLU):
   """
   _OP_REQP = []
   REQ_MASTER = False
   """
   _OP_REQP = []
   REQ_MASTER = False

+  REQ_BGL = False
+
+  def ExpandNames(self):
+    self.needed_locks = {}

 
   def CheckPrereq(self):
     """No prerequsites needed for this LU.
 
   def CheckPrereq(self):
     """No prerequsites needed for this LU.

@@ -1739,6 +1797,10 @@ class LUDumpClusterConfig(NoHooksLU):
 
   """
   _OP_REQP = []
 
   """
   _OP_REQP = []

+  REQ_BGL = False
+
+  def ExpandNames(self):
+    self.needed_locks = {}

 
   def CheckPrereq(self):
     """No prerequisites.
 
   def CheckPrereq(self):
     """No prerequisites.

@@ -3292,6 +3354,10 @@ class LUConnectConsole(NoHooksLU):
 
   """
   _OP_REQP = ["instance_name"]
 
   """
   _OP_REQP = ["instance_name"]

+  REQ_BGL = False
+
+  def ExpandNames(self):
+    self._ExpandAndLockInstance()

 
   def CheckPrereq(self):
     """Check prerequisites.
 
   def CheckPrereq(self):
     """Check prerequisites.

@@ -3299,12 +3365,9 @@ class LUConnectConsole(NoHooksLU):
     This checks that the instance is in the cluster.
 
     """
     This checks that the instance is in the cluster.
 
     """

-    instance = self.cfg.GetInstanceInfo(
-      self.cfg.ExpandInstanceName(self.op.instance_name))
-    if instance is None:
-      raise errors.OpPrereqError("Instance '%s' not known" %
-                                 self.op.instance_name)
-    self.instance = instance
+    self.instance = self.cfg.GetInstanceInfo(self.op.instance_name)
+    assert self.instance is not None, \
+      "Cannot retrieve locked instance %s" % self.op.instance_name

 
   def Exec(self, feedback_fn):
     """Connect to the console of an instance
 
   def Exec(self, feedback_fn):
     """Connect to the console of an instance

@@ -4034,6 +4097,10 @@ class LUSetInstanceParams(LogicalUnit):
   HPATH = "instance-modify"
   HTYPE = constants.HTYPE_INSTANCE
   _OP_REQP = ["instance_name"]
   HPATH = "instance-modify"
   HTYPE = constants.HTYPE_INSTANCE
   _OP_REQP = ["instance_name"]

+  REQ_BGL = False
+
+  def ExpandNames(self):
+    self._ExpandAndLockInstance()

 
   def BuildHooksEnv(self):
     """Build hooks env.
 
   def BuildHooksEnv(self):
     """Build hooks env.

@@ -4071,6 +4138,9 @@ class LUSetInstanceParams(LogicalUnit):
     This only checks the instance list against the existing names.
 
     """
     This only checks the instance list against the existing names.
 
     """

+    # FIXME: all the parameters could be checked before, in ExpandNames, or in
+    # a separate CheckArguments function, if we implement one, so the operation
+    # can be aborted without waiting for any lock, should it have an error...

     self.mem = getattr(self.op, "mem", None)
     self.vcpus = getattr(self.op, "vcpus", None)
     self.ip = getattr(self.op, "ip", None)
     self.mem = getattr(self.op, "mem", None)
     self.vcpus = getattr(self.op, "vcpus", None)
     self.ip = getattr(self.op, "ip", None)

@@ -4165,13 +4235,9 @@ class LUSetInstanceParams(LogicalUnit):
                                    " like a valid IP address" %
                                    self.op.vnc_bind_address)
 
                                    " like a valid IP address" %
                                    self.op.vnc_bind_address)
 

-    instance = self.cfg.GetInstanceInfo(
-      self.cfg.ExpandInstanceName(self.op.instance_name))
-    if instance is None:
-      raise errors.OpPrereqError("No such instance name '%s'" %
-                                 self.op.instance_name)
-    self.op.instance_name = instance.name
-    self.instance = instance
+    self.instance = self.cfg.GetInstanceInfo(self.op.instance_name)
+    assert self.instance is not None, \
+      "Cannot retrieve locked instance %s" % self.op.instance_name

     return
 
   def Exec(self, feedback_fn):
     return
 
   def Exec(self, feedback_fn):

@@ -4221,7 +4287,7 @@ class LUSetInstanceParams(LogicalUnit):
       instance.vnc_bind_address = self.vnc_bind_address
       result.append(("vnc_bind_address", self.vnc_bind_address))
 
       instance.vnc_bind_address = self.vnc_bind_address
       result.append(("vnc_bind_address", self.vnc_bind_address))
 

-    self.cfg.AddInstance(instance)
+    self.cfg.Update(instance)

 
     return result
 
 
     return result
 

@@ -4558,6 +4624,7 @@ class LUDelTags(TagsLU):
                                 " config file and the operation has been"
                                 " aborted. Please retry.")
 
                                 " config file and the operation has been"
                                 " aborted. Please retry.")
 

+

 class LUTestDelay(NoHooksLU):
   """Sleep for a specified amount of time.
 
 class LUTestDelay(NoHooksLU):
   """Sleep for a specified amount of time.
 

@@ -4566,17 +4633,26 @@ class LUTestDelay(NoHooksLU):
 
   """
   _OP_REQP = ["duration", "on_master", "on_nodes"]
 
   """
   _OP_REQP = ["duration", "on_master", "on_nodes"]

+  REQ_BGL = False

 
 

-  def CheckPrereq(self):
-    """Check prerequisites.
+  def ExpandNames(self):
+    """Expand names and set required locks.

 
 

-    This checks that we have a good list of nodes and/or the duration
-    is valid.
+    This expands the node list, if any.

 
     """
 
     """

-
+    self.needed_locks = {}

     if self.op.on_nodes:
     if self.op.on_nodes:

+      # _GetWantedNodes can be used here, but is not always appropriate to use
+      # this way in ExpandNames. Check LogicalUnit.ExpandNames docstring for
+      # more information.

       self.op.on_nodes = _GetWantedNodes(self, self.op.on_nodes)
       self.op.on_nodes = _GetWantedNodes(self, self.op.on_nodes)

+      self.needed_locks[locking.LEVEL_NODE] = self.op.on_nodes
+
+  def CheckPrereq(self):
+    """Check prerequisites.
+
+    """

 
   def Exec(self, feedback_fn):
     """Do the actual sleep.
 
   def Exec(self, feedback_fn):
     """Do the actual sleep.