On Thu, May 05, 2011 at 02:12:23PM +0300, Apollon Oikonomopoulos wrote:
> On 12:58 Thu 05 May , Iustin Pop wrote:
> > From: Apollon Oikonomopoulos <[email protected]>
> >
> > Commit faaabe3c fixed failover behaviour for DTS_INT_MIRROR instances,
> > however
> > it broke migration for DTS_EXT_MIRROR instances, by moving iallocator and
> > node
> > checks from LUInstanceMigrate to TLMigrateInstance. This has the side-effect
> > that the LU called the TL with None for both, node and iallocator when the
> > default iallocator was being used.
> >
> > This patch maintains the iallocator checks in TLMigrateInstance and fixes
> > the
> > LU-TL integration.
> >
> > Signed-off-by: Iustin Pop <[email protected]>
> > [[email protected]: rebased patch on current HEAD]
> > Reviewed-by: Iustin Pop <[email protected]>
> > ---
> > Apollon, could I get a signed-off-by on this rebased patch? I believe I
> > kept
> > the desired changes and fixed the conflicts correctly.
> >
> > lib/cmdlib.py | 28 ++++++++++++----------------
> > 1 files changed, 12 insertions(+), 16 deletions(-)
> >
> > diff --git a/lib/cmdlib.py b/lib/cmdlib.py
> > index c4fd9f5..edd610a 100644
> > --- a/lib/cmdlib.py
> > +++ b/lib/cmdlib.py
> > @@ -6090,8 +6090,6 @@ class LUInstanceMigrate(LogicalUnit):
> >
> > self._migrater = TLMigrateInstance(self, self.op.instance_name,
> > cleanup=self.op.cleanup,
> > - iallocator=self.op.iallocator,
> > - target_node=self.op.target_node,
> > failover=False,
> > fallback=self.op.allow_failover)
> > self.tasklets = [self._migrater]
> > @@ -6353,9 +6351,7 @@ class LUNodeMigrate(LogicalUnit):
> > logging.debug("Migrating instance %s", inst.name)
> > names.append(inst.name)
> >
> > - tasklets.append(TLMigrateInstance(self, inst.name, cleanup=False,
> > - iallocator=self.op.iallocator,
> > - taget_node=None))
> > + tasklets.append(TLMigrateInstance(self, inst.name, cleanup=False)
> >
> > if inst.disk_template in constants.DTS_EXT_MIRROR:
> > # We need to lock all nodes, as the iallocator will choose the
> > @@ -6420,8 +6416,8 @@ class TLMigrateInstance(Tasklet):
> > @ivar shutdown_timeout: In case of failover timeout of the shutdown
> >
> > """
> > - def __init__(self, lu, instance_name, cleanup=False, iallocator=None,
> > - target_node=None, failover=False, fallback=False,
> > + def __init__(self, lu, instance_name, cleanup=False,
> > + failover=False, fallback=False,
> > ignore_consistency=False,
> > shutdown_timeout=constants.DEFAULT_SHUTDOWN_TIMEOUT):
> > """Initializes this class.
> > @@ -6433,8 +6429,6 @@ class TLMigrateInstance(Tasklet):
> > self.instance_name = instance_name
> > self.cleanup = cleanup
> > self.live = False # will be overridden later
> > - self.iallocator = iallocator
> > - self.target_node = target_node
> > self.failover = failover
> > self.fallback = fallback
> > self.ignore_consistency = ignore_consistency
> > @@ -6469,11 +6463,13 @@ class TLMigrateInstance(Tasklet):
> > if instance.disk_template in constants.DTS_EXT_MIRROR:
> > _CheckIAllocatorOrNode(self.lu, "iallocator", "target_node")
> >
> > - if self.iallocator:
> > + if self.lu.op.iallocator:
> > self._RunAllocator()
> > + else:
> > + # We set set self.target_node as it is required by
> > + # BuildHooksEnv
> > + self.target_node = self.lu.op.target_node
> >
> > - # self.target_node is already populated, either directly or by the
> > - # iallocator run
> > target_node = self.target_node
>
> Re-reading this, I think the last comment should probably stay, I
> shouldn't have removed it. Rest LGTM.
Argh, I forgot to run make commit-check and found two issues: one typo
and one fix for new code that appeared in the meantime:
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index 1978f1f..ae5acf7 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -6486,8 +6486,8 @@ class TLMigrateInstance(Tasklet):
" %s disk template" %
instance.disk_template)
target_node = secondary_nodes[0]
- if self.iallocator or (self.target_node and
- self.target_node != target_node):
+ if self.lu.op.iallocator or (self.target_node and
+ self.target_node != target_node):
if self.failover:
text = "failed over"
else:
I'll commit with these changes, thanks.
iustin
