Merge lp:~wesley-wiedenmeier/curtin/1598310 into lp:~curtin-dev/curtin/trunk

Proposed by Wesley Wiedenmeier
Status: Merged
Merged at revision: 405
Proposed branch: lp:~wesley-wiedenmeier/curtin/1598310
Merge into: lp:~curtin-dev/curtin/trunk
Diff against target: 73 lines (+44/-1)
2 files modified
curtin/block/__init__.py (+12/-1)
tests/unittests/test_block.py (+32/-0)
To merge this branch: bzr merge lp:~wesley-wiedenmeier/curtin/1598310
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Ryan Harper Approve
Review via email: mp+298980@code.launchpad.net

Description of the change

Fix (LP: 1598310), in block.get_blockdev_sector_size, do not assume that only 1 result will be returned by _lsblock() as it will return multiple results whenever there are other devices under the device it has been called on (for example, disks with partitions).

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Ryan Harper (raharper) wrote :

Good catch. storage branch turning these out now?

review: Approve
Revision history for this message
Wesley Wiedenmeier (wesley-wiedenmeier) wrote :

Yeah, this was causing a failure in one of the whole disk format tests.

402. By Wesley Wiedenmeier

In block.get_blockdev_sector_size, when falling back to first entry in lsblk
return dict, cast dict_keys object to list before indexing

403. By Wesley Wiedenmeier

Add test for fallback lsblk entry selection in block.get_blockdev_sector_size

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'curtin/block/__init__.py'
2--- curtin/block/__init__.py 2016-06-10 14:49:22 +0000
3+++ curtin/block/__init__.py 2016-07-11 19:21:49 +0000
4@@ -423,7 +423,18 @@
5 """
6 info = _lsblock([devpath])
7 LOG.debug('get_blockdev_sector_size: info:\n%s' % util.json_dumps(info))
8- [parent] = info
9+ # (LP: 1598310) The call to _lsblock() may return multiple results.
10+ # If it does, then search for a result with the correct device path.
11+ # If no such device is found among the results, then fall back to previous
12+ # behavior, which was taking the first of the results
13+ assert len(info) > 0
14+ for (k, v) in info.items():
15+ if v.get('device_path') == devpath:
16+ parent = k
17+ break
18+ else:
19+ parent = list(info.keys())[0]
20+
21 return (int(info[parent]['LOG-SEC']), int(info[parent]['PHY-SEC']))
22
23
24
25=== modified file 'tests/unittests/test_block.py'
26--- tests/unittests/test_block.py 2016-04-04 20:12:01 +0000
27+++ tests/unittests/test_block.py 2016-07-11 19:21:49 +0000
28@@ -4,6 +4,8 @@
29 import tempfile
30 import shutil
31
32+from collections import OrderedDict
33+
34 from curtin import util
35 from curtin import block
36
37@@ -39,6 +41,36 @@
38 self.assertEqual(sorted(mountpoints),
39 sorted(["/mnt", "/sys"]))
40
41+ @mock.patch('curtin.block._lsblock')
42+ def test_get_blockdev_sector_size(self, mock_lsblk):
43+ mock_lsblk.return_value = {
44+ 'sda': {'LOG-SEC': '512', 'PHY-SEC': '4096',
45+ 'device_path': '/dev/sda'},
46+ 'sda1': {'LOG-SEC': '512', 'PHY-SEC': '4096',
47+ 'device_path': '/dev/sda1'},
48+ 'dm-0': {'LOG-SEC': '512', 'PHY-SEC': '512',
49+ 'device_path': '/dev/dm-0'},
50+ }
51+ for (devpath, expected) in [('/dev/sda', (512, 4096)),
52+ ('/dev/sda1', (512, 4096)),
53+ ('/dev/dm-0', (512, 512))]:
54+ res = block.get_blockdev_sector_size(devpath)
55+ mock_lsblk.assert_called_with([devpath])
56+ self.assertEqual(res, expected)
57+
58+ # test that fallback works and gives right return
59+ mock_lsblk.return_value = OrderedDict()
60+ mock_lsblk.return_value.update({
61+ 'vda': {'LOG-SEC': '4096', 'PHY-SEC': '4096',
62+ 'device_path': '/dev/vda'},
63+ })
64+ mock_lsblk.return_value.update({
65+ 'vda1': {'LOG-SEC': '512', 'PHY-SEC': '512',
66+ 'device_path': '/dev/vda1'},
67+ })
68+ res = block.get_blockdev_sector_size('/dev/vda2')
69+ self.assertEqual(res, (4096, 4096))
70+
71 @mock.patch("curtin.block.os.path.realpath")
72 @mock.patch("curtin.block.os.path.exists")
73 @mock.patch("curtin.block.os.listdir")

Subscribers

People subscribed via source and target branches