commit e15a00dca6a0ff4ea6ff60a6f4dec46fe6f25a02
Author: Dimitris Aragiorgis <dimara@grnet.gr>
Date:   Sat Oct 12 21:25:37 2013 +0300

    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>

diff --git a/NEWS b/NEWS
index 3d6589a..42a295e 100644
--- a/NEWS
+++ b/NEWS
@@ -36,6 +36,14 @@ New features
 - Hail and hbal now have the (optional) capability of accessing average CPU
   load information through the monitoring deamon, and to use it to dynamically
   adapt the allocation of instances.
+- Hotplug support. Introduce new option '--hotplug' to ``gnt-instance modify``
+  so that disk and NIC modifications take effect without the need of actual
+  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)
 
 Misc changes
 ~~~~~~~~~~~~
@@ -102,10 +110,6 @@ Incompatible/important changes
 New features
 ~~~~~~~~~~~~
 
-- 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.
 - DRBD 8.4 support. Depending on the installed DRBD version, Ganeti now uses
   the correct command syntax. It is possible to use different DRBD versions
   on different nodes as long as they are compatible to each other. This
diff --git a/lib/cli.py b/lib/cli.py
index 3026ef1..2b35665 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -1644,7 +1644,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 031096a..82647c8 100644
--- 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 07714ee..8f2adf7 100644
--- a/lib/cmdlib/instance.py
+++ b/lib/cmdlib/instance.py
@@ -2228,11 +2228,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:
@@ -3236,6 +3238,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),
@@ -3243,8 +3246,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.
@@ -3271,22 +3277,26 @@ 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:
       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 _PostAddDisk(self, _, disk):
     if not WaitForSync(self, self.instance, disks=[disk],
@@ -3320,10 +3330,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)
@@ -3340,6 +3351,8 @@ class LUInstanceSetParams(LogicalUnit):
     if root.dev_type in constants.DTS_DRBD:
       self.cfg.AddTcpUdpPort(root.logical_id[2])
 
+    return hotmsg
+
   def _CreateNewNic(self, idx, params, private):
     """Creates data structure for a new network interface.
 
@@ -3355,19 +3368,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.
@@ -3393,17 +3407,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 1ad8385..7e29277 100644
--- a/man/gnt-instance.rst
+++ b/man/gnt-instance.rst
@@ -1206,10 +1206,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 ab4f865..0e66be1 100644
--- a/qa/qa_instance.py
+++ b/qa/qa_instance.py
@@ -538,7 +538,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"],
