From 11bbc450c33703dba8b78c56556637e49c420376 Mon Sep 17 00:00:00 2001 From: Yuriy Nesenenko Date: Fri, 4 Sep 2015 18:25:44 +0300 Subject: [PATCH] Small optimization in Block Device driver Method _devices_sizes() in Block Device driver invokes N times _get_device_size(device) to get devices sizes. It can be done by executing one command: blockdev --getsize64 /dev/loop1 /dev/loop2. In such a way we take the devices' sizes in bytes and convert them to Megabytes. Change-Id: I0aba86239b4c580a0b12202a8f7fc863dab6fecd --- cinder/tests/unit/test_block_device.py | 48 +++++++++++++++----------- cinder/volume/drivers/block_device.py | 42 +++++++++++++--------- 2 files changed, 52 insertions(+), 38 deletions(-) diff --git a/cinder/tests/unit/test_block_device.py b/cinder/tests/unit/test_block_device.py index 61fd66272..acbeb7964 100644 --- a/cinder/tests/unit/test_block_device.py +++ b/cinder/tests/unit/test_block_device.py @@ -101,14 +101,15 @@ class TestBlockDeviceDriver(cinder.test.TestCase): with mock.patch.object(self.drv, 'local_path', return_value='/dev/loop1') as lp_mocked: - with mock.patch.object(self.drv, '_get_device_size', - return_value=1024) as gds_mocked: + with mock.patch.object(self.drv, '_get_devices_sizes', + return_value={'/dev/loop1': 1}) as \ + gds_mocked: volutils.clear_volume(gds_mocked, lp_mocked) self.drv.delete_volume(TEST_VOLUME1) lp_mocked.assert_called_once_with(TEST_VOLUME1) - gds_mocked.assert_called_once_with('/dev/loop1') + gds_mocked.assert_called_once_with(['/dev/loop1']) self.assertTrue(_exists.called) self.assertTrue(_clear_volume.called) @@ -121,8 +122,7 @@ class TestBlockDeviceDriver(cinder.test.TestCase): lp_mocked.assert_called_once_with(TEST_VOLUME2) def test_create_volume(self): - TEST_VOLUME = {'size': 1, - 'name': 'vol1'} + TEST_VOLUME = {'size': 1, 'name': 'vol1'} with mock.patch.object(self.drv, 'find_appropriate_size_device', return_value='dev_path') as fasd_mocked: @@ -162,12 +162,13 @@ class TestBlockDeviceDriver(cinder.test.TestCase): with mock.patch.object(self.drv, 'find_appropriate_size_device', return_value='/dev/loop2') as fasd_mocked: - with mock.patch.object(self.drv, '_get_device_size', - return_value=1) as gds_mocked: + with mock.patch.object(self.drv, '_get_devices_sizes', + return_value={'/dev/loop2': 2}) as \ + gds_mocked: with mock.patch.object(self.drv, 'local_path', return_value='/dev/loop1') as \ lp_mocked: - volutils.copy_volume('/dev/loop1', fasd_mocked, 2048, + volutils.copy_volume('/dev/loop1', fasd_mocked, 2, mock.sentinel, execute=self.drv._execute) @@ -176,7 +177,7 @@ class TestBlockDeviceDriver(cinder.test.TestCase): TEST_SRC)) fasd_mocked.assert_called_once_with(TEST_SRC['size']) lp_mocked.assert_called_once_with(TEST_SRC) - gds_mocked.assert_called_once_with('/dev/loop2') + gds_mocked.assert_called_once_with(['/dev/loop2']) @mock.patch.object(cinder.image.image_utils, 'fetch_to_raw') def test_copy_image_to_volume(self, _fetch_to_raw): @@ -232,19 +233,24 @@ class TestBlockDeviceDriver(cinder.test.TestCase): self.assertEqual(set([path1, path2]), self.drv._get_used_devices()) - def test_get_device_size(self): - dev_path = '/dev/loop1' - out = '2048' + def test_get_devices_sizes(self): + dev_paths = ['/dev/loop1', '/dev/loop2', '/dev/loop3'] + out = '4294967296\n2147483648\n3221225472\nn' with mock.patch.object(self.drv, '_execute', return_value=(out, None)) as _execute: - self.assertEqual(1, self.drv._get_device_size(dev_path)) - _execute.assert_called_once_with('blockdev', '--getsz', dev_path, - run_as_root=True) + actual = self.drv._get_devices_sizes(dev_paths) + self.assertEqual(3, len(actual)) + self.assertEqual({'/dev/loop1': 4096, '/dev/loop2': 2048, + '/dev/loop3': 3072}, actual) + _execute.assert_called_once_with('blockdev', '--getsize64', + *dev_paths, run_as_root=True) def test_devices_sizes(self): - with mock.patch.object(self.drv, '_get_device_size') as _get_dvc_size: - _get_dvc_size.return_value = 1 + with mock.patch.object(self.drv, '_get_devices_sizes') as \ + _get_dvc_size: + _get_dvc_size.return_value = {'/dev/loop1': 1, '/dev/loop2': 1} + self.assertEqual(2, len(self.drv._devices_sizes())) self.assertEqual({'/dev/loop1': 1, '/dev/loop2': 1}, self.drv._devices_sizes()) @@ -253,19 +259,19 @@ class TestBlockDeviceDriver(cinder.test.TestCase): with mock.patch.object(self.drv, '_devices_sizes') as _dvc_sizes: with mock.patch.object(self.drv, '_get_used_devices') as \ _get_used_dvc: - _dvc_sizes.return_value = {'/dev/loop1': 1024, - '/dev/loop2': 1024} + _dvc_sizes.return_value = {'/dev/loop1': 1, + '/dev/loop2': 1} _get_used_dvc.return_value = set(['/dev/loop1', '/dev/loop2']) self.assertRaises(cinder.exception.CinderException, self.drv.find_appropriate_size_device, size) def test_find_appropriate_size_device_not_big_enough_disk(self): - size = 2 + size = 2948 with mock.patch.object(self.drv, '_devices_sizes') as _dvc_sizes: with mock.patch.object(self.drv, '_get_used_devices') as \ _get_used_dvc: _dvc_sizes.return_value = {'/dev/loop1': 1024, - '/dev/loop2': 1024} + '/dev/loop2': 1924} _get_used_dvc.return_value = set(['/dev/loop1']) self.assertRaises(cinder.exception.CinderException, self.drv.find_appropriate_size_device, size) diff --git a/cinder/volume/drivers/block_device.py b/cinder/volume/drivers/block_device.py index cfe778a2e..1937fd494 100644 --- a/cinder/volume/drivers/block_device.py +++ b/cinder/volume/drivers/block_device.py @@ -18,6 +18,7 @@ import os from oslo_config import cfg from oslo_log import log as logging from oslo_utils import importutils +from oslo_utils import units from cinder import context from cinder.db.sqlalchemy import api @@ -76,14 +77,15 @@ class BlockDeviceDriver(driver.BaseVD, driver.LocalVD, driver.CloneableVD, return if os.path.exists(dev_path) and \ self.configuration.volume_clear != 'none': + dev_size = self._get_devices_sizes([dev_path]) volutils.clear_volume( - self._get_device_size(dev_path), dev_path, + dev_size[dev_path], dev_path, volume_clear=self.configuration.volume_clear, volume_clear_size=self.configuration.volume_clear_size) - def local_path(self, volume): - if volume['provider_location']: - path = volume['provider_location'].rsplit(" ", 1) + def local_path(self, device): + if device['provider_location']: + path = device['provider_location'].rsplit(" ", 1) return path[-1] else: return None @@ -107,9 +109,10 @@ class BlockDeviceDriver(driver.BaseVD, driver.LocalVD, driver.CloneableVD, def create_cloned_volume(self, volume, src_vref): LOG.info(_LI('Creating clone of volume: %s'), src_vref['id']) device = self.find_appropriate_size_device(src_vref['size']) + dev_size = self._get_devices_sizes([device]) volutils.copy_volume( self.local_path(src_vref), device, - self._get_device_size(device) * 2048, + dev_size[device], self.configuration.volume_dd_blocksize, execute=self._execute) return { @@ -134,8 +137,8 @@ class BlockDeviceDriver(driver.BaseVD, driver.LocalVD, driver.CloneableVD, LOG.debug("Updating volume stats") backend_name = self.configuration.safe_get('volume_backend_name') - data = {'total_capacity_gb': total_size / 1024, - 'free_capacity_gb': free_size / 1024, + data = {'total_capacity_gb': total_size / units.Ki, + 'free_capacity_gb': free_size / units.Ki, 'reserved_percentage': self.configuration.reserved_percentage, 'QoS_support': False, 'volume_backend_name': backend_name or self.__class__.__name__, @@ -155,18 +158,22 @@ class BlockDeviceDriver(driver.BaseVD, driver.LocalVD, driver.CloneableVD, used_devices.add(local_path) return used_devices - def _get_device_size(self, dev_path): - out, _err = self._execute('blockdev', '--getsz', dev_path, + def _get_devices_sizes(self, dev_paths): + """Return devices' sizes in Mb""" + out, _err = self._execute('blockdev', '--getsize64', *dev_paths, run_as_root=True) - size_in_m = int(out) - return size_in_m / 2048 + dev_sizes = {} + out = out.split('\n') + # blockdev returns devices' sizes in order that + # they have been passed to it. + for n, size in enumerate(out[:-1]): + dev_sizes[dev_paths[n]] = int(size) / units.Mi + + return dev_sizes def _devices_sizes(self): available_devices = self.configuration.available_devices - dict_of_devices_sizes = {} - for device in available_devices: - dict_of_devices_sizes[device] = self._get_device_size(device) - return dict_of_devices_sizes + return self._get_devices_sizes(available_devices) def find_appropriate_size_device(self, size): dict_of_devices_sizes = self._devices_sizes() @@ -178,8 +185,9 @@ class BlockDeviceDriver(driver.BaseVD, driver.LocalVD, driver.CloneableVD, possible_device_size = None for device in free_devices: dev_size = dict_of_devices_sizes[device] - if size * 1024 <= dev_size and (possible_device is None or - dev_size < possible_device_size): + if (size * units.Ki <= dev_size and + (possible_device is None or + dev_size < possible_device_size)): possible_device = device possible_device_size = dev_size -- 2.45.2