LGTM, thanks

On Mon, Nov 11, 2013 at 6:44 PM, Santi Raffa <[email protected]> wrote:

> DTS_FILEBASED is a constant that exists and this commit makes sure
> that it is used whenever sensible, rather than resorting to hardcoding
> the pair of templates in very many files.
>
> Signed-off-by: Santi Raffa <[email protected]>
> ---
>  lib/backend.py                            | 2 +-
>  lib/cmdlib/backup.py                      | 2 +-
>  lib/cmdlib/instance.py                    | 3 +--
>  lib/cmdlib/instance_storage.py            | 2 +-
>  lib/hypervisor/hv_xen.py                  | 2 +-
>  qa/ganeti-qa.py                           | 6 ++----
>  qa/qa_cluster.py                          | 2 +-
>  test/py/cmdlib/instance_unittest.py       | 2 +-
>  test/py/cmdlib/testsupport/config_mock.py | 2 +-
>  test/py/ganeti.utils.storage_unittest.py  | 2 +-
>  tools/move-instance                       | 3 +--
>  11 files changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/lib/backend.py b/lib/backend.py
> index 1fccfd0..52366c2 100644
> --- a/lib/backend.py
> +++ b/lib/backend.py
> @@ -2939,7 +2939,7 @@ def OSEnvironment(instance, inst_os, debug=0):
>          instance.hvparams[constants.HV_DISK_TYPE]
>      if disk.dev_type in constants.DTS_BLOCK:
>        result["DISK_%d_BACKEND_TYPE" % idx] = "block"
> -    elif disk.dev_type in [constants.DT_FILE, constants.DT_SHARED_FILE]:
> +    elif disk.dev_type in constants.DTS_FILEBASED:
>        result["DISK_%d_BACKEND_TYPE" % idx] = \
>          "file:%s" % disk.logical_id[0]
>
> diff --git a/lib/cmdlib/backup.py b/lib/cmdlib/backup.py
> index ab4c807..0054817 100644
> --- a/lib/cmdlib/backup.py
> +++ b/lib/cmdlib/backup.py
> @@ -341,7 +341,7 @@ class LUBackupExport(LogicalUnit):
>      # instance disk type verification
>      # TODO: Implement export support for file-based disks
>      for disk in self.instance.disks:
> -      if disk.dev_type in [constants.DT_FILE, constants.DT_SHARED_FILE]:
> +      if disk.dev_type in constants.DTS_FILEBASED:
>          raise errors.OpPrereqError("Export not supported for instances
> with"
>                                     " file-based disks",
> errors.ECODE_INVAL)
>
> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
> index 884c8bc..5069b9d 100644
> --- a/lib/cmdlib/instance.py
> +++ b/lib/cmdlib/instance.py
> @@ -463,8 +463,7 @@ class LUInstanceCreate(LogicalUnit):
>
>      # set default file_driver if unset and required
>      if (not self.op.file_driver and
> -        self.op.disk_template in [constants.DT_FILE,
> -                                  constants.DT_SHARED_FILE]):
> +        self.op.disk_template in constants.DTS_FILEBASED):
>        self.op.file_driver = constants.FD_LOOP
>
>      ### Node/iallocator related checks
> diff --git a/lib/cmdlib/instance_storage.py
> b/lib/cmdlib/instance_storage.py
> index 7d36060..eb022ce 100644
> --- a/lib/cmdlib/instance_storage.py
> +++ b/lib/cmdlib/instance_storage.py
> @@ -459,7 +459,7 @@ def GenerateDiskTemplate(
>          vg = disk.get(constants.IDISK_VG, vgname)
>          return (vg, names[idx])
>
> -    elif template_name in (constants.DT_FILE, constants.DT_SHARED_FILE):
> +    elif template_name in constants.DTS_FILEBASED:
>        logical_id_fn = \
>          lambda _, disk_index, disk: (file_driver,
>                                       "%s/disk%d" % (file_storage_dir,
> diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py
> index 742c8be..bbb1140 100644
> --- a/lib/hypervisor/hv_xen.py
> +++ b/lib/hypervisor/hv_xen.py
> @@ -295,7 +295,7 @@ def _GetConfigFileDiskData(block_devices,
> blockdev_prefix,
>      else:
>        mode = "r"
>
> -    if cfdev.dev_type in [constants.DT_FILE, constants.DT_SHARED_FILE]:
> +    if cfdev.dev_type in constants.DTS_FILEBASED:
>        driver = _FILE_DRIVER_MAP[cfdev.logical_id[0]]
>      else:
>        driver = "phy"
> diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py
> index cc67bfe..bf26cc5 100755
> --- a/qa/ganeti-qa.py
> +++ b/qa/ganeti-qa.py
> @@ -422,8 +422,7 @@ def RunExportImportTests(instance, inodes):
>    # based storage types are untested, though. Also note that import could
> still
>    # work, but is deeply embedded into the "export" case.
>    if (qa_config.TestEnabled("instance-export") and
> -      instance.disk_template not in [constants.DT_FILE,
> -                                     constants.DT_SHARED_FILE]):
> +      instance.disk_template not in constants.DTS_FILEBASED):
>      RunTest(qa_instance.TestInstanceExportNoTarget, instance)
>
>      pnode = inodes[0]
> @@ -449,8 +448,7 @@ def RunExportImportTests(instance, inodes):
>    # FIXME: inter-cluster-instance-move crashes on file based instances :/
>    # See Issue 414.
>    if (qa_config.TestEnabled([qa_rapi.Enabled,
> "inter-cluster-instance-move"])
> -      and (instance.disk_template not in
> -           [constants.DT_FILE, constants.DT_SHARED_FILE])):
> +      and (instance.disk_template not in constants.DTS_FILEBASED)):
>      newinst = qa_config.AcquireInstance()
>      try:
>        tnode = qa_config.AcquireNode(exclude=inodes)
> diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
> index 2b429c9..119472b 100644
> --- a/qa/qa_cluster.py
> +++ b/qa/qa_cluster.py
> @@ -492,7 +492,7 @@ def TestClusterModifyFileBasedStorageDir(
>
>    """
>    enabled_disk_templates = qa_config.GetEnabledDiskTemplates()
> -  assert file_disk_template in [constants.DT_FILE,
> constants.DT_SHARED_FILE]
> +  assert file_disk_template in constants.DTS_FILEBASED
>    if not qa_config.IsTemplateSupported(file_disk_template):
>      return
>
> diff --git a/test/py/cmdlib/instance_unittest.py
> b/test/py/cmdlib/instance_unittest.py
> index ab8423c..062d2f1 100644
> --- a/test/py/cmdlib/instance_unittest.py
> +++ b/test/py/cmdlib/instance_unittest.py
> @@ -1098,7 +1098,7 @@ class TestGenerateDiskTemplate(CmdlibTestCase):
>      self.assertRaises(errors.OpPrereqError, self._TestTrivialDisk,
>                        constants.DT_SHARED_FILE, [], 0, NotImplemented)
>
> -    for disk_template in [constants.DT_FILE, constants.DT_SHARED_FILE]:
> +    for disk_template in constants.DTS_FILEBASED:
>        disk_info = [{
>          constants.IDISK_SIZE: 80 * 1024,
>          constants.IDISK_MODE: constants.DISK_RDONLY,
> diff --git a/test/py/cmdlib/testsupport/config_mock.py
> b/test/py/cmdlib/testsupport/config_mock.py
> index 0cecdd3..71b7779 100644
> --- a/test/py/cmdlib/testsupport/config_mock.py
> +++ b/test/py/cmdlib/testsupport/config_mock.py
> @@ -399,7 +399,7 @@ class ConfigMock(config.ConfigWriter):
>      elif dev_type == constants.DT_PLAIN:
>        if logical_id is None:
>          logical_id = ("mockvg", "mock_disk_%d" % disk_id)
> -    elif dev_type in (constants.DT_FILE, constants.DT_SHARED_FILE):
> +    elif dev_type in constants.DTS_FILEBASED:
>        if logical_id is None:
>          logical_id = (constants.FD_LOOP, "/file/storage/disk%d" % disk_id)
>      elif dev_type == constants.DT_BLOCK:
> diff --git a/test/py/ganeti.utils.storage_unittest.py b/test/py/
> ganeti.utils.storage_unittest.py
> index 1f2a697..92b1648 100755
> --- a/test/py/ganeti.utils.storage_unittest.py
> +++ b/test/py/ganeti.utils.storage_unittest.py
> @@ -80,7 +80,7 @@ class TestGetStorageUnits(unittest.TestCase):
>      self._cfg = mock.Mock()
>
>    def testGetStorageUnits(self):
> -    disk_templates = [constants.DT_FILE, constants.DT_SHARED_FILE]
> +    disk_templates = constants.DTS_FILEBASED
>      storage_units = storage.GetStorageUnits(self._cfg, disk_templates)
>      self.assertEqual(len(storage_units), len(disk_templates))
>
> diff --git a/tools/move-instance b/tools/move-instance
> index d4247d3..589e06d 100755
> --- a/tools/move-instance
> +++ b/tools/move-instance
> @@ -600,8 +600,7 @@ class MoveSourceExecutor(object):
>      logging.info("Retrieving instance information from source cluster")
>      instinfo = self._GetInstanceInfo(src_client, mrt.PollJob,
>                                       mrt.move.src_instance_name)
> -    if (instinfo["disk_template"] in
> -        [constants.DT_FILE, constants.DT_SHARED_FILE]):
> +    if (instinfo["disk_template"] in constants.DTS_FILEBASED):
>        raise Error("Inter-cluster move of file-based instances is not"
>                    " supported.")
>
> --
> 1.8.4.1
>
>


Hrvoje Ribicic
Ganeti Engineering
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
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370

Reply via email to