There is one subtle side effect of this change that if a disk in 'disks' is a DRBD disk, it must have its nodes filled - before the function took the nodes from the instance's existing disks. There are two places where it could potentially cause problems: In class TemporaryDisk and in LUInstanceSetParams. In the latter case I believe that GenerateDisktemplate provides the nodes correctly, but in TemporaryDisk I'll need to check.
On Tue, Jun 24, 2014 at 4:19 PM, Dimitris Bliablias <[email protected]> wrote: > Currently, the 'all_node_uuids' variable in the 'CreateDisks' method is > computed from all the disks of the instance. This approach will not be > compatible with the upcoming disk template conversion feature. A use > case is the conversions targeting the 'drbd' template. In that case the > 'all_node_uuids' will be wrongly computed from the current instance's > disks that do not contain the secondary node's uuid of the new disks, > causing them to be only created in the primary node of the instance. > Removing the 'instance_disks' argument and using the 'disks' argument > instead, solves this issue without affecting existing functionality. > > Signed-off-by: Dimitris Bliablias <[email protected]> > --- > lib/cmdlib/instance.py | 2 +- > lib/cmdlib/instance_storage.py | 19 +++++++------------ > 2 files changed, 8 insertions(+), 13 deletions(-) > > diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py > index 8c31e66..3218801 100644 > --- a/lib/cmdlib/instance.py > +++ b/lib/cmdlib/instance.py > @@ -1727,7 +1727,7 @@ class LUInstanceCreate(LogicalUnit): > else: > feedback_fn("* creating instance disks...") > try: > - CreateDisks(self, iobj, instance_disks=disks) > + CreateDisks(self, iobj, disks=disks) > except errors.OpExecError: > self.LogWarning("Device creation failed") > self.cfg.ReleaseDRBDMinors(instance_uuid) > diff --git a/lib/cmdlib/instance_storage.py > b/lib/cmdlib/instance_storage.py > index f31806a..8282932 100644 > --- a/lib/cmdlib/instance_storage.py > +++ b/lib/cmdlib/instance_storage.py > @@ -202,8 +202,7 @@ def _UndoCreateDisks(lu, disks_created, instance): > (disk, lu.cfg.GetNodeName(node_uuid)), logging.warning) > > > -def CreateDisks(lu, instance, instance_disks=None, > - to_skip=None, target_node_uuid=None, disks=None): > +def CreateDisks(lu, instance, to_skip=None, target_node_uuid=None, > disks=None): > """Create all disks for an instance. > > This abstracts away some work from AddInstance. > @@ -216,9 +215,6 @@ def CreateDisks(lu, instance, instance_disks=None, > @param lu: the logical unit on whose behalf we execute > @type instance: L{objects.Instance} > @param instance: the instance whose disks we should create > - @type instance_disks: list of L{objects.Disk} > - @param instance_disks: the disks that belong to the instance; if not > - specified, retrieve them from config file > @type to_skip: list > @param to_skip: list of indices to skip > @type target_node_uuid: string > @@ -232,15 +228,17 @@ def CreateDisks(lu, instance, instance_disks=None, > > """ > info = GetInstanceInfoText(instance) > - if instance_disks is None: > - instance_disks = lu.cfg.GetInstanceDisks(instance.uuid) > + > + if disks is None: > + disks = lu.cfg.GetInstanceDisks(instance.uuid) > + > if target_node_uuid is None: > pnode_uuid = instance.primary_node > # We cannot use config's 'GetInstanceNodes' here as 'CreateDisks' > # is used by 'LUInstanceCreate' and the instance object is not > # stored in the config yet. > all_node_uuids = [] > - for disk in instance_disks: > + for disk in disks: > all_node_uuids.extend(disk.all_nodes) > all_node_uuids = set(all_node_uuids) > # ensure that primary node is always the first > @@ -250,13 +248,10 @@ def CreateDisks(lu, instance, instance_disks=None, > pnode_uuid = target_node_uuid > all_node_uuids = [pnode_uuid] > > - if disks is None: > - disks = instance_disks > - > CheckDiskTemplateEnabled(lu.cfg.GetClusterInfo(), > instance.disk_template) > > if instance.disk_template in constants.DTS_FILEBASED: > - file_storage_dir = os.path.dirname(instance_disks[0].logical_id[1]) > + file_storage_dir = os.path.dirname(disks[0].logical_id[1]) > result = lu.rpc.call_file_storage_dir_create(pnode_uuid, > file_storage_dir) > > result.Raise("Failed to create directory '%s' on" > -- > 1.7.10.4 > >
