MAAS

Merge ~ltrager/maas:lp1695229_2.2 into maas:2.2

  1. Git
  2. lp:~ltrager/maas
  3. lp1695229_2.2
  4. Merge into 2.2
Proposed by Lee Trager
Status: Merged
Approved by: Lee Trager
Approved revision: a2fc6337e0c83fa1b98922ec656e53147756b47c
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~ltrager/maas:lp1695229_2.2
Merge into: maas:2.2
Diff against target: 84 lines (+21/-6)
2 files modified
src/metadataserver/builtin_scripts/badblocks.py (+9/-2)
src/metadataserver/builtin_scripts/tests/test_badblocks.py (+12/-4)
Reviewer Review Type Date Requested Status
Lee Trager (community) Approve
Review via email: mp+326704@code.launchpad.net

Commit message

Backport from master: Run badblocks using the physical blocksize of the disk.

By default badblocks always runs using a blocksize of 1024 and uses a 32bit int for the number of blocks it can test. For large disks the number of blocks exceeds the 32bit int limit and causes badblocks to fail. Many large uses use a larger physical blocksize which avoids hitting the 32bit int limit which helps avoid hitting LP:1695229.

To post a comment you must log in.
Revision history for this message
Lee Trager (ltrager) wrote :

Approved in https://code.launchpad.net/~ltrager/maas/+git/maas/+merge/326204

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/metadataserver/builtin_scripts/badblocks.py b/src/metadataserver/builtin_scripts/badblocks.py
2index eba78ed..46e9111 100644
3--- a/src/metadataserver/builtin_scripts/badblocks.py
4+++ b/src/metadataserver/builtin_scripts/badblocks.py
5@@ -48,10 +48,17 @@ class RunBadBlocks(Thread):
6 self.returncode = None
7
8 def run(self):
9+ blocksize = check_output(
10+ ['sudo', '-n', 'blockdev', '--getbsz', self.drive['PATH']],
11+ timeout=TIMEOUT, stderr=DEVNULL).strip()
12 if self.destructive:
13- cmd = ['sudo', '-n', 'badblocks', '-v', '-w', self.drive['PATH']]
14+ cmd = [
15+ 'sudo', '-n', 'badblocks', '-b', blocksize, '-v', '-w',
16+ self.drive['PATH']]
17 else:
18- cmd = ['sudo', '-n', 'badblocks', '-v', '-n', self.drive['PATH']]
19+ cmd = [
20+ 'sudo', '-n', 'badblocks', '-b', blocksize, '-v', '-n',
21+ self.drive['PATH']]
22 # Run badblocks and capture its output. Once all threads have completed
23 # output the results serially so output is proerly grouped.
24 with Popen(cmd, stdout=PIPE, stderr=STDOUT) as proc:
25diff --git a/src/metadataserver/builtin_scripts/tests/test_badblocks.py b/src/metadataserver/builtin_scripts/tests/test_badblocks.py
26index 6e08dde..6b3c922 100644
27--- a/src/metadataserver/builtin_scripts/tests/test_badblocks.py
28+++ b/src/metadataserver/builtin_scripts/tests/test_badblocks.py
29@@ -127,6 +127,7 @@ class TestSmartCTL(MAASTestCase):
30 drive = self.make_drive(with_path=True)
31 run_bad_blocks = badblocks.RunBadBlocks(drive, True)
32 output = factory.make_string()
33+ self.patch(badblocks, 'check_output').return_value = '4096'
34 mock_popen = self.patch(badblocks, 'Popen')
35 mock_popen.return_value = Popen(['echo', '-n', output], stdout=PIPE)
36
37@@ -137,13 +138,16 @@ class TestSmartCTL(MAASTestCase):
38 self.assertThat(
39 mock_popen,
40 MockCalledOnceWith(
41- ['sudo', '-n', 'badblocks', '-v', '-w', drive['PATH']],
42- stdout=PIPE, stderr=STDOUT))
43+ [
44+ 'sudo', '-n', 'badblocks', '-b', '4096', '-v', '-w',
45+ drive['PATH'],
46+ ], stdout=PIPE, stderr=STDOUT))
47
48 def test_run_nondestructive(self):
49 drive = self.make_drive(with_path=True)
50 run_bad_blocks = badblocks.RunBadBlocks(drive, False)
51 output = factory.make_string()
52+ self.patch(badblocks, 'check_output').return_value = '4096'
53 mock_popen = self.patch(badblocks, 'Popen')
54 mock_popen.return_value = Popen(['echo', '-n', output], stdout=PIPE)
55
56@@ -154,8 +158,10 @@ class TestSmartCTL(MAASTestCase):
57 self.assertThat(
58 mock_popen,
59 MockCalledOnceWith(
60- ['sudo', '-n', 'badblocks', '-v', '-n', drive['PATH']],
61- stdout=PIPE, stderr=STDOUT))
62+ [
63+ 'sudo', '-n', 'badblocks', '-b', '4096', '-v', '-n',
64+ drive['PATH'],
65+ ], stdout=PIPE, stderr=STDOUT))
66
67 def test_run_badblocks(self):
68 drive = {
69@@ -166,6 +172,7 @@ class TestSmartCTL(MAASTestCase):
70 }
71 self.patch(badblocks, 'list_drives').return_value = [drive]
72 output = factory.make_string()
73+ self.patch(badblocks, 'check_output').return_value = '4096'
74 mock_popen = self.patch(badblocks, 'Popen')
75 mock_popen.return_value = Popen(['echo', '-n', output], stdout=PIPE)
76 mock_print = self.patch(badblocks, 'print')
77@@ -192,6 +199,7 @@ class TestSmartCTL(MAASTestCase):
78 }
79 self.patch(badblocks, 'list_drives').return_value = [drive]
80 output = factory.make_string()
81+ self.patch(badblocks, 'check_output').return_value = '4096'
82 mock_popen = self.patch(badblocks, 'Popen')
83 mock_popen.return_value = Popen(
84 'echo -n %s; exit 1' % output, stdout=PIPE, shell=True)

Subscribers

People subscribed via source and target branches