Validate SAN policy when having passthrough disks

One common cause for passthrough disk attachment issues is an
incorrectly configured san policy. Having it set to 'online' will
prevevent disks from being attached to VMs.

This change allows the hyperv driver to check the san policy and
log a warning message when the driver loads and when a passthrough
disk is being attached.

Depends-On: I25033c15b77f494a417c9cd01d194a8bfb3cfe13
Change-Id: Ia123d98286fa99f45f91bfc47a4f0d468fa7d13e
Closes-Bug: #1727257

compute_hyperv/nova/volumeops.py

@@ -104,6 +104,17 @@ class VolumeOps(object):
             raise exception.VolumeDriverNotFound(driver_type=driver_type)
         return self.volume_drivers[driver_type]
 
+    def validate_host_configuration(self):
+        for protocol, volume_driver in self.volume_drivers.items():
+            try:
+                volume_driver.validate_host_configuration()
+            except exception.ValidationError as ex:
+                LOG.warning(
+                    "Volume driver %(protocol)s reported a validation "
+                    "error. Attaching such volumes will probably fail. "
+                    "Error message: %(err_msg)s.",
+                    dict(protocol=protocol, err_msg=ex.message))
+
     def attach_volumes(self, context, volumes, instance):
         for vol in volumes:
             self.attach_volume(context, vol['connection_info'], instance)
@@ -406,6 +417,10 @@ class BaseVolumeDriver(object):
 
         return self._get_disk_res_path(disk_paths[0])
 
+    def validate_host_configuration(self):
+        if self._is_block_dev:
+            self._check_san_policy()
+
     def _get_disk_res_path(self, disk_path):
         if self._is_block_dev:
             # We need the Msvm_DiskDrive resource path as this
@@ -423,8 +438,22 @@ class BaseVolumeDriver(object):
             raise exception.DiskNotFound(err_msg)
         return disk_res_path
 
+    def _check_san_policy(self):
+        disk_policy = self._diskutils.get_new_disk_policy()
+
+        accepted_policies = [os_win_const.DISK_POLICY_OFFLINE_SHARED,
+                             os_win_const.DISK_POLICY_OFFLINE_ALL]
+
+        if disk_policy not in accepted_policies:
+            err_msg = _("Invalid SAN policy. The SAN policy "
+                        "must be set to 'Offline Shared' or 'Offline All' "
+                        "in order to attach passthrough disks to instances.")
+            raise exception.ValidationError(message=err_msg)
+
     def attach_volume(self, connection_info, instance_name,
                       disk_bus=constants.CTRL_TYPE_SCSI):
+        self.validate_host_configuration()
+
         dev_info = self.connect_volume(connection_info)
 
         serial = connection_info['serial']

compute_hyperv/tests/unit/test_volumeops.py

@@ -85,6 +85,20 @@ class VolumeOpsTestCase(test_base.HyperVBaseTestCase):
                           self._volumeops._get_volume_driver,
                           connection_info=fake_conn_info)
 
+    def test_validate_host_configuration(self):
+        self._volumeops.volume_drivers = {
+            constants.STORAGE_PROTOCOL_SMBFS: mock.Mock(
+                side_effect=exception.ValidationError),
+            constants.STORAGE_PROTOCOL_ISCSI: mock.Mock(
+                side_effect=exception.ValidationError),
+            constants.STORAGE_PROTOCOL_FC: mock.Mock()
+        }
+
+        self._volumeops.validate_host_configuration()
+
+        for volume_drv in self._volumeops.volume_drivers.values():
+            volume_drv.validate_host_configuration.assert_called_once_with()
+
     @mock.patch.object(volumeops.VolumeOps, 'attach_volume')
     def test_attach_volumes(self, mock_attach_volume):
         block_device_info = get_fake_block_dev_info()
@@ -624,12 +638,44 @@ class BaseVolumeDriverTestCase(test_base.HyperVBaseTestCase):
             mock.sentinel.disk_path)
         self.assertEqual(mock.sentinel.disk_path, path)
 
+    @mock.patch.object(volumeops.BaseVolumeDriver,
+        '_check_san_policy')
+    @ddt.data(True, False)
+    def test_validate_host_configuration(self, is_block_dev,
+                                         fake_check_san_policy):
+        self._base_vol_driver._is_block_dev = is_block_dev
+
+        self._base_vol_driver.validate_host_configuration()
+
+        if is_block_dev:
+            fake_check_san_policy.assert_called_once_with()
+        else:
+            fake_check_san_policy.assert_not_called()
+
+    @ddt.data(os_win_const.DISK_POLICY_OFFLINE_ALL,
+              os_win_const.DISK_POLICY_ONLINE_ALL)
+    def test_check_san_policy(self, disk_policy):
+        self._diskutils.get_new_disk_policy.return_value = disk_policy
+
+        accepted_policies = [os_win_const.DISK_POLICY_OFFLINE_SHARED,
+                             os_win_const.DISK_POLICY_OFFLINE_ALL]
+
+        if disk_policy not in accepted_policies:
+            self.assertRaises(
+                exception.ValidationError,
+                self._base_vol_driver._check_san_policy)
+        else:
+            self._base_vol_driver._check_san_policy()
+
     @mock.patch.object(volumeops.BaseVolumeDriver,
                        '_get_disk_res_path')
     @mock.patch.object(volumeops.BaseVolumeDriver, '_get_disk_ctrl_and_slot')
     @mock.patch.object(volumeops.BaseVolumeDriver,
                        'connect_volume')
-    def _test_attach_volume(self, mock_connect_volume,
+    @mock.patch.object(volumeops.BaseVolumeDriver,
+                       'validate_host_configuration')
+    def _test_attach_volume(self, mock_validate_host_config,
+                            mock_connect_volume,
                             mock_get_disk_ctrl_and_slot,
                             mock_get_disk_res_path,
                             is_block_dev=True):
@@ -666,6 +712,7 @@ class BaseVolumeDriverTestCase(test_base.HyperVBaseTestCase):
             mock.sentinel.raw_path)
         mock_get_disk_ctrl_and_slot.assert_called_once_with(
             mock.sentinel.instance_name, mock.sentinel.disk_bus)
+        mock_validate_host_config.assert_called_once_with()
 
     def test_attach_volume_image_file(self):
         self._test_attach_volume(is_block_dev=False)