On Tue, Oct 7, 2014 at 3:00 PM, 'Aaron Karper' via ganeti-devel
<[email protected]> wrote:
> The check is unnecessary because earlier the disk template is checked in
> a more restrictive way. Newly the check works on disk types, but gets
> rid of the redundancy.
>
> Signed-off-by: Aaron Karper <[email protected]>
> ---
> lib/cmdlib/instance.py | 17 ++++++-----------
> test/py/cmdlib/instance_unittest.py | 3 ++-
> 2 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
> index 0ad343d..9659ca7 100644
> --- a/lib/cmdlib/instance.py
> +++ b/lib/cmdlib/instance.py
> @@ -2111,10 +2111,12 @@ class LUInstanceMove(LogicalUnit):
> assert self.instance is not None, \
> "Cannot retrieve locked instance %s" % self.op.instance_name
>
> - if self.instance.disk_template not in constants.DTS_COPYABLE:
> - raise errors.OpPrereqError("Disk template %s not suitable for copying"
> %
> - self.instance.disk_template,
> - errors.ECODE_STATE)
> + disks = self.cfg.GetInstanceDisks(self.instance.uuid)
> + for idx, dsk in enumerate(disks):
> + if dsk.dev_type not in constants.DTS_COPYABLE:
> + raise errors.OpPrereqError("Instance disk %d has layout %s and is
> not"
Why calling it "layout" instead of "disk type"? Also, I'd leave
"suitable for copying" as it was.
> + " suitable for moving instances"
> + % (idx, dsk.dev_type), errors.ECODE_STATE)
>
> target_node = self.cfg.GetNodeInfo(self.op.target_node_uuid)
> assert target_node is not None, \
> @@ -2129,13 +2131,6 @@ class LUInstanceMove(LogicalUnit):
> cluster = self.cfg.GetClusterInfo()
> bep = cluster.FillBE(self.instance)
>
> - disks = self.cfg.GetInstanceDisks(self.instance.uuid)
> - for idx, dsk in enumerate(disks):
> - if dsk.dev_type not in (constants.DT_PLAIN, constants.DT_FILE,
> - constants.DT_SHARED_FILE,
> constants.DT_GLUSTER):
> - raise errors.OpPrereqError("Instance disk %d has a complex layout,"
> - " cannot copy" % idx, errors.ECODE_STATE)
> -
> CheckNodeOnline(self, target_node.uuid)
> CheckNodeNotDrained(self, target_node.uuid)
> CheckNodeVmCapable(self, target_node.uuid)
> diff --git a/test/py/cmdlib/instance_unittest.py
> b/test/py/cmdlib/instance_unittest.py
> index ac2f082..e22025d 100644
> --- a/test/py/cmdlib/instance_unittest.py
> +++ b/test/py/cmdlib/instance_unittest.py
> @@ -1566,7 +1566,8 @@ class TestLUInstanceMove(CmdlibTestCase):
> op = opcodes.OpInstanceMove(instance_name=inst.name,
> target_node=self.node.name)
> self.ExecOpCodeExpectOpPrereqError(
> - op, "Disk template sharedfile not suitable for copying")
> + op, "Instance disk 0 has layout sharedfile and is not suitable"
> + " for moving instances")
>
> def testAlreadyOnTargetNode(self):
> inst = self.cfg.AddNewInstance()
> --
> 2.1.0.rc2.206.gedb03e5
>
Rest LGTM, 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
