Currently, hot-modifying a device requires to firstly hot-delete the device and then re-add it using the updated NIC parameters. However, the device object passed to the HotDel method is the one with the updated NIC params instead of the previous params, resulting in passing wrong environment values to the configuration NIC scripts.
This patch fixes that behavior by adding an extra argument to the HotModDevice function, specifying the old device object. Signed-off-by: Dimitris Bliablias <[email protected]> --- lib/backend.py | 17 +++++++++-------- lib/cmdlib/instance_set_params.py | 20 +++++++++++--------- lib/hypervisor/hv_base.py | 2 +- lib/hypervisor/hv_kvm/__init__.py | 6 +++--- lib/rpc_defs.py | 3 ++- lib/server/noded.py | 12 ++++++++---- 6 files changed, 34 insertions(+), 26 deletions(-) diff --git a/lib/backend.py b/lib/backend.py index 0a781df..6dc2ce2 100644 --- a/lib/backend.py +++ b/lib/backend.py @@ -3170,18 +3170,21 @@ def GetMigrationStatus(instance): _Fail("Failed to get migration status: %s", err, exc=True) -def HotplugDevice(instance, action, dev_type, device, extra, seq): +def HotplugDevice(instance, action, dev_type, old_dev, new_dev, extra, seq): """Hotplug a device Hotplug is currently supported only for KVM Hypervisor. + @type instance: L{objects.Instance} @param instance: the instance to which we hotplug a device @type action: string @param action: the hotplug action to perform @type dev_type: string @param dev_type: the device type to hotplug - @type device: either L{objects.NIC} or L{objects.Disk} - @param device: the device object to hotplug + @type old_dev: either L{objects.NIC} or L{objects.Disk} + @param old_dev: the device object to hotplug + @type new_dev: either L{objects.NIC} or L{objects.Disk} + @param new_dev: the device object to hotplug @type extra: tuple @param extra: extra info used for disk hotplug (disk link, drive uri) @type seq: int @@ -3196,16 +3199,14 @@ def HotplugDevice(instance, action, dev_type, device, extra, seq): _Fail("Hotplug is not supported: %s", err) if action == constants.HOTPLUG_ACTION_ADD: - fn = hyper.HotAddDevice + return hyper.HotAddDevice(instance, dev_type, new_dev, extra, seq) elif action == constants.HOTPLUG_ACTION_REMOVE: - fn = hyper.HotDelDevice + return hyper.HotDelDevice(instance, dev_type, new_dev, extra, seq) elif action == constants.HOTPLUG_ACTION_MODIFY: - fn = hyper.HotModDevice + return hyper.HotModDevice(instance, dev_type, old_dev, new_dev, extra, seq) else: assert action in constants.HOTPLUG_ALL_ACTIONS - return fn(instance, dev_type, device, extra, seq) - def HotplugSupported(instance): """Checks if hotplug is generally supported. diff --git a/lib/cmdlib/instance_set_params.py b/lib/cmdlib/instance_set_params.py index 6a68e5a..655c7a1 100644 --- a/lib/cmdlib/instance_set_params.py +++ b/lib/cmdlib/instance_set_params.py @@ -1563,12 +1563,13 @@ class LUInstanceSetParams(LogicalUnit): meta_disks.append(disk.children[1]) RemoveDisks(self, self.instance, disks=meta_disks) - def _HotplugDevice(self, action, dev_type, device, extra, seq): + def _HotplugDevice(self, action, dev_type, old_dev, new_dev, 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), + (old_dev, self.instance), + (new_dev, self.instance), extra, seq) if result.fail_msg: self.LogWarning("Could not hotplug device: %s" % result.fail_msg) @@ -1636,7 +1637,7 @@ class LUInstanceSetParams(LogicalUnit): _, link_name, uri = result.payload msg = self._HotplugDevice(constants.HOTPLUG_ACTION_ADD, constants.HOTPLUG_TARGET_DISK, - disk, (link_name, uri), idx) + None, disk, (link_name, uri), idx) changes.append(("disk/%d" % idx, msg)) return (disk, changes) @@ -1695,7 +1696,7 @@ class LUInstanceSetParams(LogicalUnit): _, link_name, uri = payloads[0] msg = self._HotplugDevice(constants.HOTPLUG_ACTION_ADD, constants.HOTPLUG_TARGET_DISK, - disk, (link_name, uri), idx) + None, disk, (link_name, uri), idx) changes.append(("disk/%d" % idx, msg)) return (disk, changes) @@ -1741,7 +1742,7 @@ class LUInstanceSetParams(LogicalUnit): if self.op.hotplug: hotmsg = self._HotplugDevice(constants.HOTPLUG_ACTION_REMOVE, constants.HOTPLUG_TARGET_DISK, - root, None, idx) + None, root, None, idx) ShutdownInstanceDisks(self, self.instance, [root]) RemoveDisks(self, self.instance, disks=[root]) @@ -1766,7 +1767,7 @@ class LUInstanceSetParams(LogicalUnit): if self.op.hotplug: hotmsg = self._HotplugDevice(constants.HOTPLUG_ACTION_REMOVE, constants.HOTPLUG_TARGET_DISK, - root, None, idx) + None, root, None, idx) # Always shutdown the disk before detaching. ShutdownInstanceDisks(self, self.instance, [root]) @@ -1821,7 +1822,7 @@ class LUInstanceSetParams(LogicalUnit): if self.op.hotplug: msg = self._HotplugDevice(constants.HOTPLUG_ACTION_ADD, constants.HOTPLUG_TARGET_NIC, - nobj, None, idx) + None, nobj, None, idx) changes.append(("nic.%d" % idx, msg)) return (nobj, changes) @@ -1831,6 +1832,7 @@ class LUInstanceSetParams(LogicalUnit): """ changes = [] + old_nic = copy.deepcopy(nic) for key in [constants.INIC_MAC, constants.INIC_IP, constants.INIC_NAME]: if key in params: @@ -1852,7 +1854,7 @@ class LUInstanceSetParams(LogicalUnit): if self.op.hotplug: msg = self._HotplugDevice(constants.HOTPLUG_ACTION_MODIFY, constants.HOTPLUG_TARGET_NIC, - nic, None, idx) + old_nic, nic, None, idx) changes.append(("nic/%d" % idx, msg)) return changes @@ -1861,7 +1863,7 @@ class LUInstanceSetParams(LogicalUnit): if self.op.hotplug: return self._HotplugDevice(constants.HOTPLUG_ACTION_REMOVE, constants.HOTPLUG_TARGET_NIC, - nic, None, idx) + None, nic, None, idx) def Exec(self, feedback_fn): """Modifies an instance. diff --git a/lib/hypervisor/hv_base.py b/lib/hypervisor/hv_base.py index 4c20c6a..076402b 100644 --- a/lib/hypervisor/hv_base.py +++ b/lib/hypervisor/hv_base.py @@ -773,7 +773,7 @@ class BaseHypervisor(object): raise errors.HotplugError("Hotplug is not supported by this hypervisor") # pylint: disable=R0201,W0613 - def HotModDevice(self, instance, dev_type, device, extra, seq): + def HotModDevice(self, instance, dev_type, old_dev, new_dev, extra, seq): """Hot-mod a device. """ diff --git a/lib/hypervisor/hv_kvm/__init__.py b/lib/hypervisor/hv_kvm/__init__.py index f0f465e..02c42b6 100644 --- a/lib/hypervisor/hv_kvm/__init__.py +++ b/lib/hypervisor/hv_kvm/__init__.py @@ -2326,7 +2326,7 @@ class KVMHypervisor(hv_base.BaseHypervisor): return kvm_device.hvinfo - def HotModDevice(self, instance, dev_type, device, _, seq): + def HotModDevice(self, instance, dev_type, old_dev, new_dev, _, seq): """ Helper method for hot-mod device It gets device info from runtime file, generates the device name and @@ -2335,8 +2335,8 @@ class KVMHypervisor(hv_base.BaseHypervisor): """ if dev_type == constants.HOTPLUG_TARGET_NIC: # putting it back in the same bus and slot - device.hvinfo = self.HotDelDevice(instance, dev_type, device, _, seq) - self.HotAddDevice(instance, dev_type, device, _, seq) + new_dev.hvinfo = self.HotDelDevice(instance, dev_type, old_dev, _, seq) + self.HotAddDevice(instance, dev_type, new_dev, _, seq) @classmethod def _ParseKVMVersion(cls, text): diff --git a/lib/rpc_defs.py b/lib/rpc_defs.py index 1119c7b..ed4aa1b 100644 --- a/lib/rpc_defs.py +++ b/lib/rpc_defs.py @@ -298,7 +298,8 @@ _INSTANCE_CALLS = [ ("instance", ED_INST_DICT, "Instance object"), ("action", None, "Hotplug Action"), ("dev_type", None, "Device type"), - ("device", ED_DEVICE_DICT, "Device dict"), + ("old_dev", ED_DEVICE_DICT, "Old device dict"), + ("new_dev", ED_DEVICE_DICT, "New device dict"), ("extra", None, "Extra info for device (dev_path for disk)"), ("seq", None, "Device seq"), ], None, None, "Hoplug a device to a running instance"), diff --git a/lib/server/noded.py b/lib/server/noded.py index 5e89b7e..5f64450 100644 --- a/lib/server/noded.py +++ b/lib/server/noded.py @@ -651,15 +651,19 @@ class NodeRequestHandler(http.server.HttpServerHandler): """Hotplugs device to a running instance. """ - (idict, action, dev_type, ddict, extra, seq) = params + (idict, action, dev_type, old_ddict, new_ddict, extra, seq) = params + old_dev = None instance = objects.Instance.FromDict(idict) if dev_type == constants.HOTPLUG_TARGET_DISK: - device = objects.Disk.FromDict(ddict) + new_dev = objects.Disk.FromDict(new_ddict) elif dev_type == constants.HOTPLUG_TARGET_NIC: - device = objects.NIC.FromDict(ddict) + if isinstance(old_ddict, dict): + old_dev = objects.NIC.FromDict(old_ddict) + new_dev = objects.NIC.FromDict(new_ddict) else: assert dev_type in constants.HOTPLUG_ALL_TARGETS - return backend.HotplugDevice(instance, action, dev_type, device, extra, seq) + return backend.HotplugDevice(instance, action, dev_type, old_dev, new_dev, + extra, seq) @staticmethod def perspective_hotplug_supported(params): -- 2.1.4
