Thanks for the patch.
On Thu, Nov 06, 2014 at 12:30:40AM +0200, Alex Pyrgiotis wrote:
> The AssembleInstanceDisks function uses internally the
> call_blockdev_assemble RPC call. It does two passes with this function,
> the first in the secondary and the last in the primary node. The results
> from the latter operation can be useful to the caller for hotplugging
> reasons, so we return them in array format.
>
> Signed-off-by: Alex Pyrgiotis <[email protected]>
>
> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
> index b8e23b5..c61da4f 100644
> --- a/lib/cmdlib/instance.py
> +++ b/lib/cmdlib/instance.py
> @@ -511,8 +511,8 @@ class LUInstanceMove(LogicalUnit):
> self.LogInfo("Starting instance %s on node %s",
> self.instance.name, target_node.name)
>
> - disks_ok, _ = AssembleInstanceDisks(self, self.instance,
> - ignore_secondaries=True)
> + disks_ok, _, _ = AssembleInstanceDisks(self, self.instance,
> + ignore_secondaries=True)
> if not disks_ok:
> ShutdownInstanceDisks(self, self.instance)
> raise errors.OpExecError("Can't activate the instance's disks")
> diff --git a/lib/cmdlib/instance_migration.py b/lib/cmdlib/instance_
migration.py
> index fdfedb2..a548690 100644
> --- a/lib/cmdlib/instance_migration.py
> +++ b/lib/cmdlib/instance_migration.py
> @@ -952,8 +952,8 @@ class TLMigrateInstance(Tasklet):
> logging.info("Starting instance %s on node %s", self.instance.name,
> self.cfg.GetNodeName(self.target_node_uuid))
>
> - disks_ok, _ = AssembleInstanceDisks(self.lu, self.instance,
> - ignore_secondaries=True)
> + disks_ok, _, _ = AssembleInstanceDisks(self.lu, self.instance,
> + ignore_secondaries=True)
> if not disks_ok:
> ShutdownInstanceDisks(self.lu, self.instance)
> raise errors.OpExecError("Can't activate the instance's disks")
> diff --git a/lib/cmdlib/instance_storage.py b/lib/cmdlib/instance_storage.
py
> index dedbba8..24d50de 100644
> --- a/lib/cmdlib/instance_storage.py
> +++ b/lib/cmdlib/instance_storage.py
> @@ -1544,6 +1544,7 @@ def AssembleInstanceDisks(lu, instance, disks=None,
ignore_secondaries=False,
> """
> device_info = []
> disks_ok = True
> + results = []
>
> if disks is None:
> # only mark instance disks as active if all disks are affected
> @@ -1596,6 +1597,7 @@ def AssembleInstanceDisks(lu, instance, disks=None,
ignore_secondaries=False,
> node_disk.UnsetSize()
> result = lu.rpc.call_blockdev_assemble(node_uuid, (node_disk,
instance),
> instance, True, idx)
> + results.append(result)
Nitpick: Is it necessary to store the result instead of the payload? That
seems like an implementation detail better hidden.
> msg = result.fail_msg
> if msg:
> lu.LogWarning("Could not prepare block device %s on node %s"
> @@ -1611,7 +1613,7 @@ def AssembleInstanceDisks(lu, instance, disks=None,
ignore_secondaries=False,
> if not disks_ok:
> lu.cfg.MarkInstanceDisksInactive(instance.uuid)
>
> - return disks_ok, device_info
> + return disks_ok, device_info, results
Please update the documentation for the return value.
>
>
> def StartInstanceDisks(lu, instance, force):
> @@ -1621,8 +1623,8 @@ def StartInstanceDisks(lu, instance, force):
> instance configuration, if needed.
>
> """
> - disks_ok, _ = AssembleInstanceDisks(lu, instance,
> - ignore_secondaries=force)
> + disks_ok, _, _ = AssembleInstanceDisks(lu, instance,
> + ignore_secondaries=force)
> if not disks_ok:
> ShutdownInstanceDisks(lu, instance)
> if force is not None and not force:
> @@ -1740,7 +1742,8 @@ class LUInstanceGrowDisk(LogicalUnit):
>
> wipe_disks = self.cfg.GetClusterInfo().prealloc_wipe_disks
>
> - disks_ok, _ = AssembleInstanceDisks(self, self.instance,
disks=[self.disk])
> + disks_ok, _, _ = AssembleInstanceDisks(self, self.instance,
> + disks=[self.disk])
> if not disks_ok:
> raise errors.OpExecError("Cannot activate block device to grow")
>
> @@ -2008,9 +2011,9 @@ class LUInstanceActivateDisks(NoHooksLU):
> """Activate the disks.
>
> """
> - disks_ok, disks_info = \
> - AssembleInstanceDisks(self, self.instance,
> - ignore_size=self.op.ignore_size)
> + disks_ok, disks_info, _ = AssembleInstanceDisks(
> + self, self.instance, ignore_size=self.op.ignore_size)
> +
> if not disks_ok:
> raise errors.OpExecError("Cannot activate block devices")
>
> --
> 1.7.10.4
>
--
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
