Merge pull request #75 from raharper/fix/read-sys-block-size

probert/utils: Don't use logical_sector_size for calculations

probert/lvm.py

@@ -19,7 +19,7 @@
 import pyudev
 import subprocess
 
-from probert.utils import read_sys_block_size
+from probert.utils import read_sys_block_size_bytes
 
 log = logging.getLogger('probert.lvm')
 
@@ -122,7 +122,8 @@ def extract_lvm_partition(probe_data):
         lv_id, {'fullname': lv_id,
                 'name': probe_data['DM_LV_NAME'],
                 'volgroup': probe_data['DM_VG_NAME'],
-                'size': "%sB" % read_sys_block_size(probe_data['DEVNAME'])})
+                'size': "%sB" % read_sys_block_size_bytes(
+                    probe_data['DEVNAME'])})
 
 
 def extract_lvm_volgroup(vg_name, report_data):

probert/raid.py

@@ -17,7 +17,7 @@
 import pyudev
 import subprocess
 
-from probert.utils import (read_sys_block_size,
+from probert.utils import (read_sys_block_size_bytes,
                            udev_get_attributes)
 
 log = logging.getLogger('probert.raid')
@@ -115,7 +115,7 @@ def probe(context=None, report=False):
         if 'MD_NAME' in device and device.get('DEVTYPE') == 'disk':
             devname = device['DEVNAME']
             attrs = udev_get_attributes(device)
-            attrs['size'] = str(read_sys_block_size(devname))
+            attrs['size'] = str(read_sys_block_size_bytes(devname))
             devices, spares = get_mdadm_array_members(devname, device)
             cfg = dict(device)
             cfg.update({'raidlevel': device['MD_LEVEL'],

probert/storage.py

@@ -18,7 +18,7 @@
 import pyudev
 import subprocess
 
-from probert.utils import udev_get_attributes, read_sys_block_size
+from probert.utils import udev_get_attributes, read_sys_block_size_bytes
 from probert import (bcache, dmcrypt, filesystem, lvm, mount, multipath,
                      raid, zfs)
 
@@ -117,7 +117,7 @@ def _extract_partition_table(devname):
             # update the size attr as it may only be the number
             # of blocks rather than size in bytes.
             attrs['size'] = \
-                str(read_sys_block_size(device['DEVNAME']))
+                str(read_sys_block_size_bytes(device['DEVNAME']))
             blockdev[device['DEVNAME']] = dict(device)
             blockdev[device['DEVNAME']].update({'attrs': attrs})
             # include partition table info if present

probert/tests/helpers.py

@@ -13,10 +13,38 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import contextlib
+import imp
+import importlib
+import mock
 import random
 import string
 
 
+def builtin_module_name():
+    options = ('builtins', '__builtin__')
+    for name in options:
+        try:
+            imp.find_module(name)
+        except ImportError:
+            continue
+        else:
+            print('importing and returning: %s' % name)
+            importlib.import_module(name)
+            return name
+
+
+@contextlib.contextmanager
+def simple_mocked_open(content=None):
+    if not content:
+        content = ''
+    m_open = mock.mock_open(read_data=content)
+    mod_name = builtin_module_name()
+    m_patch = '{}.open'.format(mod_name)
+    with mock.patch(m_patch, m_open, create=True):
+        yield m_open
+
+
 def random_string(length=8):
     """ return a random lowercase string with default length of 8"""
     return ''.join(

probert/tests/test_lvm.py

@@ -244,7 +244,7 @@ def test_extract_lvm_volgroup_no_size_set_to_zero_bytes(self, m_run):
                      'size': '0B'}),
             lvm.extract_lvm_volgroup('vg0', input_data))
 
-    @mock.patch('probert.lvm.read_sys_block_size')
+    @mock.patch('probert.lvm.read_sys_block_size_bytes')
     def test_extract_lvm_partition(self, m_size, m_run):
         size = 100000000
         m_size.return_value = size
@@ -261,7 +261,7 @@ def test_extract_lvm_partition(self, m_size, m_run):
             lvm.extract_lvm_partition(input_data))
         m_size.assert_called_with('/dev/dm-2')
 
-    @mock.patch('probert.lvm.read_sys_block_size')
+    @mock.patch('probert.lvm.read_sys_block_size_bytes')
     @mock.patch('probert.lvm.activate_volgroups')
     @mock.patch('probert.lvm.lvm_scan')
     @mock.patch('probert.lvm.pyudev.Context.list_devices')
@@ -294,7 +294,7 @@ def test_probe(self, m_vgs, m_pyudev, m_scan, m_activate, m_size, m_run):
         }
         self.assertEqual(expected_result, lvm.probe())
 
-    @mock.patch('probert.lvm.read_sys_block_size')
+    @mock.patch('probert.lvm.read_sys_block_size_bytes')
     @mock.patch('probert.lvm.activate_volgroups')
     @mock.patch('probert.lvm.lvm_scan')
     @mock.patch('probert.lvm.pyudev.Context.list_devices')

probert/tests/test_utils.py

@@ -1,6 +1,8 @@
 import testtools
+from mock import call
 
 from probert import utils
+from probert.tests.helpers import random_string, simple_mocked_open
 
 
 class ProbertTestUtils(testtools.TestCase):
@@ -35,3 +37,23 @@ def test_utils_dict_merge_dicts(self):
         }
         test_result = utils.dict_merge(r1, r2)
         self.assertEqual(sorted(combined), sorted(test_result))
+
+    def test_utils_read_sys_block_size_bytes(self):
+        devname = random_string()
+        expected_fname = '/sys/class/block/%s/size' % devname
+        expected_bytes = 10737418240
+        content = '20971520'
+        with simple_mocked_open(content=content) as m_open:
+            result = utils.read_sys_block_size_bytes(devname)
+            self.assertEqual(expected_bytes, result)
+            self.assertEqual([call(expected_fname)], m_open.call_args_list)
+
+    def test_utils_read_sys_block_size_bytes_strips_value(self):
+        devname = random_string()
+        expected_fname = '/sys/class/block/%s/size' % devname
+        expected_bytes = 10737418240
+        content = ' 20971520 \n '
+        with simple_mocked_open(content=content) as m_open:
+            result = utils.read_sys_block_size_bytes(devname)
+            self.assertEqual(expected_bytes, result)
+            self.assertEqual([call(expected_fname)], m_open.call_args_list)

probert/utils.py

@@ -22,6 +22,10 @@
     "bridge_hello", "bridge_maxage", "bridge_maxwait", "bridge_stp",
 ]
 
+# sysfs size attribute is always in 512-byte units
+# https://github.com/torvalds/linux/blob/6f0d349d922ba44e4348a17a78ea51b7135965b1/include/linux/types.h#L125
+SECTOR_SIZE_BYTES = 512
+
 
 # from juju-deployer utils.relation_merge
 def dict_merge(onto, source):
@@ -225,21 +229,12 @@ def parse_etc_network_interfaces(ifaces, contents, path):
             ifaces[iface]['auto'] = False
 
 
-def read_sys_block_size(device):
+def read_sys_block_size_bytes(device):
+    """ /sys/class/block/<device>/size and return integer value in bytes"""
     device_dir = os.path.join('/sys/class/block', os.path.basename(device))
     blockdev_size = os.path.join(device_dir, 'size')
     with open(blockdev_size) as d:
-        size = int(d.read().strip())
-
-    logsize_base = device_dir
-    if not os.path.exists(os.path.join(device_dir, 'queue')):
-        parent_dev = os.path.basename(re.split(r'[\d+]', device)[0])
-        logsize_base = os.path.join('/sys/class/block', parent_dev)
-
-    logical_size = os.path.join(logsize_base, 'queue', 'logical_block_size')
-    if os.path.exists(logical_size):
-        with open(logical_size) as s:
-            size *= int(s.read().strip())
+        size = int(d.read().strip()) * SECTOR_SIZE_BYTES
 
     return size