(2.10) Minor changes regarding hotplug support

* Mention restrictions in NEWS
  - RBD userspace access mode
  - In case of a downgrade instances should suffer a reboot to be migrateable
* Bypass interactive verification in NIC modifications via --force option
  - Mention it in man page
* Print "modifications take place after restart" message no matter if
  --hotplug option is passed. Change cmdlib to append hotplug status info
  (if any) to the final result printed out to the user
* Change hotplug option description

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Signed-off-by: Thomas Thrainer <thomasth@google.com>
Reviewed-by: Thomas Thrainer <thomasth@google.com>

Conflicts:
	NEWS

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>

diff --git a/NEWS b/NEWS
index d7c7faf..a0e6b70 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -95,10 +95,14 @@ Incompatible/important changes
 New features
 ~~~~~~~~~~~~
 
-- Hotplug support. Introduce new option --hotplug to gnt-instance modify
+- Hotplug support. Introduce new option '--hotplug' to ``gnt-instance modify``
   so that disk and NIC modifications take effect without the need of actual
-  reboot. This feature is currently supported only for KVM hypervisor with
-  version greater than 1.0.
+  reboot. There are a couple of constrains currently for this feature:
+
+   - only KVM hypervisor (versions >= 1.0) supports it,
+   - one can not (yet) hotplug a disk using userspace access mode for RBD
+   - in case of a downgrade instances should suffer a reboot in order to
+     be migratable (due to core change of runtime files)
 - The :doc:`Remote API <rapi>` daemon now supports a command line flag
   to always require authentication, ``--require-authentication``. It can
   be specified in ``$sysconfdir/default/ganeti``.

diff --git a/lib/cli.py b/lib/cli.py
index 74d706a..8e8402c 100644 (file)
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -1642,7 +1642,7 @@ INCLUDEDEFAULTS_OPT = cli_option("--include-defaults", dest="include_defaults",
 
 HOTPLUG_OPT = cli_option("--hotplug", dest="hotplug",
                          action="store_true", default=False,
-                         help="Try to hotplug device")
+                         help="Hotplug supported devices (NICs and Disks)")
 
 #: Options provided by all commands
 COMMON_OPTS = [DEBUG_OPT, REASON_OPT]

diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py
index 7659ff7..6a408e4 100644 (file)
--- a/lib/client/gnt_instance.py
+++ b/lib/client/gnt_instance.py
@@ -1315,7 +1315,7 @@ def SetInstanceParams(opts, args):
 
   nics = _ConvertNicDiskModifications(opts.nics)
   for action, _, __ in nics:
-    if action == constants.DDM_MODIFY and opts.hotplug:
+    if action == constants.DDM_MODIFY and opts.hotplug and not opts.force:
       usertext = ("You are about to hot-modify a NIC. This will be done"
                   " by removing the exisiting and then adding a new one."
                   " Network connection might be lost. Continue?")
@@ -1364,11 +1364,10 @@ def SetInstanceParams(opts, args):
     ToStdout("Modified instance %s", args[0])
     for param, data in result:
       ToStdout(" - %-5s -> %s", param, data)
-    if not opts.hotplug:
-      ToStdout("Please don't forget that most parameters take effect"
-               " only at the next (re)start of the instance initiated by"
-               " ganeti; restarting from within the instance will"
-               " not be enough.")
+    ToStdout("Please don't forget that most parameters take effect"
+             " only at the next (re)start of the instance initiated by"
+             " ganeti; restarting from within the instance will"
+             " not be enough.")
   return 0
 
 

diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
index f22e134..72558e7 100644 (file)
--- a/lib/cmdlib/instance.py
+++ b/lib/cmdlib/instance.py
@@ -2206,11 +2206,13 @@ def _ApplyContainerMods(kind, container, chgdesc, mods,
       if op == constants.DDM_REMOVE:
         assert not params
 
-        if remove_fn is not None:
-          remove_fn(absidx, item, private)
-
         changes = [("%s/%s" % (kind, absidx), "remove")]
 
+        if remove_fn is not None:
+          msg = remove_fn(absidx, item, private)
+          if msg:
+            changes.append(("%s/%s" % (kind, absidx), msg))
+
         assert container[absidx] == item
         del container[absidx]
       elif op == constants.DDM_MODIFY:
@@ -3171,6 +3173,7 @@ class LUInstanceSetParams(LogicalUnit):
 
   def _HotplugDevice(self, action, dev_type, device, extra, seq):
     self.LogInfo("Trying to hotplug device...")
+    msg = "hotplug:"
     result = self.rpc.call_hotplug_device(self.instance.primary_node,
                                           self.instance, action, dev_type,
                                           (device, self.instance),
@@ -3178,8 +3181,11 @@ class LUInstanceSetParams(LogicalUnit):
     if result.fail_msg:
       self.LogWarning("Could not hotplug device: %s" % result.fail_msg)
       self.LogInfo("Continuing execution..")
+      msg += "failed"
     else:
       self.LogInfo("Hotplug done.")
+      msg += "done"
+    return msg
 
   def _CreateNewDisk(self, idx, params, _):
     """Creates a new disk.
@@ -3208,23 +3214,27 @@ class LUInstanceSetParams(LogicalUnit):
                          disks=[(idx, disk, 0)],
                          cleanup=new_disks)
 
+    changes = [
+      ("disk/%d" % idx,
+      "add:size=%s,mode=%s" % (disk.size, disk.mode)),
+      ]
     if self.op.hotplug:
       self.cfg.SetDiskID(disk, self.instance.primary_node)
       result = self.rpc.call_blockdev_assemble(self.instance.primary_node,
                                                (disk, self.instance),
                                                self.instance.name, True, idx)
       if result.fail_msg:
+        changes.append(("disk/%d" % idx, "assemble:failed"))
         self.LogWarning("Can't assemble newly created disk %d: %s",
                         idx, result.fail_msg)
       else:
         _, link_name = result.payload
-        self._HotplugDevice(constants.HOTPLUG_ACTION_ADD,
-                            constants.HOTPLUG_TARGET_DISK,
-                            disk, link_name, idx)
+        msg = self._HotplugDevice(constants.HOTPLUG_ACTION_ADD,
+                                  constants.HOTPLUG_TARGET_DISK,
+                                  disk, link_name, idx)
+        changes.append(("disk/%d" % idx, msg))
 
-    return (disk, [
-      ("disk/%d" % idx, "add:size=%s,mode=%s" % (disk.size, disk.mode)),
-      ])
+    return (disk, changes)
 
   def _ModifyDisk(self, idx, disk, params, _):
     """Modifies a disk.
@@ -3259,10 +3269,11 @@ class LUInstanceSetParams(LogicalUnit):
     """Removes a disk.
 
     """
+    hotmsg = ""
     if self.op.hotplug:
-      self._HotplugDevice(constants.HOTPLUG_ACTION_REMOVE,
-                          constants.HOTPLUG_TARGET_DISK,
-                          root, None, idx)
+      hotmsg = self._HotplugDevice(constants.HOTPLUG_ACTION_REMOVE,
+                                   constants.HOTPLUG_TARGET_DISK,
+                                   root, None, idx)
       ShutdownInstanceDisks(self, self.instance, [root])
 
     (anno_disk,) = AnnotateDiskParams(self.instance, [root], self.cfg)
@@ -3277,6 +3288,8 @@ class LUInstanceSetParams(LogicalUnit):
     if root.dev_type in constants.LDS_DRBD:
       self.cfg.AddTcpUdpPort(root.logical_id[2])
 
+    return hotmsg
+
   def _CreateNewNic(self, idx, params, private):
     """Creates data structure for a new network interface.
 
@@ -3292,19 +3305,20 @@ class LUInstanceSetParams(LogicalUnit):
                        nicparams=nicparams)
     nobj.uuid = self.cfg.GenerateUniqueID(self.proc.GetECId())
 
-    if self.op.hotplug:
-      self._HotplugDevice(constants.HOTPLUG_ACTION_ADD,
-                          constants.HOTPLUG_TARGET_NIC,
-                          nobj, None, idx)
-
-    desc = [
+    changes = [
       ("nic.%d" % idx,
        "add:mac=%s,ip=%s,mode=%s,link=%s,network=%s" %
        (mac, ip, private.filled[constants.NIC_MODE],
        private.filled[constants.NIC_LINK], net)),
       ]
 
-    return (nobj, desc)
+    if self.op.hotplug:
+      msg = self._HotplugDevice(constants.HOTPLUG_ACTION_ADD,
+                                constants.HOTPLUG_TARGET_NIC,
+                                nobj, None, idx)
+      changes.append(("nic.%d" % idx, msg))
+
+    return (nobj, changes)
 
   def _ApplyNicMods(self, idx, nic, params, private):
     """Modifies a network interface.
@@ -3330,17 +3344,18 @@ class LUInstanceSetParams(LogicalUnit):
         changes.append(("nic.%s/%d" % (key, idx), val))
 
     if self.op.hotplug:
-      self._HotplugDevice(constants.HOTPLUG_ACTION_MODIFY,
-                          constants.HOTPLUG_TARGET_NIC,
-                          nic, None, idx)
+      msg = self._HotplugDevice(constants.HOTPLUG_ACTION_MODIFY,
+                                constants.HOTPLUG_TARGET_NIC,
+                                nic, None, idx)
+      changes.append(("nic/%d" % idx, msg))
 
     return changes
 
   def _RemoveNic(self, idx, nic, _):
     if self.op.hotplug:
-      self._HotplugDevice(constants.HOTPLUG_ACTION_REMOVE,
-                          constants.HOTPLUG_TARGET_NIC,
-                          nic, None, idx)
+      return self._HotplugDevice(constants.HOTPLUG_ACTION_REMOVE,
+                                 constants.HOTPLUG_TARGET_NIC,
+                                 nic, None, idx)
 
   def Exec(self, feedback_fn):
     """Modifies an instance.

diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst
index 217a751..7ea2fc8 100644 (file)
--- a/man/gnt-instance.rst
+++ b/man/gnt-instance.rst
@@ -1195,10 +1195,15 @@ immediately.
 If ``--ignore-ipolicy`` is given any instance policy violations occuring
 during this operation are ignored.
 
-If ``--hotplug`` is given any disk and nic modifications will take
+If ``--hotplug`` is given any disk and NIC modifications will take
 effect without the need of actual reboot. Please note that this feature
-is currently supported only for KVM hypervisor and for versions greater
-than 1.0.
+is currently supported only for KVM hypervisor and there are some
+restrictions: a) KVM versions >= 1.0 support it b) instances with chroot
+or uid pool security model do not support disk hotplug c) RBD disks with
+userspace access mode can not be hotplugged (yet) d) if hotplug fails
+(for any reason) a warning is printed but execution is continued e)
+for existing NIC modification interactive verification is needed unless
+``--force`` option is passed.
 
 See **ganeti**\(7) for a description of ``--submit`` and other common
 options.

diff --git a/qa/qa_instance.py b/qa/qa_instance.py
index 0f09c82..1a65251 100644 (file)
--- a/qa/qa_instance.py
+++ b/qa/qa_instance.py
@@ -684,7 +684,7 @@ def TestInstanceModify(instance):
     qa_config.TestEnabled("instance-device-hotplug"):
     args.extend([
       ["--net", "-1:add", "--hotplug"],
-      ["--net", "-1:modify,mac=aa:bb:cc:dd:ee:ff", "--hotplug"],
+      ["--net", "-1:modify,mac=aa:bb:cc:dd:ee:ff", "--hotplug", "--force"],
       ["--net", "-1:remove", "--hotplug"],
       ["--disk", "-1:add,size=1G", "--hotplug"],
       ["--disk", "-1:remove", "--hotplug"],