* Guido Trotter <[email protected]> [2013-07-30 13:32:39 +0200]: > 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. >
Noted that already.
> > + 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.
>
I got lost again with the "add or del switch"..Please explain that a bit more.
> >
> > 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?
>
This will be a no-way scenario so assert fits I guess.
> > 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 ,
dimara
signature.asc
Description: Digital signature
