LGTM, thanks
On Thu, Jun 12, 2014 at 3:50 PM, Dimitris Aragiorgis <[email protected]> wrote: > ..and let utils compute the hooks environment related to disks. > > Until now, the only exported information regarding disks in hooks > context was their size, mode, name and uuid. > > With this patch, depending on the disk template, we export the info > contained in the logical_id of the disk (e.g. driver, id, provider, > drbd's minors, port, etc.) along with the template name as well. > > Introduce a new helper method, BuildDiskEnv(), that takes > either a disk object or a dict with IDISK_PARAMS returned by > ComputeDisks(), and creates the proper hooks' environment for > the corresponding disk. > > In case the argument is a Disk object then it updates the > environment with the driver and id info extracted from the > logical_id and based on its template (dev_type). > > Signed-off-by: Dimitris Aragiorgis <[email protected]> > --- > doc/hooks.rst | 5 +- > lib/cmdlib/instance.py | 6 +-- > lib/cmdlib/instance_utils.py | 120 > ++++++++++++++++++++++++++++++++++++++---- > 3 files changed, 118 insertions(+), 13 deletions(-) > > diff --git a/doc/hooks.rst b/doc/hooks.rst > index 9c112e7..2e13540 100644 > --- a/doc/hooks.rst > +++ b/doc/hooks.rst > @@ -290,10 +290,13 @@ INSTANCE_NICn_NETWORK, > INSTANCE_NICn_NETWORK_UUID, INSTANCE_NICn_NETWORK_SUBNET, > INSTANCE_NICn_NETWORK_GATEWAY, INSTANCE_NICn_NETWORK_SUBNET6, > INSTANCE_NICn_NETWORK_GATEWAY6, INSTANCE_NICn_NETWORK_MAC_PREFIX, > -INSTANCE_DISK_COUNT, INSTANCE_DISKn_SIZE, INSTANCE_DISKn_MODE. > +INSTANCE_DISK_COUNT, INSTANCE_DISKn_SIZE, INSTANCE_DISKn_MODE, > +INSTANCE_DISKn_NAME, INSTANCE_DISKn_UUID, INSTANCE_DISKn_DEV_TYPE. > > The INSTANCE_NICn_* and INSTANCE_DISKn_* variables represent the > properties of the *n* -th NIC and disk, and are zero-indexed. > +Depending on the disk template, Ganeti exports some info related to > +the logical id of the disk, that is basically its driver and id. > > The INSTANCE_NICn_NETWORK_* variables are only passed if a NIC's network > parameter is set (that is if the NIC is associated to a network defined > diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py > index 95659c4..8b445c9 100644 > --- a/lib/cmdlib/instance.py > +++ b/lib/cmdlib/instance.py > @@ -717,9 +717,9 @@ class LUInstanceCreate(LogicalUnit): > vcpus=self.be_full[constants.BE_VCPUS], > nics=NICListToTuple(self, self.nics), > disk_template=self.op.disk_template, > - disks=[(d[constants.IDISK_NAME], d.get("uuid", ""), > - d[constants.IDISK_SIZE], d[constants.IDISK_MODE]) > - for d in self.disks], > + # Note that self.disks here is not a list with objects.Disk > + # but with dicts as returned by ComputeDisks. > + disks=self.disks, > bep=self.be_full, > hvp=self.hv_full, > hypervisor_name=self.op.hypervisor, > diff --git a/lib/cmdlib/instance_utils.py b/lib/cmdlib/instance_utils.py > index bebdebf..d2418a5 100644 > --- a/lib/cmdlib/instance_utils.py > +++ b/lib/cmdlib/instance_utils.py > @@ -64,7 +64,7 @@ def BuildInstanceHookEnv(name, primary_node_name, > secondary_node_names, os_type, > @type disk_template: string > @param disk_template: the disk template of the instance > @type disks: list > - @param disks: list of tuples (name, uuid, size, mode) > + @param disks: list of disks (either objects.Disk or dict) > @type bep: dict > @param bep: the backend parameters for the instance > @type hvp: dict > @@ -124,12 +124,8 @@ def BuildInstanceHookEnv(name, primary_node_name, > secondary_node_names, os_type, > > if disks: > disk_count = len(disks) > - for idx, (name, uuid, size, mode) in enumerate(disks): > - if name: > - env["INSTANCE_DISK%d_NAME" % idx] = name > - env["INSTANCE_DISK%d_UUID" % idx] = uuid > - env["INSTANCE_DISK%d_SIZE" % idx] = size > - env["INSTANCE_DISK%d_MODE" % idx] = mode > + for idx, disk in enumerate(disks): > + env.update(BuildDiskEnv(idx, disk)) > else: > disk_count = 0 > > @@ -186,8 +182,7 @@ def BuildInstanceHookEnvByObject(lu, instance, > secondary_nodes=None, > "vcpus": bep[constants.BE_VCPUS], > "nics": NICListToTuple(lu, instance.nics), > "disk_template": instance.disk_template, > - "disks": [(disk.name, disk.uuid, disk.size, disk.mode) > - for disk in disks], > + "disks": disks, > "bep": bep, > "hvp": hvp, > "hypervisor_name": instance.hypervisor, > @@ -623,3 +618,110 @@ def CheckCompressionTool(lu, compression_tool): > "Compression tool not allowed, tools allowed are [%s]" > % ", ".join(allowed_tools) > ) > + > + > +def BuildDiskLogicalIDEnv(idx, disk): > + """Helper method to create hooks env related to disk's logical_id > + > + @type idx: integer > + @param idx: The index of the disk > + @type disk: L{objects.Disk} > + @param disk: The disk object > + > + """ > + if disk.dev_type == constants.DT_PLAIN: > + vg, name = disk.logical_id > + ret = { > + "INSTANCE_DISK%d_VG" % idx: vg, > + "INSTANCE_DISK%d_ID" % idx: name > + } > + elif disk.dev_type in (constants.DT_FILE, constants.DT_SHARED_FILE): > + file_driver, name = disk.logical_id > + ret = { > + "INSTANCE_DISK%d_DRIVER" % idx: file_driver, > + "INSTANCE_DISK%d_ID" % idx: name > + } > + elif disk.dev_type == constants.DT_BLOCK: > + block_driver, adopt = disk.logical_id > + ret = { > + "INSTANCE_DISK%d_DRIVER" % idx: block_driver, > + "INSTANCE_DISK%d_ID" % idx: adopt > + } > + elif disk.dev_type == constants.DT_RBD: > + rbd, name = disk.logical_id > + ret = { > + "INSTANCE_DISK%d_DRIVER" % idx: rbd, > + "INSTANCE_DISK%d_ID" % idx: name > + } > + elif disk.dev_type == constants.DT_EXT: > + provider, name = disk.logical_id > + ret = { > + "INSTANCE_DISK%d_PROVIDER" % idx: provider, > + "INSTANCE_DISK%d_ID" % idx: name > + } > + elif disk.dev_type == constants.DT_DRBD8: > + pnode, snode, port, pmin, smin, _ = disk.logical_id > + data, meta = disk.children > + data_vg, data_name = data.logical_id > + meta_vg, meta_name = meta.logical_id > + ret = { > + "INSTANCE_DISK%d_PNODE" % idx: pnode, > + "INSTANCE_DISK%d_SNODE" % idx: snode, > + "INSTANCE_DISK%d_PORT" % idx: port, > + "INSTANCE_DISK%d_PMINOR" % idx: pmin, > + "INSTANCE_DISK%d_SMINOR" % idx: smin, > + "INSTANCE_DISK%d_DATA_VG" % idx: data_vg, > + "INSTANCE_DISK%d_DATA_ID" % idx: data_name, > + "INSTANCE_DISK%d_META_VG" % idx: meta_vg, > + "INSTANCE_DISK%d_META_ID" % idx: meta_name, > + } > + elif disk.dev_type == constants.DT_GLUSTER: > + file_driver, name = disk.logical_id > + ret = { > + "INSTANCE_DISK%d_DRIVER" % idx: file_driver, > + "INSTANCE_DISK%d_ID" % idx: name > + } > + elif disk.dev_type == constants.DT_DISKLESS: > + ret = {} > + else: > + ret = {} > + > + ret.update({ > + "INSTANCE_DISK%d_DEV_TYPE" % idx: disk.dev_type > + }) > + > + return ret > + > + > +def BuildDiskEnv(idx, disk): > + """Helper method to create disk's hooks env > + > + @type idx: integer > + @param idx: The index of the disk > + @type disk: L{objects.Disk} or dict > + @param disk: The disk object or a simple dict in case of > LUInstanceCreate > + > + """ > + ret = {} > + # In case of LUInstanceCreate this runs in CheckPrereq where lu.disks > + # is a list of dicts i.e the result of ComputeDisks > + if isinstance(disk, dict): > + uuid = disk.get("uuid", "") > + name = disk.get(constants.IDISK_NAME, "") > + size = disk.get(constants.IDISK_SIZE, "") > + mode = disk.get(constants.IDISK_MODE, "") > + elif isinstance(disk, objects.Disk): > + uuid = disk.uuid > + name = disk.name > + size = disk.size > + mode = disk.mode > + ret.update(BuildDiskLogicalIDEnv(idx, disk)) > + > + # only name is optional here > + if name: > + ret["INSTANCE_DISK%d_NAME" % idx] = name > + ret["INSTANCE_DISK%d_UUID" % idx] = uuid > + ret["INSTANCE_DISK%d_SIZE" % idx] = size > + ret["INSTANCE_DISK%d_MODE" % idx] = mode > + > + return ret > -- > 1.7.10.4 > > -- -- Helga Velroyen | Software Engineer | [email protected] | Google Germany GmbH Dienerstr. 12 80331 München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores
