From ce401853f3e0f952bf6b1f9e327c53b8c34a5e1b Mon Sep 17 00:00:00 2001 From: Tomoki Sekiyama Date: Wed, 7 May 2014 16:52:20 -0400 Subject: [PATCH] I/O rate limit for volume copy with dd Currently, volume copy operations consumes disk I/O bandwidth heavily and may slow down the other guest instances. This patch limits bandwidth for volume copy to mitigate interference to other instance performance. In this implementation, 'dd' is put in a blkio cgroup for throttling, when CONF.volume_copy_bps_limit is set to non-zero. Change-Id: I32347d2e28842725207a1b3afac7fe137a3fd3b4 Signed-off-by: Tomoki Sekiyama Implements: blueprint limit-volume-copy-bandwidth --- cinder/tests/test_utils.py | 53 +++++++++++++++++++++++++++++++ cinder/tests/test_volume_utils.py | 31 ++++++++++++++++++ cinder/utils.py | 44 +++++++++++++++++++++++++ cinder/volume/driver.py | 8 +++++ cinder/volume/utils.py | 51 +++++++++++++++++++++++++++++ etc/cinder/cinder.conf.sample | 8 +++++ 6 files changed, 195 insertions(+) diff --git a/cinder/tests/test_utils.py b/cinder/tests/test_utils.py index 5ba73cfae..ff7b8c7ee 100644 --- a/cinder/tests/test_utils.py +++ b/cinder/tests/test_utils.py @@ -573,6 +573,59 @@ class GenericUtilsTestCase(test.TestCase): self.assertEqual(gid, 33333) mock_stat.assert_called_once_with(test_file) + @mock.patch('os.stat') + def test_get_blkdev_major_minor(self, mock_stat): + + class stat_result: + st_mode = 0o60660 + st_rdev = os.makedev(253, 7) + + test_device = '/dev/made_up_blkdev' + mock_stat.return_value = stat_result + dev = utils.get_blkdev_major_minor(test_device) + self.assertEqual('253:7', dev) + mock_stat.aseert_called_once_with(test_device) + + @mock.patch('os.stat') + @mock.patch.object(utils, 'execute') + def test_get_blkdev_major_minor_file(self, mock_exec, mock_stat): + + mock_exec.return_value = ( + 'Filesystem Size Used Avail Use% Mounted on\n' + '/dev/made_up_disk1 4096 2048 2048 50% /tmp\n', None) + + test_file = '/tmp/file' + test_partition = '/dev/made_up_disk1' + test_disk = '/dev/made_up_disk' + + class stat_result_file: + st_mode = 0o660 + + class stat_result_partition: + st_mode = 0o60660 + st_rdev = os.makedev(8, 65) + + class stat_result_disk: + st_mode = 0o60660 + st_rdev = os.makedev(8, 64) + + def fake_stat(path): + try: + return {test_file: stat_result_file, + test_partition: stat_result_partition, + test_disk: stat_result_disk}[path] + except KeyError: + raise OSError + + mock_stat.side_effect = fake_stat + + dev = utils.get_blkdev_major_minor(test_file) + self.assertEqual('8:64', dev) + mock_exec.aseert_called_once_with(test_file) + mock_stat.aseert_called_once_with(test_file) + mock_stat.aseert_called_once_with(test_partition) + mock_stat.aseert_called_once_with(test_disk) + class MonkeyPatchTestCase(test.TestCase): """Unit test for utils.monkey_patch().""" diff --git a/cinder/tests/test_volume_utils.py b/cinder/tests/test_volume_utils.py index 358a5e66a..5d0d68bc5 100644 --- a/cinder/tests/test_volume_utils.py +++ b/cinder/tests/test_volume_utils.py @@ -15,6 +15,7 @@ """Tests For miscellaneous util methods used with volume.""" +import mock import os import re @@ -217,3 +218,33 @@ class CopyVolumeTestCase(test.TestCase): volume_utils.copy_volume('/dev/zero', '/dev/null', 1024, CONF.volume_dd_blocksize, sync=True, ionice=None, execute=fake_utils_execute) + + +class BlkioCgroupTestCase(test.TestCase): + + @mock.patch.object(utils, 'get_blkdev_major_minor') + def test_setup_blkio_cgroup(self, mock_major_minor): + + def fake_get_blkdev_major_minor(path): + return {'src_volume': "253:0", 'dst_volume': "253:1"}[path] + + mock_major_minor.side_effect = fake_get_blkdev_major_minor + + self.exec_cnt = 0 + + def fake_utils_execute(*cmd, **kwargs): + exec_cmds = [('cgcreate', '-g', + 'blkio:' + CONF.volume_copy_blkio_cgroup_name), + ('cgset', '-r', + 'blkio.throttle.read_bps_device=253:0 1024', + CONF.volume_copy_blkio_cgroup_name), + ('cgset', '-r', + 'blkio.throttle.write_bps_device=253:1 1024', + CONF.volume_copy_blkio_cgroup_name)] + self.assertEqual(exec_cmds[self.exec_cnt], cmd) + self.exec_cnt += 1 + + cmd = volume_utils.setup_blkio_cgroup('src_volume', 'dst_volume', 1024, + execute=fake_utils_execute) + self.assertEqual(['cgexec', '-g', + 'blkio:' + CONF.volume_copy_blkio_cgroup_name], cmd) diff --git a/cinder/utils.py b/cinder/utils.py index 6bc03835e..5bc89dae6 100644 --- a/cinder/utils.py +++ b/cinder/utils.py @@ -783,6 +783,50 @@ def get_file_gid(path): return os.stat(path).st_gid +def _get_disk_of_partition(devpath, st=None): + """Returns a disk device path from a partition device path, and stat for + the device. If devpath is not a partition, devpath is returned as it is. + For example, '/dev/sda' is returned for '/dev/sda1', and '/dev/disk1' is + for '/dev/disk1p1' ('p' is prepended to the partition number if the disk + name ends with numbers). + """ + if st is None: + st = os.stat(devpath) + diskpath = re.sub('(?:(?<=\d)p)?\d+$', '', devpath) + if diskpath != devpath: + try: + st = os.stat(diskpath) + if stat.S_ISBLK(st.st_mode): + return (diskpath, st) + except OSError: + pass + # devpath is not a partition + return (devpath, st) + + +def get_blkdev_major_minor(path, lookup_for_file=True): + """Get the device's "major:minor" number of a block device to control + I/O ratelimit of the specified path. + If lookup_for_file is True and the path is a regular file, lookup a disk + device which the file lies on and returns the result for the device. + """ + st = os.stat(path) + if stat.S_ISBLK(st.st_mode): + path, st = _get_disk_of_partition(path, st) + return '%d:%d' % (os.major(st.st_rdev), os.minor(st.st_rdev)) + elif stat.S_ISCHR(st.st_mode): + # No I/O ratelimit control is provided for character devices + return None + elif lookup_for_file: + # lookup the mounted disk which the file lies on + out, _err = execute('df', path) + devpath = out.split("\n")[1].split()[0] + return get_blkdev_major_minor(devpath, False) + else: + msg = _("Unable to get a block device for file \'%s\'") % path + raise exception.Error(msg) + + def check_string_length(value, name, min_length=0, max_length=None): """Check the length of specified string :param value: the value of the string diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index a398c04c8..3f651d40a 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -103,6 +103,14 @@ volume_opts = [ default='1M', help='The default block size used when copying/clearing ' 'volumes'), + cfg.StrOpt('volume_copy_blkio_cgroup_name', + default='cinder-volume-copy', + help='The blkio cgroup name to be used to limit bandwidth ' + 'of volume copy'), + cfg.IntOpt('volume_copy_bps_limit', + default=0, + help='The upper limit of bandwidth of volume copy. ' + '0 => unlimited'), ] # for backward compatibility diff --git a/cinder/volume/utils.py b/cinder/volume/utils.py index 0ca7b68b5..d10219a26 100644 --- a/cinder/volume/utils.py +++ b/cinder/volume/utils.py @@ -103,6 +103,53 @@ def notify_about_snapshot_usage(context, snapshot, event_suffix, usage_info) +def setup_blkio_cgroup(srcpath, dstpath, bps_limit, execute=utils.execute): + if not bps_limit: + return None + + try: + srcdev = utils.get_blkdev_major_minor(srcpath) + except exception.Error as e: + msg = (_('Failed to get device number for read throttling: %(error)s') + % {'error': e}) + LOG.error(msg) + srcdev = None + + try: + dstdev = utils.get_blkdev_major_minor(dstpath) + except exception.Error as e: + msg = (_('Failed to get device number for write throttling: %(error)s') + % {'error': e}) + LOG.error(msg) + dstdev = None + + if not srcdev and not dstdev: + return None + + group_name = CONF.volume_copy_blkio_cgroup_name + try: + execute('cgcreate', '-g', 'blkio:%s' % group_name, run_as_root=True) + except processutils.ProcessExecutionError: + LOG.warn(_('Failed to create blkio cgroup')) + return None + + try: + if srcdev: + execute('cgset', '-r', 'blkio.throttle.read_bps_device=%s %d' + % (srcdev, bps_limit), group_name, run_as_root=True) + if dstdev: + execute('cgset', '-r', 'blkio.throttle.write_bps_device=%s %d' + % (dstdev, bps_limit), group_name, run_as_root=True) + except processutils.ProcessExecutionError: + msg = (_('Failed to setup blkio cgroup to throttle the devices: ' + '\'%(src)s\',\'%(dst)s\'') + % {'src': srcdev, 'dst': dstdev}) + LOG.warn(msg) + return None + + return ['cgexec', '-g', 'blkio:%s' % group_name] + + def _calculate_count(size_in_m, blocksize): # Check if volume_dd_blocksize is valid @@ -156,6 +203,10 @@ def copy_volume(srcstr, deststr, size_in_m, blocksize, sync=False, if ionice is not None: cmd = ['ionice', ionice] + cmd + cgcmd = setup_blkio_cgroup(srcstr, deststr, CONF.volume_copy_bps_limit) + if cgcmd: + cmd = cgcmd + cmd + # Perform the copy execute(*cmd, run_as_root=True) diff --git a/etc/cinder/cinder.conf.sample b/etc/cinder/cinder.conf.sample index 545df556a..3b3eba9ed 100644 --- a/etc/cinder/cinder.conf.sample +++ b/etc/cinder/cinder.conf.sample @@ -1026,6 +1026,14 @@ # (string value) #volume_dd_blocksize=1M +# The blkio cgroup name to be used to limit bandwidth of +# volume copy (string value) +#volume_copy_blkio_cgroup_name=cinder-volume-copy + +# The upper limit of bandwidth of volume copy. 0 => unlimited +# (integer value) +#volume_copy_bps_limit=0 + # # Options defined in cinder.volume.drivers.block_device -- 2.45.2