Add Hyper-V storage QoS support

Hyper-V provides options to specify maximum IOPS per virtual disk
image.

By leveraging this feature, this blueprint proposes to add support
for setting QoS specs targeting instance local disks as well
as volumes exported through SMB.

This will work similar to the libvirt driver, expecting the same
extra specs.

Implements: blueprint hyperv-storage-qos

Change-Id: I4ce44c8b7ab54a0bfc8c7401e675876dbd109e2d

nova/tests/unit/virt/hyperv/test_vmops.py

@@ -34,6 +34,7 @@ from nova.tests.unit.virt.hyperv import test_base
 from nova.virt import hardware
 from nova.virt.hyperv import constants
 from nova.virt.hyperv import vmops
+from nova.virt.hyperv import volumeops
 
 CONF = cfg.CONF
 
@@ -475,6 +476,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
 
     @mock.patch('nova.virt.hyperv.volumeops.VolumeOps'
                 '.attach_volumes')
+    @mock.patch.object(vmops.VMOps, '_set_instance_disk_qos_specs')
     @mock.patch.object(vmops.VMOps, '_create_vm_com_port_pipes')
     @mock.patch.object(vmops.VMOps, '_attach_ephemerals')
     @mock.patch.object(vmops.VMOps, '_attach_root_device')
@@ -483,6 +485,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
                               mock_attach_root_device,
                               mock_attach_ephemerals,
                               mock_create_pipes,
+                              mock_set_qos_specs,
                               mock_attach_volumes,
                               enable_instance_metrics,
                               vm_gen=constants.VM_GEN_1):
@@ -529,6 +532,7 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
         mock_enable = self._vmops._metricsutils.enable_vm_metrics_collection
         if enable_instance_metrics:
             mock_enable.assert_called_once_with(mock_instance.name)
+        mock_set_qos_specs.assert_called_once_with(mock_instance)
 
     def test_create_instance(self):
         self._test_create_instance(enable_instance_metrics=True)
@@ -1444,3 +1448,64 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
         self.assertRaises(exception.InstanceNotRescuable,
                           self._vmops.unrescue_instance,
                           mock_instance)
+
+    @mock.patch.object(volumeops.VolumeOps, 'bytes_per_sec_to_iops')
+    @mock.patch.object(vmops.VMOps, '_get_scoped_flavor_extra_specs')
+    @mock.patch.object(vmops.VMOps, '_get_instance_local_disks')
+    def test_set_instance_disk_qos_specs(self, mock_get_local_disks,
+                                         mock_get_scoped_specs,
+                                         mock_bytes_per_sec_to_iops):
+        fake_total_bytes_sec = 8
+        fake_total_iops_sec = 1
+        mock_instance = fake_instance.fake_instance_obj(self.context)
+        mock_local_disks = [mock.sentinel.root_vhd_path,
+                            mock.sentinel.eph_vhd_path]
+
+        mock_get_local_disks.return_value = mock_local_disks
+        mock_set_qos_specs = self._vmops._vmutils.set_disk_qos_specs
+        mock_get_scoped_specs.return_value = dict(
+            disk_total_bytes_sec=fake_total_bytes_sec)
+        mock_bytes_per_sec_to_iops.return_value = fake_total_iops_sec
+
+        self._vmops._set_instance_disk_qos_specs(mock_instance)
+
+        mock_bytes_per_sec_to_iops.assert_called_once_with(
+            fake_total_bytes_sec)
+
+        mock_get_local_disks.assert_called_once_with(mock_instance.name)
+        expected_calls = [mock.call(disk_path, fake_total_iops_sec)
+                          for disk_path in mock_local_disks]
+        mock_set_qos_specs.assert_has_calls(expected_calls)
+
+    def test_get_instance_local_disks(self):
+        fake_instance_dir = 'fake_instance_dir'
+        fake_local_disks = [os.path.join(fake_instance_dir, disk_name)
+                            for disk_name in ['root.vhd', 'configdrive.iso']]
+        fake_instance_disks = ['fake_remote_disk'] + fake_local_disks
+
+        mock_get_storage_paths = self._vmops._vmutils.get_vm_storage_paths
+        mock_get_storage_paths.return_value = [fake_instance_disks, []]
+        mock_get_instance_dir = self._vmops._pathutils.get_instance_dir
+        mock_get_instance_dir.return_value = fake_instance_dir
+
+        ret_val = self._vmops._get_instance_local_disks(
+            mock.sentinel.instance_name)
+
+        self.assertEqual(fake_local_disks, ret_val)
+
+    def test_get_scoped_flavor_extra_specs(self):
+        # The flavor extra spect dict contains only string values.
+        fake_total_bytes_sec = '8'
+
+        mock_instance = fake_instance.fake_instance_obj(self.context)
+        mock_instance.flavor.extra_specs = {
+            'spec_key': 'spec_value',
+            'quota:total_bytes_sec': fake_total_bytes_sec}
+
+        ret_val = self._vmops._get_scoped_flavor_extra_specs(
+            mock_instance, scope='quota')
+
+        expected_specs = {
+            'total_bytes_sec': fake_total_bytes_sec
+        }
+        self.assertEqual(expected_specs, ret_val)

nova/tests/unit/virt/hyperv/test_volumeops.py

@@ -19,6 +19,7 @@ import os
 import mock
 from os_win import exceptions as os_win_exc
 from oslo_config import cfg
+from oslo_utils import units
 
 from nova import exception
 from nova import test
@@ -145,6 +146,25 @@ class VolumeOpsTestCase(test_base.HyperVBaseTestCase):
         fake_volume_driver.disconnect_volumes.assert_called_once_with(
             block_device_mapping)
 
+    @mock.patch.object(volumeops.VolumeOps, '_get_volume_driver')
+    def test_attach_volume(self, mock_get_volume_driver):
+        fake_conn_info = {
+            'data': {'qos_specs': mock.sentinel.qos_specs}
+        }
+
+        mock_volume_driver = mock_get_volume_driver.return_value
+
+        self._volumeops.attach_volume(fake_conn_info,
+                                      mock.sentinel.instance_name,
+                                      disk_bus=mock.sentinel.disk_bus)
+
+        mock_volume_driver.attach_volume.assert_called_once_with(
+            fake_conn_info,
+            mock.sentinel.instance_name,
+            disk_bus=mock.sentinel.disk_bus)
+        mock_volume_driver.set_disk_qos_specs.assert_called_once_with(
+            fake_conn_info, mock.sentinel.qos_specs)
+
     def test_get_volume_connector(self):
         mock_instance = mock.DEFAULT
         initiator = self._volumeops._volutils.get_iscsi_initiator.return_value
@@ -213,6 +233,23 @@ class VolumeOpsTestCase(test_base.HyperVBaseTestCase):
         self.assertEqual(get_mounted_disk.return_value,
                          resulted_disk_path)
 
+    def test_bytes_per_sec_to_iops(self):
+        no_bytes = 15 * units.Ki
+        expected_iops = 2
+
+        resulted_iops = self._volumeops.bytes_per_sec_to_iops(no_bytes)
+        self.assertEqual(expected_iops, resulted_iops)
+
+    @mock.patch.object(volumeops.LOG, 'warning')
+    def test_validate_qos_specs(self, mock_warning):
+        supported_qos_specs = [mock.sentinel.spec1, mock.sentinel.spec2]
+        requested_qos_specs = {mock.sentinel.spec1: mock.sentinel.val,
+                               mock.sentinel.spec3: mock.sentinel.val2}
+
+        self._volumeops.validate_qos_specs(requested_qos_specs,
+                                           supported_qos_specs)
+        self.assertTrue(mock_warning.called)
+
 
 class ISCSIVolumeDriverTestCase(test_base.HyperVBaseTestCase):
     """Unit tests for Hyper-V ISCSIVolumeDriver class."""
@@ -601,3 +638,31 @@ class SMBFSVolumeDriverTestCase(test_base.HyperVBaseTestCase):
         mock_unmount_share = self._volume_driver._smbutils.unmount_smb_share
         mock_unmount_share.assert_called_once_with(
             self._FAKE_SHARE_NORMALIZED)
+
+    @mock.patch.object(volumeops.VolumeOps, 'bytes_per_sec_to_iops')
+    @mock.patch.object(volumeops.VolumeOps, 'validate_qos_specs')
+    @mock.patch.object(volumeops.SMBFSVolumeDriver, '_get_disk_path')
+    def test_set_disk_qos_specs(self, mock_get_disk_path,
+                                mock_validate_qos_specs,
+                                mock_bytes_per_sec_to_iops):
+        fake_total_bytes_sec = 8
+        fake_total_iops_sec = 1
+
+        storage_qos_specs = {'total_bytes_sec': fake_total_bytes_sec}
+        expected_supported_specs = ['total_iops_sec', 'total_bytes_sec']
+        mock_set_qos_specs = self._volume_driver._vmutils.set_disk_qos_specs
+        mock_bytes_per_sec_to_iops.return_value = fake_total_iops_sec
+
+        self._volume_driver.set_disk_qos_specs(mock.sentinel.connection_info,
+                                               storage_qos_specs)
+
+        mock_validate_qos_specs.assert_called_once_with(
+            storage_qos_specs, expected_supported_specs)
+        mock_bytes_per_sec_to_iops.assert_called_once_with(
+            fake_total_bytes_sec)
+        mock_disk_path = mock_get_disk_path.return_value
+        mock_get_disk_path.assert_called_once_with(
+            mock.sentinel.connection_info)
+        mock_set_qos_specs.assert_called_once_with(
+            mock_disk_path,
+            fake_total_iops_sec)

nova/virt/hyperv/constants.py

@@ -83,3 +83,5 @@ DEFAULT_SERIAL_CONSOLE_PORT = 1
 FLAVOR_ESPEC_REMOTEFX_RES = 'os:resolution'
 FLAVOR_ESPEC_REMOTEFX_MONITORS = 'os:monitors'
 FLAVOR_ESPEC_REMOTEFX_VRAM = 'os:vram'
+
+IOPS_BASE_SIZE = 8 * units.Ki

nova/virt/hyperv/vmops.py

@@ -350,6 +350,8 @@ class VMOps(object):
         if CONF.hyperv.enable_instance_metrics_collection:
             self._metricsutils.enable_vm_metrics_collection(instance_name)
 
+        self._set_instance_disk_qos_specs(instance)
+
     def _configure_remotefx(self, instance, vm_gen):
         extra_specs = instance.flavor.extra_specs
         remotefx_max_resolution = extra_specs.get(
@@ -869,3 +871,35 @@ class VMOps(object):
             self.attach_config_drive(instance, configdrive_path, vm_gen)
 
         self.power_on(instance)
+
+    def _set_instance_disk_qos_specs(self, instance):
+        quota_specs = self._get_scoped_flavor_extra_specs(instance, 'quota')
+
+        disk_total_bytes_sec = int(
+            quota_specs.get('disk_total_bytes_sec') or 0)
+        disk_total_iops_sec = int(
+            quota_specs.get('disk_total_iops_sec') or
+            self._volumeops.bytes_per_sec_to_iops(disk_total_bytes_sec))
+
+        if disk_total_iops_sec:
+            local_disks = self._get_instance_local_disks(instance.name)
+            for disk_path in local_disks:
+                self._vmutils.set_disk_qos_specs(disk_path,
+                                                 disk_total_iops_sec)
+
+    def _get_instance_local_disks(self, instance_name):
+        instance_path = self._pathutils.get_instance_dir(instance_name)
+        instance_disks = self._vmutils.get_vm_storage_paths(instance_name)[0]
+        local_disks = [disk_path for disk_path in instance_disks
+                       if instance_path in disk_path]
+        return local_disks
+
+    def _get_scoped_flavor_extra_specs(self, instance, scope):
+        extra_specs = instance.flavor.extra_specs or {}
+        filtered_specs = {}
+        for spec, value in extra_specs.items():
+            if ':' in spec:
+                _scope, key = spec.split(':')
+                if _scope == scope:
+                    filtered_specs[key] = value
+        return filtered_specs

nova/virt/hyperv/volumeops.py

@@ -30,7 +30,7 @@ from six.moves import range
 
 import nova.conf
 from nova import exception
-from nova.i18n import _, _LE, _LW
+from nova.i18n import _, _LE, _LI, _LW
 from nova import utils
 from nova.virt import driver
 from nova.virt.hyperv import constants
@@ -78,6 +78,11 @@ class VolumeOps(object):
         volume_driver.attach_volume(connection_info, instance_name,
                                     disk_bus=disk_bus)
 
+        qos_specs = connection_info['data'].get('qos_specs') or {}
+        if qos_specs:
+            volume_driver.set_disk_qos_specs(connection_info,
+                                             qos_specs)
+
     def detach_volume(self, connection_info, instance_name):
         volume_driver = self._get_volume_driver(
             connection_info=connection_info)
@@ -147,6 +152,26 @@ class VolumeOps(object):
         return volume_driver.get_mounted_disk_path_from_volume(
             connection_info)
 
+    @staticmethod
+    def bytes_per_sec_to_iops(no_bytes):
+        # Hyper-v uses normalized IOPS (8 KB increments)
+        # as IOPS allocation units.
+        return (
+            (no_bytes + constants.IOPS_BASE_SIZE - 1) //
+            constants.IOPS_BASE_SIZE)
+
+    @staticmethod
+    def validate_qos_specs(qos_specs, supported_qos_specs):
+        unsupported_specs = set(qos_specs.keys()).difference(
+            supported_qos_specs)
+        if unsupported_specs:
+            msg = (_LW('Got unsupported QoS specs: '
+                       '%(unsupported_specs)s. '
+                       'Supported qos specs: %(supported_qos_specs)s') %
+                   {'unsupported_specs': unsupported_specs,
+                    'supported_qos_specs': supported_qos_specs})
+            LOG.warning(msg)
+
 
 class ISCSIVolumeDriver(object):
     def __init__(self):
@@ -325,6 +350,10 @@ class ISCSIVolumeDriver(object):
     def initialize_volume_connection(self, connection_info):
         self.login_storage_target(connection_info)
 
+    def set_disk_qos_specs(self, connection_info, disk_qos_specs):
+        LOG.info(_LI("The iSCSI Hyper-V volume driver does not support QoS. "
+                     "Ignoring QoS specs."))
+
 
 def export_path_synchronized(f):
     def wrapper(inst, connection_info, *args, **kwargs):
@@ -439,3 +468,16 @@ class SMBFSVolumeDriver(object):
         def unmount_synchronized():
             self._smbutils.unmount_smb_share(export_path)
         unmount_synchronized()
+
+    def set_disk_qos_specs(self, connection_info, qos_specs):
+        supported_qos_specs = ['total_iops_sec', 'total_bytes_sec']
+        VolumeOps.validate_qos_specs(qos_specs, supported_qos_specs)
+
+        total_bytes_sec = int(qos_specs.get('total_bytes_sec') or 0)
+        total_iops_sec = int(qos_specs.get('total_iops_sec') or
+                             VolumeOps.bytes_per_sec_to_iops(
+                                total_bytes_sec))
+
+        if total_iops_sec:
+            disk_path = self._get_disk_path(connection_info)
+            self._vmutils.set_disk_qos_specs(disk_path, total_iops_sec)

releasenotes/notes/bp-hyperv-storage-qos-d559634e5df0f1d4.yaml

@@ -0,0 +1,17 @@
+---
+features:
+  - |
+    The Hyper-V driver now supports the following quota flavor extra
+    specs, allowing to specify IO limits applied for each of the
+    instance local disks, individually.
+
+    - quota:disk_total_bytes_sec
+    - quota:disk_total_iops_sec - those are normalized IOPS, thus each
+      IO request is accounted for as 1 normalized IO if the size of the
+      request is less than or equal to a predefined base size (8KB).
+
+    Also, the following Cinder front-end QoS specs are now supported
+    for SMB Cinder backends:
+
+    - total_bytes_sec
+    - total_iops_sec - normalized IOPS