On Tue, Jul 30, 2013 at 8:57 PM, Dimitris Aragiorgis <[email protected]> wrote:
> * 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.
>
What I said in the other patch about having HotAddDevice,
HotDelDevice, and doing the case switch here.
>> >
>> > 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.
>
Ack,
>> > 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
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iQEcBAEBCAAGBQJR+AymAAoJEMk10sCdtK5jpRMH/Rbvu3mQw/3aZrBueLMnmPob
> T7QttMuW0kdp2Lr+gqrnC185gNydtIpkG2LWD3clfy8iASaAjyIWpXsawKx6DA3d
> FdnFKuEXS19gzDurEl7N0WMyXxxZOSgnZSDompqWf7CvT6OHYoFvpSZDWZ24t15h
> 7gFiVt5wkDJDzI/weSOw0/YiC3FVnSu457gSlRuVm39D+qJDVLIks4EcM6LXkpu+
> aCaMvouA7urlirt44rfpxTn61jsd+nvJCtG/YNEq54tKkhOsSqsvhbiqoYEJiSrN
> UAVGVbzE/PlM7XSF5tPY0KvD6t5L9ASl+4lzIdS8SEUAuV7hqQ7KkTJqF4sBo3s=
> =oXXb
> -----END PGP SIGNATURE-----
>
Thanks,
Guido
