(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 93888ea..4787b00 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -79,10 +79,14 @@ Incompatible/important changes
 New features
 ~~~~~~~~~~~~
 
 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
   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``.
 - 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 723b221..3006a71 100644 (file)
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -1641,7 +1641,7 @@ INCLUDEDEFAULTS_OPT = cli_option("--include-defaults", dest="include_defaults",
 
 HOTPLUG_OPT = cli_option("--hotplug", dest="hotplug",
                          action="store_true", default=False,
 
 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]
 
 #: 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 6aae105..5159fd9 100644 (file)
--- a/lib/client/gnt_instance.py
+++ b/lib/client/gnt_instance.py
@@ -1314,7 +1314,7 @@ def SetInstanceParams(opts, args):
 
   nics = _ConvertNicDiskModifications(opts.nics)
   for action, _, __ in nics:
 
   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?")
       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?")

@@ -1363,11 +1363,10 @@ def SetInstanceParams(opts, args):
     ToStdout("Modified instance %s", args[0])
     for param, data in result:
       ToStdout(" - %-5s -> %s", param, data)
     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
 
 
   return 0
 
 

diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
index 500ee5e..316fadb 100644 (file)
--- a/lib/cmdlib/instance.py
+++ b/lib/cmdlib/instance.py
@@ -2190,11 +2190,13 @@ def _ApplyContainerMods(kind, container, chgdesc, mods,
       if op == constants.DDM_REMOVE:
         assert not params
 
       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")]
 
         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:
         assert container[absidx] == item
         del container[absidx]
       elif op == constants.DDM_MODIFY:

@@ -3155,6 +3157,7 @@ class LUInstanceSetParams(LogicalUnit):
 
   def _HotplugDevice(self, action, dev_type, device, extra, seq):
     self.LogInfo("Trying to hotplug device...")
 
   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),
     result = self.rpc.call_hotplug_device(self.instance.primary_node,
                                           self.instance, action, dev_type,
                                           (device, self.instance),

@@ -3162,8 +3165,11 @@ class LUInstanceSetParams(LogicalUnit):
     if result.fail_msg:
       self.LogWarning("Could not hotplug device: %s" % result.fail_msg)
       self.LogInfo("Continuing execution..")
     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.")
     else:
       self.LogInfo("Hotplug done.")

+      msg += "done"
+    return msg

 
   def _CreateNewDisk(self, idx, params, _):
     """Creates a new disk.
 
   def _CreateNewDisk(self, idx, params, _):
     """Creates a new disk.

@@ -3192,23 +3198,27 @@ class LUInstanceSetParams(LogicalUnit):
                          disks=[(idx, disk, 0)],
                          cleanup=new_disks)
 
                          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:
     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.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.
 
   def _ModifyDisk(self, idx, disk, params, _):
     """Modifies a disk.

@@ -3243,10 +3253,11 @@ class LUInstanceSetParams(LogicalUnit):
     """Removes a disk.
 
     """
     """Removes a disk.
 
     """

+    hotmsg = ""

     if self.op.hotplug:
     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)
       ShutdownInstanceDisks(self, self.instance, [root])
 
     (anno_disk,) = AnnotateDiskParams(self.instance, [root], self.cfg)

@@ -3261,6 +3272,8 @@ class LUInstanceSetParams(LogicalUnit):
     if root.dev_type in constants.LDS_DRBD:
       self.cfg.AddTcpUdpPort(root.logical_id[2])
 
     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.
 
   def _CreateNewNic(self, idx, params, private):
     """Creates data structure for a new network interface.
 

@@ -3276,19 +3289,20 @@ class LUInstanceSetParams(LogicalUnit):
                        nicparams=nicparams)
     nobj.uuid = self.cfg.GenerateUniqueID(self.proc.GetECId())
 
                        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)),
       ]
 
       ("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.
 
   def _ApplyNicMods(self, idx, nic, params, private):
     """Modifies a network interface.

@@ -3314,17 +3328,18 @@ class LUInstanceSetParams(LogicalUnit):
         changes.append(("nic.%s/%d" % (key, idx), val))
 
     if self.op.hotplug:
         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:
 
     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.
 
   def Exec(self, feedback_fn):
     """Modifies an instance.

diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst
index 3c670fd..401decc 100644 (file)
--- a/man/gnt-instance.rst
+++ b/man/gnt-instance.rst
@@ -1184,10 +1184,15 @@ immediately.
 If ``--ignore-ipolicy`` is given any instance policy violations occuring
 during this operation are ignored.
 
 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
 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.
 
 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"],
     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"],
       ["--net", "-1:remove", "--hotplug"],
       ["--disk", "-1:add,size=1G", "--hotplug"],
       ["--disk", "-1:remove", "--hotplug"],