Merge ~ltrager/maas:lp1695229 into maas:master

[Lee Trager]

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.

[Andres Rodriguez (andreserl)]

review: Needs Information

[Andres Rodriguez (andreserl)]

review: Needs Information

[Andres Rodriguez (andreserl)]

What is the TIMEOUT current? What's the value for it ?

review: Needs Information

[Andres Rodriguez (andreserl)]

And is this set for this particular test or is it a global variable for all tests?

[Lee Trager (ltrager)]

Just that one command.

[Andres Rodriguez (andreserl)]

review: Approve

diff --git a/src/metadataserver/builtin_scripts/badblocks.py b/src/metadataserver/builtin_scripts/badblocks.py
index eba78ed..46e9111 100644
--- a/src/metadataserver/builtin_scripts/badblocks.py
+++ b/src/metadataserver/builtin_scripts/badblocks.py
@@ -48,10 +48,17 @@ class RunBadBlocks(Thread):
         self.returncode = None
 
     def run(self):
+        blocksize = check_output(
+            ['sudo', '-n', 'blockdev', '--getbsz', self.drive['PATH']],
+            timeout=TIMEOUT, stderr=DEVNULL).strip()
         if self.destructive:
-            cmd = ['sudo', '-n', 'badblocks', '-v', '-w', self.drive['PATH']]
+            cmd = [
+                'sudo', '-n', 'badblocks', '-b', blocksize, '-v', '-w',
+                self.drive['PATH']]
         else:
-            cmd = ['sudo', '-n', 'badblocks', '-v', '-n', self.drive['PATH']]
+            cmd = [
+                'sudo', '-n', 'badblocks', '-b', blocksize, '-v', '-n',
+                self.drive['PATH']]
         # Run badblocks and capture its output. Once all threads have completed
         # output the results serially so output is proerly grouped.
         with Popen(cmd, stdout=PIPE, stderr=STDOUT) as proc:
diff --git a/src/metadataserver/builtin_scripts/tests/test_badblocks.py b/src/metadataserver/builtin_scripts/tests/test_badblocks.py
index 6e08dde..6b3c922 100644
--- a/src/metadataserver/builtin_scripts/tests/test_badblocks.py
+++ b/src/metadataserver/builtin_scripts/tests/test_badblocks.py
@@ -127,6 +127,7 @@ class TestSmartCTL(MAASTestCase):
         drive = self.make_drive(with_path=True)
         run_bad_blocks = badblocks.RunBadBlocks(drive, True)
         output = factory.make_string()
+        self.patch(badblocks, 'check_output').return_value = '4096'
         mock_popen = self.patch(badblocks, 'Popen')
         mock_popen.return_value = Popen(['echo', '-n', output], stdout=PIPE)
 
@@ -137,13 +138,16 @@ class TestSmartCTL(MAASTestCase):
         self.assertThat(
             mock_popen,
             MockCalledOnceWith(
-                ['sudo', '-n', 'badblocks', '-v', '-w', drive['PATH']],
-                stdout=PIPE, stderr=STDOUT))
+                [
+                    'sudo', '-n', 'badblocks', '-b', '4096', '-v', '-w',
+                    drive['PATH'],
+                ], stdout=PIPE, stderr=STDOUT))
 
     def test_run_nondestructive(self):
         drive = self.make_drive(with_path=True)
         run_bad_blocks = badblocks.RunBadBlocks(drive, False)
         output = factory.make_string()
+        self.patch(badblocks, 'check_output').return_value = '4096'
         mock_popen = self.patch(badblocks, 'Popen')
         mock_popen.return_value = Popen(['echo', '-n', output], stdout=PIPE)
 
@@ -154,8 +158,10 @@ class TestSmartCTL(MAASTestCase):
         self.assertThat(
             mock_popen,
             MockCalledOnceWith(
-                ['sudo', '-n', 'badblocks', '-v', '-n', drive['PATH']],
-                stdout=PIPE, stderr=STDOUT))
+                [
+                    'sudo', '-n', 'badblocks', '-b', '4096', '-v', '-n',
+                    drive['PATH'],
+                ], stdout=PIPE, stderr=STDOUT))
 
     def test_run_badblocks(self):
         drive = {
@@ -166,6 +172,7 @@ class TestSmartCTL(MAASTestCase):
         }
         self.patch(badblocks, 'list_drives').return_value = [drive]
         output = factory.make_string()
+        self.patch(badblocks, 'check_output').return_value = '4096'
         mock_popen = self.patch(badblocks, 'Popen')
         mock_popen.return_value = Popen(['echo', '-n', output], stdout=PIPE)
         mock_print = self.patch(badblocks, 'print')
@@ -192,6 +199,7 @@ class TestSmartCTL(MAASTestCase):
         }
         self.patch(badblocks, 'list_drives').return_value = [drive]
         output = factory.make_string()
+        self.patch(badblocks, 'check_output').return_value = '4096'
         mock_popen = self.patch(badblocks, 'Popen')
         mock_popen.return_value = Popen(
             'echo -n %s; exit 1' % output, stdout=PIPE, shell=True)