Merge ~mwhudson/curtin:fix-DasdPartitionTable into curtin:master

Proposed by Michael Hudson-Doyle
Status: Merged
Approved by: Michael Hudson-Doyle
Approved revision: 70954686adb559e8bdccbf68cc4a44e19ec604da
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~mwhudson/curtin:fix-DasdPartitionTable
Merge into: curtin:master
Diff against target: 149 lines (+74/-28)
3 files modified
curtin/block/dasd.py (+32/-27)
tests/unittests/test_block_dasd.py (+41/-0)
tests/unittests/test_commands_block_meta.py (+1/-1)
Reviewer Review Type Date Requested Status
Paride Legovini Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+394985@code.launchpad.net

Commit message

fix construction of DasdPartitionTable from fdasd output

DasdPartition had an extra argument in its constructor that should not
have been there. Add some tests.

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
f25277b... by Michael Hudson-Doyle

fix tests

Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
7095468... by Michael Hudson-Doyle

py27 compat

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

LGTM. Pity that fdasd doesn't have a machine-readable output format for the table. I had a look for alternative ways but found none. I filed [1], so maybe one day!

[1] https://github.com/ibm-s390-tools/s390-tools/issues/104

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/curtin/block/dasd.py b/curtin/block/dasd.py
2index 7450300..21ce7be 100644
3--- a/curtin/block/dasd.py
4+++ b/curtin/block/dasd.py
5@@ -10,7 +10,7 @@ Dasdvalue = collections.namedtuple('Dasdvalue', ['hex', 'dec', 'txt'])
6
7
8 class DasdPartition:
9- def __init__(self, table, device, start, end, length, id, system):
10+ def __init__(self, device, start, end, length, id, system):
11 self.device = device
12 self.start = int(start)
13 self.end = int(end)
14@@ -34,6 +34,35 @@ class DasdPartitionTable:
15 return ((size_in_bytes - 1) // self.bytes_per_track) + 1
16
17 @classmethod
18+ def from_fdasd_output(cls, devname, output):
19+ line_iter = iter(output.splitlines())
20+ for line in line_iter:
21+ if line.startswith("Disk"):
22+ break
23+ kw = {'devname': devname}
24+ label_to_attr = {
25+ 'blocks per track': 'blocks_per_track',
26+ 'bytes per block': 'bytes_per_block'
27+ }
28+ for line in line_iter:
29+ if '--- tracks ---' in line:
30+ break
31+ if ':' in line:
32+ label, value = line.split(':', 1)
33+ label = label.strip(' .')
34+ value = value.strip()
35+ if label in label_to_attr:
36+ kw[label_to_attr[label]] = int(value)
37+ table = cls(**kw)
38+ for line in line_iter:
39+ if line.startswith('exiting'):
40+ break
41+ vals = line.split(None, 5)
42+ if vals[0].startswith('/dev/'):
43+ table.partitions.append(DasdPartition(*vals))
44+ return table
45+
46+ @classmethod
47 def from_fdasd(cls, devname):
48 """Use fdasd to construct a DasdPartitionTable.
49
50@@ -61,32 +90,8 @@ class DasdPartitionTable:
51 """
52 cmd = ['fdasd', '--table', devname]
53 out, _err = util.subp(cmd, capture=True)
54- line_iter = iter(out.splitlines())
55- for line in line_iter:
56- if line.startswith("Disk"):
57- break
58- kw = {'devname': devname}
59- label_to_attr = {
60- 'blocks per track': 'blocks_per_track',
61- 'bytes per block': 'bytes_per_block'
62- }
63- for line in line_iter:
64- if '--- tracks ---' in line:
65- break
66- if ':' in line:
67- label, value = line.split(':', 1)
68- label = label.strip(' .')
69- value = value.strip()
70- if label in label_to_attr:
71- kw[label_to_attr[label]] = int(value)
72- table = cls(**kw)
73- for line in line_iter:
74- if line.startswith('exiting'):
75- break
76- vals = line.split(maxsplit=5)
77- if vals[0].startswith('/dev/'):
78- table.partitions.append(DasdPartition(*vals))
79- return table
80+ LOG.debug("from_fdasd output:\n---\n%s\n---\n", out)
81+ return cls.from_fdasd_output(devname, out)
82
83
84 def dasdinfo(device_id):
85diff --git a/tests/unittests/test_block_dasd.py b/tests/unittests/test_block_dasd.py
86index dfd62d3..450ca16 100644
87--- a/tests/unittests/test_block_dasd.py
88+++ b/tests/unittests/test_block_dasd.py
89@@ -15,6 +15,47 @@ def random_device_id():
90 random.randint(1, 0x10000 - 1))
91
92
93+FDASD_OUTPUT = '''
94+reading volume label ..: VOL1
95+reading vtoc ..........: ok
96+
97+
98+Disk /dev/dasda:
99+ cylinders ............: 10016
100+ tracks per cylinder ..: 15
101+ blocks per track .....: 12
102+ bytes per block ......: 4096
103+ volume label .........: VOL1
104+ volume serial ........: 0X0200
105+ max partitions .......: 3
106+
107+ ------------------------------- tracks -------------------------------
108+ Device start end length Id System
109+ /dev/dasda1 2 21847 21846 1 Linux native
110+ 21848 150239 128392 unused
111+exiting...
112+ '''
113+
114+
115+class TestDasdPartitionTable(CiTestCase):
116+
117+ def test_from_dasd_output(self):
118+ devname = self.random_string()
119+ dasd_pt = dasd.DasdPartitionTable.from_fdasd_output(
120+ devname, FDASD_OUTPUT)
121+ self.assertEqual(dasd_pt.devname, devname)
122+ self.assertEqual(dasd_pt.blocks_per_track, 12)
123+ self.assertEqual(dasd_pt.bytes_per_block, 4096)
124+ self.assertEqual(len(dasd_pt.partitions), 1)
125+ part = dasd_pt.partitions[0]
126+ self.assertEqual(part.device, '/dev/dasda1')
127+ self.assertEqual(part.start, 2)
128+ self.assertEqual(part.end, 21847)
129+ self.assertEqual(part.length, 21846)
130+ self.assertEqual(part.id, '1')
131+ self.assertEqual(part.system, 'Linux native')
132+
133+
134 class TestDasdValidDeviceId(CiTestCase):
135
136 nonhex = [letter for letter in string.ascii_lowercase
137diff --git a/tests/unittests/test_commands_block_meta.py b/tests/unittests/test_commands_block_meta.py
138index a5129a5..499f2d1 100644
139--- a/tests/unittests/test_commands_block_meta.py
140+++ b/tests/unittests/test_commands_block_meta.py
141@@ -2482,7 +2482,7 @@ class TestPartitionVerifyFdasd(CiTestCase):
142 tracks_needed = fake_pt.tracks_needed(
143 util.human2bytes(self.info['size']))
144 fake_pt.partitions = [
145- dasd.DasdPartition(fake_pt, '', 0, 0, tracks_needed, '1', 'Linux'),
146+ dasd.DasdPartition('', 0, 0, tracks_needed, '1', 'Linux'),
147 ]
148 self.m_from_fdasd.side_effect = iter([fake_pt])
149 block_meta.partition_verify_fdasd(self.devpath, 1, self.info)

Subscribers

People subscribed via source and target branches