On Thu, Jul 25, 2013 at 1:41 AM, Dimitris Aragiorgis <[email protected]> wrote:
> Introduce new RPC that eventually invoke hypervisor specific
> hotplug functions. In order to be generic it has the following
> arguments: device type, action, device, extra info, seq.
> Device type can be NIC or DISK, action can be ADD, REMOVE,
> device is the NIC or Disk object, extra info is used by Disk
> hotplug to point the device path and seq is the device index
> (from the master perspective)
>
> If hypervisor does not support hotplug the opcode will fail.
>
> Signed-off-by: Dimitris Aragiorgis <[email protected]>
> ---
> lib/backend.py | 26 ++++++++++++++++++++++++++
> lib/rpc.py | 7 +++++++
> lib/rpc_defs.py | 11 ++++++++++-
> lib/server/noded.py | 13 +++++++++++++
> 4 files changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/lib/backend.py b/lib/backend.py
> index 7063aa5..a3d4ee5 100644
> --- a/lib/backend.py
> +++ b/lib/backend.py
> @@ -1934,6 +1934,32 @@ def GetMigrationStatus(instance):
> except Exception, err: # pylint: disable=W0703
> _Fail("Failed to get migration status: %s", err, exc=True)
>
> +def HotplugDevice(instance, action, dev_type, device, 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 extra: string
> + @param extra: extra info used by hotplug code (e.g. disk link)
> + @type seq: int
> + @param seq: the index of the device from master perspective
> + @raise RPCFail: in case instance does not have KVM hypervisor
> +
> + """
> + hyper = hypervisor.GetHypervisor(instance.hypervisor)
> + try:
> + getattr(hyper, "HotplugDevice")
> + except NameError:
> + _Fail("Hotplug is not supported for %s hypervisor",
> + instance.hypervisor, exc=True )
Nack. Please add this to the hypervisor base class with a nice
NotImplementedError, instead of doing introspection here.
> + return hyper.HotplugDevice(instance, action, dev_type, device, extra, seq)
Which as we know doesn't say anything on failure, not failure, what happened....
Nope. Also here's a good way for the if ADD or DEL switch which is
common to all hypervisors.
>
> def BlockdevCreate(disk, size, owner, on_primary, info, excl_stor):
> """Creates a block device for an instance.
> diff --git a/lib/rpc.py b/lib/rpc.py
> index 1058dcc..0219554 100644
> --- a/lib/rpc.py
> +++ b/lib/rpc.py
> @@ -820,6 +820,7 @@ class RpcRunner(_RpcClientBase,
> rpc_defs.ED_INST_DICT_HVP_BEP_DP: self._InstDictHvpBepDp,
> rpc_defs.ED_INST_DICT_OSP_DP: self._InstDictOspDp,
> rpc_defs.ED_NIC_DICT: self._NicDict,
> + rpc_defs.ED_DEVICE_DICT: self._DeviceDict,
>
> # Encoders annotating disk parameters
> rpc_defs.ED_DISKS_DICT_DP: self._DisksDictDP,
> @@ -858,6 +859,12 @@ class RpcRunner(_RpcClientBase,
> n.netinfo = objects.Network.ToDict(nobj)
> return n.ToDict()
>
> + def _DeviceDict(self, device):
> + if isinstance(device, objects.NIC):
> + return self._NicDict(device)
> + elif isinstance(device, objects.Disk):
> + return _ObjectToDict(device)
> +
else raise? assert?
> def _InstDict(self, instance, hvp=None, bep=None, osp=None):
> """Convert the given instance to a dict.
>
> diff --git a/lib/rpc_defs.py b/lib/rpc_defs.py
> index ffb5ceb..77152db 100644
> --- a/lib/rpc_defs.py
> +++ b/lib/rpc_defs.py
> @@ -72,7 +72,8 @@ ACCEPT_OFFLINE_NODE = object()
> ED_DISKS_DICT_DP,
> ED_MULTI_DISKS_DICT_DP,
> ED_SINGLE_DISK_DICT_DP,
> - ED_NIC_DICT) = range(1, 16)
> + ED_NIC_DICT,
> + ED_DEVICE_DICT) = range(1, 17)
>
>
> def _Prepare(calls):
> @@ -296,6 +297,14 @@ _INSTANCE_CALLS = [
> ("reinstall", None, None),
> ("debug", None, None),
> ], None, None, "Starts an instance"),
> + ("hotplug_device", SINGLE, None, constants.RPC_TMO_NORMAL, [
> + ("instance", ED_INST_DICT, "Instance object"),
> + ("action", None, "Hotplug Action"),
> + ("dev_type", None, "Device type"),
> + ("device", ED_DEVICE_DICT, "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"),
> ]
>
> _IMPEXP_CALLS = [
> diff --git a/lib/server/noded.py b/lib/server/noded.py
> index ecc7c95..fab5709 100644
> --- a/lib/server/noded.py
> +++ b/lib/server/noded.py
> @@ -617,6 +617,19 @@ class NodeRequestHandler(http.server.HttpServerHandler):
> return backend.StartInstance(instance, startup_paused, trail)
>
> @staticmethod
> + def perspective_hotplug_device(params):
> + """Hotplugs device to a running instance.
> +
> + """
> + (idict, action, dev_type, ddict, extra, seq) = params
> + instance = objects.Instance.FromDict(idict)
> + if dev_type == constants.HOTPLUG_DISK:
> + device = objects.Disk.FromDict(ddict)
> + elif dev_type == constants.HOTPLUG_NIC:
> + device = objects.NIC.FromDict(ddict)
> + return backend.HotplugDevice(instance, action, dev_type, device, extra,
> seq)
> +
> + @staticmethod
> def perspective_migration_info(params):
> """Gather information about an instance to be migrated.
>
Thanks,
Guido
