Additionally:
* fixed some spurious indentation
* removed the spurious import to storage/__init__.py at the source
Interdiff:
diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py
index 309a177..8eb2b00 100644
--- a/lib/hypervisor/hv_kvm.py
+++ b/lib/hypervisor/hv_kvm.py
@@ -1150,10 +1150,12 @@ class KVMHypervisor(hv_base.BaseHypervisor):
boot_val = ",boot=on"
if cfdev.params[constants.LDP_ACCESS] == constants.DISK_USERSPACE:
- dev_path = device.GetUserspaceAccessUri(constants.HT_KVM)
+ drive_uri = device.GetUserspaceAccessUri(constants.HT_KVM)
+ else:
+ drive_uri = dev_path
- drive_val = "file=%s,format=raw%s%s%s" % (dev_path, if_val,
- boot_val, cache_val)
+ 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/storage/__init__.py b/lib/storage/__init__.py
index efe9600..9b9e38c 100644
--- a/lib/storage/__init__.py
+++ b/lib/storage/__init__.py
@@ -22,5 +22,3 @@
"""Block device abstraction
"""
-
-from ganeti.storage import base
diff --git a/test/py/ganeti.hypervisor.hv_xen_unittest.py
b/test/py/ganeti.hypervisor.hv_xen_unittest.py
index 75f22dc..c40e50c 100755
--- a/test/py/ganeti.hypervisor.hv_xen_unittest.py
+++ b/test/py/ganeti.hypervisor.hv_xen_unittest.py
@@ -271,7 +271,7 @@ class TestGetConfigFileDiskData(unittest.TestCase):
for offset in [0, 1, 10]:
disks = [(objects.Disk(dev_type=constants.DT_PLAIN),
"/tmp/disk/%s" % idx,
- None)
+ NotImplemented)
for idx in range(len(hv_xen._DISK_LETTERS) + offset)]
if offset == 0:
@@ -292,10 +292,10 @@ class TestGetConfigFileDiskData(unittest.TestCase):
disks = [
(objects.Disk(dev_type=constants.DT_PLAIN, mode=constants.DISK_RDWR),
"/tmp/diskFirst",
- None),
+ NotImplemented),
(objects.Disk(dev_type=constants.DT_PLAIN, mode=constants.DISK_RDONLY),
"/tmp/diskLast",
- None),
+ NotImplemented),
]
result = hv_xen._GetConfigFileDiskData(disks, "hd")
@@ -309,19 +309,19 @@ class TestGetConfigFileDiskData(unittest.TestCase):
(objects.Disk(dev_type=constants.DT_FILE, mode=constants.DISK_RDWR,
logical_id=[constants.FD_LOOP]),
"/tmp/diskFirst",
- None),
+ NotImplemented),
(objects.Disk(dev_type=constants.DT_FILE, mode=constants.DISK_RDONLY,
logical_id=[constants.FD_BLKTAP]),
"/tmp/diskTwo",
- None),
+ NotImplemented),
(objects.Disk(dev_type=constants.DT_FILE, mode=constants.DISK_RDWR,
logical_id=[constants.FD_LOOP]),
"/tmp/diskThree",
- None),
+ NotImplemented),
(objects.Disk(dev_type=constants.DT_FILE, mode=constants.DISK_RDWR,
logical_id=[constants.FD_BLKTAP]),
"/tmp/diskLast",
- None),
+ NotImplemented),
]
result = hv_xen._GetConfigFileDiskData(disks, "sd")
@@ -337,7 +337,7 @@ class TestGetConfigFileDiskData(unittest.TestCase):
(objects.Disk(dev_type=constants.DT_FILE, mode=constants.DISK_RDWR,
logical_id=["#unknown#"]),
"/tmp/diskinvalid",
- None),
+ NotImplemented),
]
self.assertRaises(KeyError, hv_xen._GetConfigFileDiskData, disks, "sd")
@@ -689,17 +689,16 @@ class _TestXenHypervisor(object):
disks = [
(objects.Disk(dev_type=constants.DT_PLAIN, mode=constants.DISK_RDWR),
utils.PathJoin(self.tmpdir, "disk0"),
- None),
+ NotImplemented),
(objects.Disk(dev_type=constants.DT_PLAIN, mode=constants.DISK_RDONLY),
utils.PathJoin(self.tmpdir, "disk1"),
- None),
+ NotImplemented),
]
inst = objects.Instance(name="server01.example.com",
hvparams=hvp, beparams=bep,
osparams={}, nics=[], os="deb1",
disks=map(compat.fst, disks))
-
inst.UpgradeConfig()
return (inst, disks)
On Mon, Sep 30, 2013 at 10:54 AM, Thomas Thrainer <[email protected]> wrote:
>
>
>
> On Fri, Sep 27, 2013 at 3:35 PM, 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 | 9 +++++---
>> lib/hypervisor/hv_xen.py | 2 +-
>> lib/storage/__init__.py | 2 ++
>> test/py/ganeti.hypervisor.hv_xen_unittest.py | 32
>> ++++++++++++++++++--------
>> 5 files changed, 32 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..309a177 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,11 @@ 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:
>> + dev_path = device.GetUserspaceAccessUri(constants.HT_KVM)
>
>
> NIT: the dev_path variable does not hold a device path at this point.
> Consider introducing another variable (drive_uri, file_param or something)
> which you can use instead.
>
>>
>> +
>> + drive_val = "file=%s,format=raw%s%s%s" % (dev_path, 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/lib/storage/__init__.py b/lib/storage/__init__.py
>> index 9b9e38c..efe9600 100644
>> --- a/lib/storage/__init__.py
>> +++ b/lib/storage/__init__.py
>> @@ -22,3 +22,5 @@
>> """Block device abstraction
>>
>> """
>> +
>> +from ganeti.storage import base
>> diff --git a/test/py/ganeti.hypervisor.hv_xen_unittest.py
>> b/test/py/ganeti.hypervisor.hv_xen_unittest.py
>> index 4a3270b..75f22dc 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,
>> + None)
>
>
> We usually use NotImplemented for values which are not required in a test
> instead of None (as None could be a valid value too). Same for all
> occurrences below.
>
>>
>> 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",
>> + None),
>> (objects.Disk(dev_type=constants.DT_PLAIN,
>> mode=constants.DISK_RDONLY),
>> - "/tmp/diskLast"),
>> + "/tmp/diskLast",
>> + None),
>> ]
>>
>> 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",
>> + None),
>> (objects.Disk(dev_type=constants.DT_FILE,
>> mode=constants.DISK_RDONLY,
>> logical_id=[constants.FD_BLKTAP]),
>> - "/tmp/diskTwo"),
>> + "/tmp/diskTwo",
>> + None),
>> (objects.Disk(dev_type=constants.DT_FILE, mode=constants.DISK_RDWR,
>> logical_id=[constants.FD_LOOP]),
>> - "/tmp/diskThree"),
>> + "/tmp/diskThree",
>> + None),
>> (objects.Disk(dev_type=constants.DT_FILE, mode=constants.DISK_RDWR,
>> logical_id=[constants.FD_BLKTAP]),
>> - "/tmp/diskLast"),
>> + "/tmp/diskLast",
>> + None),
>> ]
>>
>> 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",
>> + None),
>> ]
>>
>> self.assertRaises(KeyError, hv_xen._GetConfigFileDiskData, disks,
>> "sd")
>> @@ -679,15 +688,18 @@ 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"),
>> + None),
>> (objects.Disk(dev_type=constants.DT_PLAIN,
>> mode=constants.DISK_RDONLY),
>> - utils.PathJoin(self.tmpdir, "disk1")),
>> + utils.PathJoin(self.tmpdir, "disk1"),
>> + None),
>> ]
>>
>> inst = objects.Instance(name="server01.example.com",
>> hvparams=hvp, beparams=bep,
>> osparams={}, nics=[], os="deb1",
>> disks=map(compat.fst, disks))
>> +
>
>
> Spurious new newline?
>
>>
>> inst.UpgradeConfig()
>>
>> return (inst, disks)
>> --
>> 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
--
Raffa Santi
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
