LGTM, thanks.

On Wed, Oct 2, 2013 at 11:02 AM, Raffa Santi <[email protected]> wrote:

> * Add device class object in block_device tuple
> * Update hv_xen.py for new block_devices format
> * Fix tests broken by the change
>
> Signed-off-by: Raffa Santi <[email protected]>
> ---
>  lib/backend.py                               |    2 +-
>  lib/hypervisor/hv_kvm.py                     |   11 ++++++---
>  lib/hypervisor/hv_xen.py                     |    2 +-
>  test/py/ganeti.hypervisor.hv_xen_unittest.py |   31
> +++++++++++++++++---------
>  4 files changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/lib/backend.py b/lib/backend.py
> index 5e27048..a16b22a 100644
> --- a/lib/backend.py
> +++ b/lib/backend.py
> @@ -1636,7 +1636,7 @@ def _GatherAndLinkBlockDevs(instance):
>        raise errors.BlockDeviceError("Cannot create block device symlink:
> %s" %
>                                      e.strerror)
>
> -    block_devices.append((disk, link_name))
> +    block_devices.append((disk, link_name, device))
>
>    return block_devices
>
> diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py
> index 91a3b35..8eb2b00 100644
> --- a/lib/hypervisor/hv_kvm.py
> +++ b/lib/hypervisor/hv_kvm.py
> @@ -1137,7 +1137,7 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>        cache_val = ",cache=%s" % disk_cache
>      else:
>        cache_val = ""
> -    for cfdev, dev_path in block_devices:
> +    for cfdev, dev_path, device in block_devices:
>        if cfdev.mode != constants.DISK_RDWR:
>          raise errors.HypervisorError("Instance has read-only disks which"
>                                       " are not supported by KVM")
> @@ -1149,8 +1149,13 @@ class KVMHypervisor(hv_base.BaseHypervisor):
>          if needs_boot_flag and disk_type != constants.HT_DISK_IDE:
>            boot_val = ",boot=on"
>
> -      drive_val = "file=%s,format=raw%s%s%s" % (dev_path, if_val,
> boot_val,
> -                                                cache_val)
> +      if cfdev.params[constants.LDP_ACCESS] == constants.DISK_USERSPACE:
> +        drive_uri = device.GetUserspaceAccessUri(constants.HT_KVM)
> +      else:
> +        drive_uri = dev_path
> +
> +      drive_val = "file=%s,format=raw%s%s%s" % (drive_uri, if_val,
> +                                                boot_val, cache_val)
>        kvm_cmd.extend(["-drive", drive_val])
>
>      #Now we can specify a different device type for CDROM devices.
> diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py
> index cf697e9..a37bbc9 100644
> --- a/lib/hypervisor/hv_xen.py
> +++ b/lib/hypervisor/hv_xen.py
> @@ -287,7 +287,7 @@ def _GetConfigFileDiskData(block_devices,
> blockdev_prefix,
>
>    disk_data = []
>
> -  for sd_suffix, (cfdev, dev_path) in zip(_letters, block_devices):
> +  for sd_suffix, (cfdev, dev_path, _) in zip(_letters, block_devices):
>      sd_name = blockdev_prefix + sd_suffix
>
>      if cfdev.mode == constants.DISK_RDWR:
> diff --git a/test/py/ganeti.hypervisor.hv_xen_unittest.py b/test/py/
> ganeti.hypervisor.hv_xen_unittest.py
> index 4a3270b..c40e50c 100755
> --- a/test/py/ganeti.hypervisor.hv_xen_unittest.py
> +++ b/test/py/ganeti.hypervisor.hv_xen_unittest.py
> @@ -269,7 +269,9 @@ class TestGetConfigFileDiskData(unittest.TestCase):
>
>    def testManyDisks(self):
>      for offset in [0, 1, 10]:
> -      disks = [(objects.Disk(dev_type=constants.DT_PLAIN), "/tmp/disk/%s"
> % idx)
> +      disks = [(objects.Disk(dev_type=constants.DT_PLAIN),
> +               "/tmp/disk/%s" % idx,
> +               NotImplemented)
>                 for idx in range(len(hv_xen._DISK_LETTERS) + offset)]
>
>        if offset == 0:
> @@ -289,9 +291,11 @@ class TestGetConfigFileDiskData(unittest.TestCase):
>    def testTwoLvDisksWithMode(self):
>      disks = [
>        (objects.Disk(dev_type=constants.DT_PLAIN,
> mode=constants.DISK_RDWR),
> -       "/tmp/diskFirst"),
> +       "/tmp/diskFirst",
> +       NotImplemented),
>        (objects.Disk(dev_type=constants.DT_PLAIN,
> mode=constants.DISK_RDONLY),
> -       "/tmp/diskLast"),
> +       "/tmp/diskLast",
> +       NotImplemented),
>        ]
>
>      result = hv_xen._GetConfigFileDiskData(disks, "hd")
> @@ -304,16 +308,20 @@ class TestGetConfigFileDiskData(unittest.TestCase):
>      disks = [
>        (objects.Disk(dev_type=constants.DT_FILE, mode=constants.DISK_RDWR,
>                      logical_id=[constants.FD_LOOP]),
> -       "/tmp/diskFirst"),
> +       "/tmp/diskFirst",
> +       NotImplemented),
>        (objects.Disk(dev_type=constants.DT_FILE,
> mode=constants.DISK_RDONLY,
>                      logical_id=[constants.FD_BLKTAP]),
> -       "/tmp/diskTwo"),
> +       "/tmp/diskTwo",
> +       NotImplemented),
>        (objects.Disk(dev_type=constants.DT_FILE, mode=constants.DISK_RDWR,
>                      logical_id=[constants.FD_LOOP]),
> -       "/tmp/diskThree"),
> +       "/tmp/diskThree",
> +       NotImplemented),
>        (objects.Disk(dev_type=constants.DT_FILE, mode=constants.DISK_RDWR,
>                      logical_id=[constants.FD_BLKTAP]),
> -       "/tmp/diskLast"),
> +       "/tmp/diskLast",
> +       NotImplemented),
>        ]
>
>      result = hv_xen._GetConfigFileDiskData(disks, "sd")
> @@ -328,7 +336,8 @@ class TestGetConfigFileDiskData(unittest.TestCase):
>      disks = [
>        (objects.Disk(dev_type=constants.DT_FILE, mode=constants.DISK_RDWR,
>                      logical_id=["#unknown#"]),
> -       "/tmp/diskinvalid"),
> +       "/tmp/diskinvalid",
> +       NotImplemented),
>        ]
>
>      self.assertRaises(KeyError, hv_xen._GetConfigFileDiskData, disks,
> "sd")
> @@ -679,9 +688,11 @@ class _TestXenHypervisor(object):
>
>      disks = [
>        (objects.Disk(dev_type=constants.DT_PLAIN,
> mode=constants.DISK_RDWR),
> -       utils.PathJoin(self.tmpdir, "disk0")),
> +       utils.PathJoin(self.tmpdir, "disk0"),
> +       NotImplemented),
>        (objects.Disk(dev_type=constants.DT_PLAIN,
> mode=constants.DISK_RDONLY),
> -       utils.PathJoin(self.tmpdir, "disk1")),
> +       utils.PathJoin(self.tmpdir, "disk1"),
> +       NotImplemented),
>        ]
>
>      inst = objects.Instance(name="server01.example.com",
> --
> 1.7.10.4
>
>


-- 
Thomas Thrainer | Software Engineer | [email protected] |

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

Reply via email to