Hi Dimitris, On Wed, Jan 8, 2014 at 8:55 PM, Dimitris Aragiorgis <[email protected]> wrote: > In case of DRBD, hooks run on both primary (source) and secondary > (target) nodes. To get the same behavior for DTS_EXT_MIRROR, where we > do not have secondary node, we should explicitly add target node to > hooks nodes during instance migration/failover. > > CheckPrereq() of TLMigrateInstance runs before BuildHooksManager(), > thus target_node calculated by Iallocator is available under > self._migrater.target_node. Use this value instead of > self.op.target_node which can be None. > > Update related doc entries. > > Signed-off-by: Dimitris Aragiorgis <[email protected]> > --- > doc/hooks.rst | 8 ++++---- > lib/cmdlib/instance_migration.py | 8 +++++--- > 2 files changed, 9 insertions(+), 7 deletions(-) > > diff --git a/doc/hooks.rst b/doc/hooks.rst > index c7fa9fb..5b317d2 100644 > --- a/doc/hooks.rst > +++ b/doc/hooks.rst > @@ -379,8 +379,8 @@ and secondary before failover. > > :directory: instance-failover > :env. vars: IGNORE_CONSISTENCY, SHUTDOWN_TIMEOUT, OLD_PRIMARY, > OLD_SECONDARY, NEW_PRIMARY, NEW_SECONDARY > -:pre-execution: master node, secondary node > -:post-execution: master node, primary and secondary nodes > +:pre-execution: master node, secondary (target) node > +:post-execution: master node, primary (source) and secondary (target) nodes > > OP_INSTANCE_MIGRATE > ++++++++++++++++++++ > @@ -391,8 +391,8 @@ and secondary before migration. > > :directory: instance-migrate > :env. vars: MIGRATE_LIVE, MIGRATE_CLEANUP, OLD_PRIMARY, OLD_SECONDARY, > NEW_PRIMARY, NEW_SECONDARY > -:pre-execution: master node, primary and secondary nodes > -:post-execution: master node, primary and secondary nodes > +:pre-execution: master node, primary (source) and secondary (target) nodes > +:post-execution: master node, primary (source) and secondary (target) nodes > > > OP_INSTANCE_REMOVE > diff --git a/lib/cmdlib/instance_migration.py > b/lib/cmdlib/instance_migration.py > index a5cbf4d..b680443 100644 > --- a/lib/cmdlib/instance_migration.py > +++ b/lib/cmdlib/instance_migration.py > @@ -134,7 +134,7 @@ class LUInstanceFailover(LogicalUnit): > """ > instance = self._migrater.instance > source_node = instance.primary_node > - target_node = self.op.target_node > + target_node = self._migrater.target_node > env = { > "IGNORE_CONSISTENCY": self.op.ignore_consistency, > "SHUTDOWN_TIMEOUT": self.op.shutdown_timeout, > @@ -159,6 +159,7 @@ class LUInstanceFailover(LogicalUnit): > """ > instance = self._migrater.instance > nl = [self.cfg.GetMasterNode()] + list(instance.secondary_nodes) > + nl.append(self._migrater.target_node) > return (nl, nl + [instance.primary_node]) > > > @@ -197,7 +198,7 @@ class LUInstanceMigrate(LogicalUnit): > """ > instance = self._migrater.instance > source_node = instance.primary_node > - target_node = self.op.target_node > + target_node = self._migrater.target_node > env = BuildInstanceHookEnvByObject(self, instance) > env.update({ > "MIGRATE_LIVE": self._migrater.live, > @@ -211,7 +212,7 @@ class LUInstanceMigrate(LogicalUnit): > env["OLD_SECONDARY"] = target_node > env["NEW_SECONDARY"] = source_node > else: > - env["OLD_SECONDARY"] = env["NEW_SECONDARY"] = None > + env["OLD_SECONDARY"] = env["NEW_SECONDARY"] = ""
Why changing the value from None to an empty string? > > return env > > @@ -222,6 +223,7 @@ class LUInstanceMigrate(LogicalUnit): > instance = self._migrater.instance > snodes = list(instance.secondary_nodes) > nl = [self.cfg.GetMasterNode(), instance.primary_node] + snodes > + nl.append(self._migrater.target_node) > return (nl, nl) > > > -- > 1.7.10.4 > The patch in general looks good, but this seems to be an extension of the functionality of the hook, not a bugfix. As of now, 2.8 is not only stable, but old stable (2.9 is out as well). Therefore we limit new code in the 2.8 branch to fixing actual bugs. Therefore, I'd suggest you to resubmit this for 2.10, which is not stable yet, or to 2.9 if you think there is a good reason to have it out as part of the source code as soon as possible. Thanks, Michele -- 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
