Here is the second revision of my patch. As you have suggested, I now use existing hotplug methods instead of creating new ones. (except the _VerifyHotResizeCommand special case)
For GetBlockDevices, I choose to keep it because it returns different informations than PCI devices (eg : disk size) so it would be difficult to merge them together. For the integration of "grow-disk" in "modify", I'll probably do that in a separate patch. 2015-06-28 20:59 GMT+02:00 Yoann Laissus <[email protected]>: > This patch adds support for disk hot resizing when using a KVM > hypervisor. It's done by calling the "block_resize" QMP command. > It can be enabled via the new '--hot-resize' option of gnt-instance > grow-disk but it remains disabled by default. > > Signed-off-by: Yoann Laissus <[email protected]> > --- > lib/backend.py | 15 ++++++-- > lib/cli_opts.py | 6 +++ > lib/client/gnt_instance.py | 5 ++- > lib/cmdlib/instance_set_params.py | 43 +++++++++++++++++++-- > lib/cmdlib/instance_storage.py | 49 ++++++++++++++++++++++-- > lib/hypervisor/hv_base.py | 8 ---- > lib/hypervisor/hv_kvm/__init__.py | 79 > ++++++++++++++++++++++++--------------- > lib/hypervisor/hv_kvm/monitor.py | 53 ++++++++++++++++++++++++++ > lib/rapi/client.py | 3 +- > lib/rpc_defs.py | 2 + > lib/server/noded.py | 5 ++- > src/Ganeti/OpCodes.hs | 1 + > src/Ganeti/OpParams.hs | 4 ++ > test/hs/Test/Ganeti/OpCodes.hs | 1 + > 14 files changed, 218 insertions(+), 56 deletions(-) > > diff --git a/lib/backend.py b/lib/backend.py > index f80fc20..8021301 100644 > --- a/lib/backend.py > +++ b/lib/backend.py > @@ -2993,17 +2993,24 @@ def HotplugDevice(instance, action, dev_type, > device, extra, seq): > return fn(instance, dev_type, device, extra, seq) > > > -def HotplugSupported(instance): > - """Checks if hotplug is generally supported. > +def VerifyHotplugSupport(instance, action, dev_type): > + """Checks if hotplug is supported on the specified device type > + > + @type instance: L{objects.Instance} > + @param instance: the instance to which we want to 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 > + @raise RPCFail: in case instance does not have KVM hypervisor > > """ > hyper = hypervisor.GetHypervisor(instance.hypervisor) > try: > - hyper.HotplugSupported(instance) > + hyper.VerifyHotplugSupport(instance, action, dev_type) > except errors.HotplugError, err: > _Fail("Hotplug is not supported: %s", err) > > - > def ModifyInstanceMetadata(metadata): > """Sends instance data to the metadata daemon. > > diff --git a/lib/cli_opts.py b/lib/cli_opts.py > index a6126e9..3f8677f 100644 > --- a/lib/cli_opts.py > +++ b/lib/cli_opts.py > @@ -104,6 +104,7 @@ __all__ = [ > "HELPER_SHUTDOWN_TIMEOUT_OPT", > "HELPER_STARTUP_TIMEOUT_OPT", > "HID_OS_OPT", > + "HOT_RESIZE_OPT", > "HOTPLUG_IF_POSSIBLE_OPT", > "HOTPLUG_OPT", > "HV_STATE_OPT", > @@ -1513,6 +1514,11 @@ INCLUDEDEFAULTS_OPT = > cli_option("--include-defaults", dest="include_defaults", > default=False, action="store_true", > help="Include default values") > > +HOT_RESIZE_OPT = cli_option("--hot-resize", dest="hot_resize", > + action="store_true", default=False, > + help="Make the extra space available for the" > + "running instance without rebooting.") > + > HOTPLUG_OPT = cli_option("--hotplug", dest="hotplug", > action="store_true", default=False, > help="Hotplug supported devices (NICs and > Disks)") > diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py > index 224ee44..c2931a5 100644 > --- a/lib/client/gnt_instance.py > +++ b/lib/client/gnt_instance.py > @@ -609,7 +609,8 @@ def GrowDisk(opts, args): > disk=disk, amount=amount, > wait_for_sync=opts.wait_for_sync, > absolute=opts.absolute, > - ignore_ipolicy=opts.ignore_ipolicy > + ignore_ipolicy=opts.ignore_ipolicy, > + hot_resize=opts.hot_resize > ) > SubmitOrSend(op, opts) > return 0 > @@ -1705,7 +1706,7 @@ commands = { > [ArgInstance(min=1, max=1), ArgUnknown(min=1, max=1), > ArgUnknown(min=1, max=1)], > SUBMIT_OPTS + > - [NWSYNC_OPT, DRY_RUN_OPT, PRIORITY_OPT, ABSOLUTE_OPT, > IGNORE_IPOLICY_OPT], > + [NWSYNC_OPT, DRY_RUN_OPT, PRIORITY_OPT, ABSOLUTE_OPT, > IGNORE_IPOLICY_OPT, HOT_RESIZE_OPT], > "<instance> <disk> <size>", "Grow an instance's disk"), > "change-group": ( > ChangeGroup, ARGS_ONE_INSTANCE, > diff --git a/lib/cmdlib/instance_set_params.py > b/lib/cmdlib/instance_set_params.py > index 12d573a..f200df3 100644 > --- a/lib/cmdlib/instance_set_params.py > +++ b/lib/cmdlib/instance_set_params.py > @@ -883,10 +883,45 @@ class LUInstanceSetParams(LogicalUnit): > cluster_hvparams) > return instance_info > > - def _CheckHotplug(self): > + def _CheckDeviceHotplug(self, device_type, action): > + """_CheckDeviceHotplug checks if an hotplug action is possible on the > + specified device type. > + > + @type device_type: string > + @param device_type: the device type to hotplug > + @type action: string > + @param action: the hotplug action to perform > + > + """ > + if action in (constants.DDM_ADD, constants.DDM_ATTACH): > + hotplug_action = constants.HOTPLUG_ACTION_ADD > + elif action in (constants.DDM_REMOVE, constants.DDM_DETACH): > + hotplug_action = constants.HOTPLUG_ACTION_REMOVE > + else: > + hotplug_action = constants.HOTPLUG_ACTION_MODIFY > + > + return self.rpc.call_hotplug_supported(self.instance.primary_node, > + self.instance, > + hotplug_action, > + device_type) > + > + def _CheckDisksNicsHotplug(self): > + """_CheckDisksNicsHotplug checks if hotplug is available for each > disk and > + NIC of the request. > + If one check fail, hotplug will be globally unavailable. > + > + """ > if self.op.hotplug or self.op.hotplug_if_possible: > - result = self.rpc.call_hotplug_supported(self.instance.primary_node, > - self.instance) > + for op, _, _ in self.op.nics: > + result = self._CheckDeviceHotplug(constants.HOTPLUG_TARGET_NIC, > op) > + if result.fail_msg: > + break > + > + for op, _, _ in self.op.disks: > + result = self._CheckDeviceHotplug(constants.HOTPLUG_TARGET_DISK, > op) > + if result.fail_msg: > + break > + > if result.fail_msg: > if self.op.hotplug: > result.Raise("Hotplug is not possible: %s" % result.fail_msg, > @@ -1148,7 +1183,7 @@ class LUInstanceSetParams(LogicalUnit): > # dictionary with instance information after the modification > ispec = {} > > - self._CheckHotplug() > + self._CheckDisksNicsHotplug() > > self._PrepareNicCommunication() > > diff --git a/lib/cmdlib/instance_storage.py > b/lib/cmdlib/instance_storage.py > index 3f609d6..1a2a2a0 100644 > --- a/lib/cmdlib/instance_storage.py > +++ b/lib/cmdlib/instance_storage.py > @@ -1710,6 +1710,7 @@ class LUInstanceGrowDisk(LogicalUnit): > "DISK": self.op.disk, > "AMOUNT": self.op.amount, > "ABSOLUTE": self.op.absolute, > + "HOT_RESIZE": self.op.hot_resize, > } > env.update(BuildInstanceHookEnvByObject(self, self.instance)) > return env > @@ -1764,6 +1765,8 @@ class LUInstanceGrowDisk(LogicalUnit): > > self._CheckIPolicy(self.target) > > + self._CheckDiskHotResize() > + > def _CheckDiskSpace(self, node_uuids, req_vgspace): > template = self.disk.dev_type > if (template not in constants.DTS_NO_FREE_SPACE_CHECK and > @@ -1798,6 +1801,30 @@ class LUInstanceGrowDisk(LogicalUnit): > else: > raise errors.OpPrereqError(msg, errors.ECODE_INVAL) > > + def _CheckDiskHotResize(self): > + if self.op.hot_resize: > + result = self.rpc.call_hotplug_supported(self.instance.primary_node, > + self.instance, > + > constants.HOTPLUG_ACTION_MODIFY, > + > constants.HOTPLUG_TARGET_DISK) > + if result.fail_msg: > + result.Raise("Hot resizing is not possible: %s" % result.fail_msg, > + prereq=True, ecode=errors.ECODE_STATE) > + else: > + self.LogInfo("Modification will take place without hot resizing.") > + > + def _HotResizeDisk(self, device, size): > + result = self.rpc.call_hotplug_device(self.instance.primary_node, > + self.instance, > constants.HOTPLUG_ACTION_MODIFY, > + constants.HOTPLUG_TARGET_DISK, > + (device, self.instance), > + (size,), device.logical_id) > + if result.fail_msg: > + self.LogWarning("Could not hot resize disk: %s" % result.fail_msg) > + self.LogInfo("Continuing execution..") > + else: > + self.LogInfo("Hot resize successful") > + > def Exec(self, feedback_fn): > """Execute disk grow. > > @@ -1835,14 +1862,14 @@ class LUInstanceGrowDisk(LogicalUnit): > result.Raise("Failed to retrieve disk size from node '%s'" % > self.instance.primary_node) > > - (disk_dimensions, ) = result.payload > + (old_disk_dimensions, ) = result.payload > > - if disk_dimensions is None: > + if old_disk_dimensions is None: > raise errors.OpExecError("Failed to retrieve disk size from > primary" > " node '%s'" % > self.instance.primary_node) > - (disk_size_in_bytes, _) = disk_dimensions > + (old_disk_size_in_bytes, _) = old_disk_dimensions > > - old_disk_size = _DiskSizeInBytesToMebibytes(self, > disk_size_in_bytes) > + old_disk_size = _DiskSizeInBytesToMebibytes(self, > old_disk_size_in_bytes) > > assert old_disk_size >= self.disk.size, \ > ("Retrieved disk size too small (got %s, should be at least %s)" % > @@ -1889,6 +1916,20 @@ class LUInstanceGrowDisk(LogicalUnit): > WipeDisks(self, self.instance, > disks=[(self.op.disk, self.disk, old_disk_size)]) > > + if self.op.hot_resize: > + result = self.rpc.call_blockdev_getdimensions( > + self.instance.primary_node, [([self.disk], > self.instance)]) > + result.Raise("Failed to retrieve disk size from node '%s'" % > + self.instance.primary_node) > + > + (new_disk_dimensions, ) = result.payload > + > + if new_disk_dimensions is None: > + raise errors.OpExecError("Failed to retrieve disk size from > primary" > + " node '%s'" % > self.instance.primary_node) > + (new_disk_size_in_bytes, _) = new_disk_dimensions > + self._HotResizeDisk(self.disk, new_disk_size_in_bytes) > + > if self.op.wait_for_sync: > disk_abort = not WaitForSync(self, self.instance, disks=[self.disk]) > if disk_abort: > diff --git a/lib/hypervisor/hv_base.py b/lib/hypervisor/hv_base.py > index 8aaa062..ba669d0 100644 > --- a/lib/hypervisor/hv_base.py > +++ b/lib/hypervisor/hv_base.py > @@ -773,11 +773,3 @@ class BaseHypervisor(object): > > """ > raise errors.HotplugError("Hotplug is not supported.") > - > - def HotplugSupported(self, instance): > - """Checks if hotplug is supported. > - > - By default is not. Currently only KVM hypervisor supports it. > - > - """ > - raise errors.HotplugError("Hotplug is not supported by this > hypervisor") > diff --git a/lib/hypervisor/hv_kvm/__init__.py > b/lib/hypervisor/hv_kvm/__init__.py > index 340ddb1..b08de59 100644 > --- a/lib/hypervisor/hv_kvm/__init__.py > +++ b/lib/hypervisor/hv_kvm/__init__.py > @@ -1830,36 +1830,18 @@ class KVMHypervisor(hv_base.BaseHypervisor): > @raise errors.HypervisorError: in one of the previous cases > > """ > - if dev_type == constants.HOTPLUG_TARGET_DISK: > - if action == constants.HOTPLUG_ACTION_ADD: > - self.qmp.CheckDiskHotAddSupport() > - if dev_type == constants.HOTPLUG_TARGET_NIC: > - if action == constants.HOTPLUG_ACTION_ADD: > - self.qmp.CheckNicHotAddSupport() > - > - def HotplugSupported(self, instance): > - """Checks if hotplug is generally supported. > - > - Hotplug is *not* supported in case of: > - - qemu versions < 1.7 (where all qmp related commands are supported) > - - for stopped instances > - > - @raise errors.HypervisorError: in one of the previous cases > - > - """ > - try: > - output = self._CallMonitorCommand(instance.name, > self._INFO_VERSION_CMD) > - except errors.HypervisorError: > - raise errors.HotplugError("Instance is probably down") > - > - match = self._INFO_VERSION_RE.search(output.stdout) > - if not match: > - raise errors.HotplugError("Cannot parse qemu version via monitor") > - > - #TODO: delegate more fine-grained checks to VerifyHotplugSupport > - v_major, v_min, _, _ = match.groups() > - if (int(v_major), int(v_min)) < (1, 7): > - raise errors.HotplugError("Hotplug not supported for qemu versions > < 1.7") > + if action == constants.HOTPLUG_ACTION_ADD: > + if dev_type == constants.HOTPLUG_TARGET_DISK: > + self.qmp.CheckDiskHotAddSupport() > + elif dev_type == constants.HOTPLUG_TARGET_NIC: > + self.qmp.CheckNicHotAddSupport() > + elif action == constants.HOTPLUG_ACTION_MODIFY: > + if dev_type == constants.HOTPLUG_TARGET_DISK: > + # Currently, it's the only modification supported on disks > + self.qmp.CheckDiskHotResizeSupport() > + elif dev_type == constants.HOTPLUG_TARGET_NIC: > + # It consists of removing the NIC and adding it again > + self.qmp.CheckNicHotAddSupport() > > @_with_qmp > def _VerifyHotplugCommand(self, _instance, > @@ -1952,17 +1934,52 @@ class KVMHypervisor(hv_base.BaseHypervisor): > > return kvm_device.pci > > - def HotModDevice(self, instance, dev_type, device, _, seq): > + def HotModDevice(self, instance, dev_type, device, extra, seq): > """ Helper method for hot-mod device > > It gets device info from runtime file, generates the device name and > invokes the device specific method. Currently only NICs support > hot-mod > > """ > + runtime = self._LoadKVMRuntime(instance) > + entry = _GetExistingDeviceInfo(dev_type, device, runtime) > + kvm_device = _RUNTIME_DEVICE[dev_type](entry) > + kvm_devid = _GenerateDeviceKVMId(dev_type, kvm_device) > + > if dev_type == constants.HOTPLUG_TARGET_NIC: > # putting it back in the same pci slot > device.pci = self.HotDelDevice(instance, dev_type, device, _, seq) > self.HotAddDevice(instance, dev_type, device, _, seq) > + elif dev_type == constants.HOTPLUG_TARGET_DISK: > + new_size = extra[0] > + self.qmp.HotResizeDisk(kvm_devid, new_size) > + self._VerifyHotResizeCommand(kvm_devid, new_size) > + > + @_with_qmp > + def _VerifyHotResizeCommand(self, kvm_devid, size): > + """Checks if a previous disk hot resize command was successful. > + > + The current block device size is retrieved and compared to the > specified > + size. > + > + @raise errors.HypervisorError: if result is not the expected one > + > + """ > + for i in range(5): > + found_size = self.qmp.GetBlockDeviceSize(kvm_devid) > + logging.info("Verifying hotplug command (retry %s): %s", i, > found_size) > + if found_size == size: > + break > + time.sleep(1) > + > + if found_size != size: > + msg = ("Device %s should have been resized to %s but its size is > still " > + "%s" % (kvm_devid, size, found_size)) > + raise errors.HypervisorError(msg) > + > + logging.info("Device %s has been correctly hot resized to %s", > + kvm_devid, > + size) > > @classmethod > def _ParseKVMVersion(cls, text): > diff --git a/lib/hypervisor/hv_kvm/monitor.py > b/lib/hypervisor/hv_kvm/monitor.py > index f2b7d02..0db8318 100644 > --- a/lib/hypervisor/hv_kvm/monitor.py > +++ b/lib/hypervisor/hv_kvm/monitor.py > @@ -579,6 +579,45 @@ class QmpConnection(MonitorSocket): > #TODO: uncomment when drive_del gets implemented in upstream qemu > # self.Execute("drive_del", {"id": devid}) > > + @_ensure_connection > + def HotResizeDisk(self, devid, size): > + """Hot resize a Disk to the specified size > + > + This command is available since QEMU 0.12. > + > + """ > + arguments = { > + "device": devid, > + "size": size, > + } > + self.Execute("block_resize", arguments) > + > + def _GetBlockDevices(self): > + """Get the block devices of a running instance. > + > + """ > + self._check_connection() > + devices = self.Execute("query-block") > + return devices > + > + def _GetBlockDevice(self, devid): > + """Get the block device JSON object of the specified id > + > + """ > + for d in self._GetBlockDevices(): > + if d["device"] == devid: > + return d > + > + return False > + > + @_ensure_connection > + def GetBlockDeviceSize(self, devid): > + """Get the size of the specified block device > + > + """ > + device = self._GetBlockDevice(devid) > + return device["inserted"]["image"]["virtual-size"] > + > def _GetPCIDevices(self): > """Get the devices of the first PCI bus of a running instance. > > @@ -617,6 +656,20 @@ class QmpConnection(MonitorSocket): > return utils.GetFreeSlot(slots) > > @_ensure_connection > + def CheckDiskHotResizeSupport(self): > + """Check if disk hot resizing is possible > + block_resize and query-block qmp commands need to be available. > + > + """ > + def _raise(reason): > + raise errors.HotplugError("Cannot hot resize disk: %s." % reason) > + > + if "block_resize" not in self.supported_commands: > + _raise("block_resize qmp command is not supported") > + if "query-block" not in self.supported_commands: > + _raise("query-block qmp command is not supported") > + > + @_ensure_connection > def CheckDiskHotAddSupport(self): > """Check if disk hotplug is possible > > diff --git a/lib/rapi/client.py b/lib/rapi/client.py > index 65f82ab..7439ef6 100644 > --- a/lib/rapi/client.py > +++ b/lib/rapi/client.py > @@ -1012,7 +1012,7 @@ class GanetiRapiClient(object): # pylint: > disable=R0904 > (GANETI_RAPI_VERSION, instance)), query, > body) > > def GrowInstanceDisk(self, instance, disk, amount, wait_for_sync=None, > - reason=None): > + reason=None, hot_resize=None): > """Grows a disk of an instance. > > More details for parameters can be found in the RAPI documentation. > @@ -1036,6 +1036,7 @@ class GanetiRapiClient(object): # pylint: > disable=R0904 > } > > _SetItemIf(body, wait_for_sync is not None, "wait_for_sync", > wait_for_sync) > + _SetItemIf(body, hot_resize is not None, "hot_resize", hot_resize) > > query = [] > _AppendReason(query, reason) > diff --git a/lib/rpc_defs.py b/lib/rpc_defs.py > index dfdc5de..3c25a71 100644 > --- a/lib/rpc_defs.py > +++ b/lib/rpc_defs.py > @@ -304,6 +304,8 @@ _INSTANCE_CALLS = [ > ], None, None, "Hoplug a device to a running instance"), > ("hotplug_supported", SINGLE, None, constants.RPC_TMO_NORMAL, [ > ("instance", ED_INST_DICT, "Instance object"), > + ("action", None, "Hotplug Action"), > + ("dev_type", None, "Device type"), > ], None, None, "Check if hotplug is supported"), > ("instance_metadata_modify", SINGLE, None, constants.RPC_TMO_URGENT, [ > ("instance", None, "Instance object"), > diff --git a/lib/server/noded.py b/lib/server/noded.py > index fe1be33..6910abb 100644 > --- a/lib/server/noded.py > +++ b/lib/server/noded.py > @@ -666,8 +666,9 @@ class > NodeRequestHandler(http.server.HttpServerHandler): > """Checks if hotplug is supported. > > """ > - instance = objects.Instance.FromDict(params[0]) > - return backend.HotplugSupported(instance) > + (idict, action, dev_type) = params > + instance = objects.Instance.FromDict(idict) > + return backend.VerifyHotplugSupport(instance, action, dev_type) > > @staticmethod > def perspective_instance_metadata_modify(params): > diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs > index 271b409..cf38673 100644 > --- a/src/Ganeti/OpCodes.hs > +++ b/src/Ganeti/OpCodes.hs > @@ -710,6 +710,7 @@ $(genOpCode "OpCode" > , pDiskChgAmount > , pDiskChgAbsolute > , pIgnoreIpolicy > + , pHotResize > ], > "instance_name") > , ("OpInstanceChangeGroup", > diff --git a/src/Ganeti/OpParams.hs b/src/Ganeti/OpParams.hs > index 63bf34b..5aba89c 100644 > --- a/src/Ganeti/OpParams.hs > +++ b/src/Ganeti/OpParams.hs > @@ -111,6 +111,7 @@ module Ganeti.OpParams > , pIgnoreIpolicy > , pHotplug > , pHotplugIfPossible > + , pHotResize > , pAllowRuntimeChgs > , pInstDisks > , pDiskTemplate > @@ -589,6 +590,9 @@ pHotplug = defaultFalse "hotplug" > pHotplugIfPossible :: Field > pHotplugIfPossible = defaultFalse "hotplug_if_possible" > > +pHotResize :: Field > +pHotResize = defaultFalse "hot_resize" > + > pInstances :: Field > pInstances = > withDoc "List of instances" . > diff --git a/test/hs/Test/Ganeti/OpCodes.hs > b/test/hs/Test/Ganeti/OpCodes.hs > index dfcb483..b5a433c 100644 > --- a/test/hs/Test/Ganeti/OpCodes.hs > +++ b/test/hs/Test/Ganeti/OpCodes.hs > @@ -427,6 +427,7 @@ instance Arbitrary OpCodes.OpCode where > "OP_INSTANCE_GROW_DISK" -> > OpCodes.OpInstanceGrowDisk <$> genFQDN <*> return Nothing <*> > arbitrary <*> arbitrary <*> arbitrary <*> arbitrary <*> > arbitrary > + <*> arbitrary > "OP_INSTANCE_CHANGE_GROUP" -> > OpCodes.OpInstanceChangeGroup <$> genFQDN <*> return Nothing <*> > arbitrary <*> genMaybe genNameNE <*> > -- > 2.1.4 > > -- Yoann Laissus
