From e71b9ef4ad58a224feedc415d78bcafa74956c90 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Thu, 15 Jul 2010 18:05:46 +0200
Subject: [PATCH] Add a migration type global hypervisor parameter

Since migration live/non-live is more stable (e.g.) for Xen-PVM versus
Xen-HVM, we introduce a new parameter for what mode we should use by
default (if not overridden by the user, in the opcode).

The meaning of the opcode 'live' field changes from boolean to either
None (use the hypervisor default), or one of the allowed migration
string constants. The live parameter of the TLMigrateInstance is still a
boolean, computed from the opcode field (which is no longer passed to
the TL).

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>
---
 lib/cli.py                |  7 +++++++
 lib/cmdlib.py             | 23 +++++++++++++++++------
 lib/constants.py          | 11 +++++++++++
 lib/hypervisor/hv_base.py |  5 +++++
 lib/hypervisor/hv_kvm.py  |  1 +
 lib/hypervisor/hv_xen.py  |  2 ++
 man/gnt-instance.sgml     | 25 ++++++++++++++++---------
 man/gnt-node.sgml         |  6 ++++--
 scripts/gnt-instance      | 14 ++++++++++++--
 scripts/gnt-node          | 13 +++++++++++--
 10 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/lib/cli.py b/lib/cli.py
index 1de60a02f..92877e2fd 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -87,6 +87,7 @@ __all__ = [
   "MAINTAIN_NODE_HEALTH_OPT",
   "MASTER_NETDEV_OPT",
   "MC_OPT",
+  "MIGRATION_TYPE_OPT",
   "NET_OPT",
   "NEW_CLUSTER_CERT_OPT",
   "NEW_CLUSTER_DOMAIN_SECRET_OPT",
@@ -698,6 +699,12 @@ NONLIVE_OPT = cli_option("--non-live", dest="live",
                          " freeze the instance, save the state, transfer and"
                          " only then resume running on the secondary node)")
 
+MIGRATION_TYPE_OPT = cli_option("--migration-type", dest="migration_type",
+                                default=None,
+                                choices=list(constants.HT_MIGRATION_TYPES),
+                                help="Override default migration type (choose"
+                                " either live or non-live")
+
 NODE_PLACEMENT_OPT = cli_option("-n", "--node", dest="node",
                                 help="Target node and optional secondary node",
                                 metavar="<pnode>[:<snode>]",
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index 37fcca58d..d65b8f540 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -232,6 +232,10 @@ _PInstanceName = ("instance_name", _NoDefault, _TNonEmptyString)
 #: a required node name (for single-node LUs)
 _PNodeName = ("node_name", _NoDefault, _TNonEmptyString)
 
+#: the migration type (live/non-live)
+_PMigrationLive = ("live", None, _TOr(_TNone,
+                                      _TElemOf(constants.HT_MIGRATION_TYPES)))
+
 
 # End types
 class LogicalUnit(object):
@@ -5486,7 +5490,7 @@ class LUMigrateInstance(LogicalUnit):
   HTYPE = constants.HTYPE_INSTANCE
   _OP_PARAMS = [
     _PInstanceName,
-    ("live", True, _TBool),
+    _PMigrationLive,
     ("cleanup", False, _TBool),
     ]
 
@@ -5499,7 +5503,7 @@ class LUMigrateInstance(LogicalUnit):
     self.recalculate_locks[locking.LEVEL_NODE] = constants.LOCKS_REPLACE
 
     self._migrater = TLMigrateInstance(self, self.op.instance_name,
-                                       self.op.live, self.op.cleanup)
+                                       self.op.cleanup)
     self.tasklets = [self._migrater]
 
   def DeclareLocks(self, level):
@@ -5717,7 +5721,7 @@ class LUMigrateNode(LogicalUnit):
   HTYPE = constants.HTYPE_NODE
   _OP_PARAMS = [
     _PNodeName,
-    ("live", False, _TBool),
+    _PMigrationLive,
     ]
   REQ_BGL = False
 
@@ -5738,7 +5742,7 @@ class LUMigrateNode(LogicalUnit):
       logging.debug("Migrating instance %s", inst.name)
       names.append(inst.name)
 
-      tasklets.append(TLMigrateInstance(self, inst.name, self.op.live, False))
+      tasklets.append(TLMigrateInstance(self, inst.name, False))
 
     self.tasklets = tasklets
 
@@ -5765,7 +5769,7 @@ class LUMigrateNode(LogicalUnit):
 
 
 class TLMigrateInstance(Tasklet):
-  def __init__(self, lu, instance_name, live, cleanup):
+  def __init__(self, lu, instance_name, cleanup):
     """Initializes this class.
 
     """
@@ -5773,8 +5777,8 @@ class TLMigrateInstance(Tasklet):
 
     # Parameters
     self.instance_name = instance_name
-    self.live = live
     self.cleanup = cleanup
+    self.live = False # will be overridden later
 
   def CheckPrereq(self):
     """Check prerequisites.
@@ -5815,6 +5819,13 @@ class TLMigrateInstance(Tasklet):
 
     self.instance = instance
 
+    if self.lu.op.live is None:
+      # read the default value from the hypervisor
+      i_hv = self.cfg.GetClusterInfo().FillHV(instance, skip_globals=False)
+      self.lu.op.live = i_hv[constants.HV_MIGRATION_TYPE]
+
+    self.live = self.lu.op.live == constants.HT_MIGRATION_LIVE
+
   def _WaitUntilSync(self):
     """Poll with custom rpc for disk sync.
 
diff --git a/lib/constants.py b/lib/constants.py
index 0a4687b8c..4b6d41883 100644
--- a/lib/constants.py
+++ b/lib/constants.py
@@ -537,6 +537,7 @@ HV_INIT_SCRIPT = "init_script"
 HV_MIGRATION_PORT = "migration_port"
 HV_MIGRATION_BANDWIDTH = "migration_bandwidth"
 HV_MIGRATION_DOWNTIME = "migration_downtime"
+HV_MIGRATION_TYPE = "migration_type"
 HV_USE_LOCALTIME = "use_localtime"
 HV_DISK_CACHE = "disk_cache"
 HV_SECURITY_MODEL = "security_model"
@@ -571,6 +572,7 @@ HVS_PARAMETER_TYPES = {
   HV_MIGRATION_PORT: VTYPE_INT,
   HV_MIGRATION_BANDWIDTH: VTYPE_INT,
   HV_MIGRATION_DOWNTIME: VTYPE_INT,
+  HV_MIGRATION_TYPE: VTYPE_STRING,
   HV_USE_LOCALTIME: VTYPE_BOOL,
   HV_DISK_CACHE: VTYPE_STRING,
   HV_SECURITY_MODEL: VTYPE_STRING,
@@ -717,6 +719,11 @@ HT_KVM_DISABLED = "disabled"
 
 HT_KVM_FLAG_VALUES = frozenset([HT_KVM_ENABLED, HT_KVM_DISABLED])
 
+# Migration type
+HT_MIGRATION_LIVE = "live"
+HT_MIGRATION_NONLIVE = "non-live"
+HT_MIGRATION_TYPES = frozenset([HT_MIGRATION_LIVE, HT_MIGRATION_NONLIVE])
+
 # Cluster Verify steps
 VERIFY_NPLUSONE_MEM = 'nplusone_mem'
 VERIFY_OPTIONAL_CHECKS = frozenset([VERIFY_NPLUSONE_MEM])
@@ -859,6 +866,7 @@ HVC_DEFAULTS = {
     HV_ROOT_PATH: '/dev/sda1',
     HV_KERNEL_ARGS: 'ro',
     HV_MIGRATION_PORT: 8002,
+    HV_MIGRATION_TYPE: HT_MIGRATION_LIVE,
     },
   HT_XEN_HVM: {
     HV_BOOT_ORDER: "cd",
@@ -872,6 +880,7 @@ HVC_DEFAULTS = {
     HV_KERNEL_PATH: "/usr/lib/xen/boot/hvmloader",
     HV_DEVICE_MODEL: "/usr/lib/xen/bin/qemu-dm",
     HV_MIGRATION_PORT: 8002,
+    HV_MIGRATION_TYPE: HT_MIGRATION_NONLIVE,
     HV_USE_LOCALTIME: False,
     },
   HT_KVM: {
@@ -894,6 +903,7 @@ HVC_DEFAULTS = {
     HV_MIGRATION_PORT: 8102,
     HV_MIGRATION_BANDWIDTH: 32, # MiB/s
     HV_MIGRATION_DOWNTIME: 30,  # ms
+    HV_MIGRATION_TYPE: HT_MIGRATION_LIVE,
     HV_USE_LOCALTIME: False,
     HV_DISK_CACHE: HT_CACHE_DEFAULT,
     HV_SECURITY_MODEL: HT_SM_NONE,
@@ -914,6 +924,7 @@ HVC_DEFAULTS = {
 HVC_GLOBALS = frozenset([
   HV_MIGRATION_PORT,
   HV_MIGRATION_BANDWIDTH,
+  HV_MIGRATION_TYPE,
   ])
 
 BEC_DEFAULTS = {
diff --git a/lib/hypervisor/hv_base.py b/lib/hypervisor/hv_base.py
index cd9632484..6bcf6d7bf 100644
--- a/lib/hypervisor/hv_base.py
+++ b/lib/hypervisor/hv_base.py
@@ -44,6 +44,7 @@ import logging
 
 from ganeti import errors
 from ganeti import utils
+from ganeti import constants
 
 
 # Read the BaseHypervisor.PARAMETERS docstring for the syntax of the
@@ -71,6 +72,10 @@ NO_CHECK = (False, None, None, None, None)
 # required, but no other checks
 REQUIRED_CHECK = (True, None, None, None, None)
 
+# migration type
+MIGRATION_TYPE_CHECK = (True, lambda x: x in constants.HT_MIGRATION_TYPES,
+                        "invalid migration type", None, None)
+
 
 def ParamInSet(required, my_set):
   """Builds parameter checker for set membership.
diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py
index 35082a7cd..7ddd46b3a 100644
--- a/lib/hypervisor/hv_kvm.py
+++ b/lib/hypervisor/hv_kvm.py
@@ -194,6 +194,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
     constants.HV_MIGRATION_PORT: hv_base.NET_PORT_CHECK,
     constants.HV_MIGRATION_BANDWIDTH: hv_base.NO_CHECK,
     constants.HV_MIGRATION_DOWNTIME: hv_base.NO_CHECK,
+    constants.HV_MIGRATION_TYPE: hv_base.MIGRATION_TYPE_CHECK,
     constants.HV_USE_LOCALTIME: hv_base.NO_CHECK,
     constants.HV_DISK_CACHE:
       hv_base.ParamInSet(True, constants.HT_VALID_CACHE_TYPES),
diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py
index 4f53d7f1b..a02035867 100644
--- a/lib/hypervisor/hv_xen.py
+++ b/lib/hypervisor/hv_xen.py
@@ -458,6 +458,7 @@ class XenPvmHypervisor(XenHypervisor):
     constants.HV_ROOT_PATH: hv_base.REQUIRED_CHECK,
     constants.HV_KERNEL_ARGS: hv_base.NO_CHECK,
     constants.HV_MIGRATION_PORT: hv_base.NET_PORT_CHECK,
+    constants.HV_MIGRATION_TYPE: hv_base.MIGRATION_TYPE_CHECK,
     }
 
   @classmethod
@@ -556,6 +557,7 @@ class XenHvmHypervisor(XenHypervisor):
     constants.HV_DEVICE_MODEL: hv_base.REQ_FILE_CHECK,
     constants.HV_VNC_PASSWORD_FILE: hv_base.REQ_FILE_CHECK,
     constants.HV_MIGRATION_PORT: hv_base.NET_PORT_CHECK,
+    constants.HV_MIGRATION_TYPE: hv_base.MIGRATION_TYPE_CHECK,
     constants.HV_USE_LOCALTIME: hv_base.NO_CHECK,
     }
 
diff --git a/man/gnt-instance.sgml b/man/gnt-instance.sgml
index dd68fd15c..b12337506 100644
--- a/man/gnt-instance.sgml
+++ b/man/gnt-instance.sgml
@@ -2389,6 +2389,7 @@ node1.example.com:disk/1:/dev/drbd1
           <command>migrate</command>
           <arg>-f</arg>
           <arg>--non-live</arg>
+          <arg>--migration-type=live|non-live</arg>
           <arg choice="req"><replaceable>instance</replaceable></arg>
         </cmdsynopsis>
 
@@ -2405,15 +2406,21 @@ node1.example.com:disk/1:/dev/drbd1
         </para>
 
         <para>
-          The <option>--non-live</option> option will switch (for the
-          hypervisors that support it) between a "fully live"
-          (i.e. the interruption is as minimal as possible) migration
-          and one in which the instance is frozen, its state saved and
-          transported to the remote node, and then resumed there. This
-          all depends on the hypervisor support for two different
-          methods. In any case, it is not an error to pass this
-          parameter (it will just be ignored if the hypervisor doesn't
-          support it).
+          The <option>--non-live</option>
+          and <option>--migration-type=non-live</option> options will
+          switch (for the hypervisors that support it) between a
+          "fully live" (i.e. the interruption is as minimal as
+          possible) migration and one in which the instance is frozen,
+          its state saved and transported to the remote node, and then
+          resumed there. This all depends on the hypervisor support
+          for two different methods. In any case, it is not an error
+          to pass this parameter (it will just be ignored if the
+          hypervisor doesn't support it). The
+          option <option>--migration-type=live</option> option will
+          request a fully-live migration. The default, when neither
+          option is passed, depends on the hypervisor parameters (and
+          can be viewed with the <command>gnt-cluster info</command>
+          command).
         </para>
 
         <para>
diff --git a/man/gnt-node.sgml b/man/gnt-node.sgml
index c8509d672..ea20e32da 100644
--- a/man/gnt-node.sgml
+++ b/man/gnt-node.sgml
@@ -571,6 +571,7 @@
         <command>migrate</command>
         <arg>-f</arg>
         <arg>--non-live</arg>
+        <arg>--migration-type=live|non-live</arg>
         <arg choice="req"><replaceable>node</replaceable></arg>
       </cmdsynopsis>
 
@@ -582,8 +583,9 @@
 
       <para>
         As for the <command>gnt-instance migrate</command> command,
-        the <option>--no-live</option> option can be given to do a
-        non-live migration.
+        the options <option>--no-live</option>
+        and <option>--migration-type</option> can be given to
+        influence the migration type.
       </para>
 
       <para>
diff --git a/scripts/gnt-instance b/scripts/gnt-instance
index 1bddbc9ca..0502cdea3 100755
--- a/scripts/gnt-instance
+++ b/scripts/gnt-instance
@@ -897,7 +897,17 @@ def MigrateInstance(opts, args):
     if not AskUser(usertext):
       return 1
 
-  op = opcodes.OpMigrateInstance(instance_name=instance_name, live=opts.live,
+  # this should be removed once --non-live is deprecated
+  if not opts.live and opts.migration_type is not None:
+    raise errors.OpPrereqError("Only one of the --non-live and "
+                               "--migration-type options can be passed",
+                               errors.ECODE_INVAL)
+  if not opts.live: # --non-live passed
+    live = constants.HT_MIGRATION_NONLIVE
+  else:
+    live = opts.migration_type
+
+  op = opcodes.OpMigrateInstance(instance_name=instance_name, live=live,
                                  cleanup=opts.cleanup)
   SubmitOpCode(op, cl=cl, opts=opts)
   return 0
@@ -1409,7 +1419,7 @@ commands = {
     " using the remote mirror (only for instances of type drbd)"),
   'migrate': (
     MigrateInstance, ARGS_ONE_INSTANCE,
-    [FORCE_OPT, NONLIVE_OPT, CLEANUP_OPT],
+    [FORCE_OPT, NONLIVE_OPT, MIGRATION_TYPE_OPT, CLEANUP_OPT],
     "[-f] <instance>", "Migrate instance to its secondary node"
     " (only for instances of type drbd)"),
   'move': (
diff --git a/scripts/gnt-node b/scripts/gnt-node
index 75161017b..363b676dd 100755
--- a/scripts/gnt-node
+++ b/scripts/gnt-node
@@ -360,7 +360,16 @@ def MigrateNode(opts, args):
                                (",".join("'%s'" % name for name in pinst))):
     return 2
 
-  op = opcodes.OpMigrateNode(node_name=args[0], live=opts.live)
+  # this should be removed once --non-live is deprecated
+  if not opts.live and opts.migration_type is not None:
+    raise errors.OpPrereqError("Only one of the --non-live and "
+                               "--migration-type options can be passed",
+                               errors.ECODE_INVAL)
+  if not opts.live: # --non-live passed
+    live = constants.HT_MIGRATION_NONLIVE
+  else:
+    live = opts.migration_type
+  op = opcodes.OpMigrateNode(node_name=args[0], live=live)
   SubmitOpCode(op, cl=cl, opts=opts)
 
 
@@ -652,7 +661,7 @@ commands = {
     "Stops the primary instances on a node and start them on their"
     " secondary node (only for instances with drbd disk template)"),
   'migrate': (
-    MigrateNode, ARGS_ONE_NODE, [FORCE_OPT, NONLIVE_OPT],
+    MigrateNode, ARGS_ONE_NODE, [FORCE_OPT, NONLIVE_OPT, MIGRATION_TYPE_OPT],
     "[-f] <node>",
     "Migrate all the primary instance on a node away from it"
     " (only for instances of type drbd)"),
-- 
GitLab