On 11/10/2014 05:52 PM, Aaron Karper wrote:
> 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] <mailto:[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 <http://self.instance.name>,
> target_node.name <http://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 <http://logging.info>("Starting instance %s on
> node %s", self.instance.name <http://self.instance.name>,
>> self.cfg.GetNodeName(self.__target_node_uuid))
>>
>> - disks_ok, _ = AssembleInstanceDisks(self.lu <http://self.lu>,
> self.instance,
>> - ignore_secondaries=True)
>> + disks_ok, _, _ = AssembleInstanceDisks(self.lu
> <http://self.lu>, self.instance,
>> + ignore_secondaries=True)
>> if not disks_ok:
>> ShutdownInstanceDisks(self.lu <http://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.
>
ACK, I'll store the payload.
>>
>> 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.
>
ACK
>>
>>
>> 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
--
Alex | [email protected]
