Merge "Improve detaching passthrough disks"

compute_hyperv/nova/volumeops.py

@@ -396,14 +396,27 @@ class BaseVolumeDriver(object):
                                        slot)
 
     def detach_volume(self, connection_info, instance_name):
-        disk_path = self.get_disk_resource_path(connection_info)
+        # Retrieving the disk path can be a time consuming operation in
+        # case of passthrough disks. As such disks attachments will be
+        # tagged using the volume id, we'll just use that instead.
+        #
+        # Note that Hyper-V does not allow us to attach the same passthrough
+        # disk to multiple instances, which means that we're safe to rely
+        # on this tag.
+        if not self._is_block_dev:
+            disk_path = self.get_disk_resource_path(connection_info)
+        else:
+            disk_path = None
 
-        LOG.debug("Detaching disk %(disk_path)s "
-                  "from instance: %(instance_name)s",
+        serial = connection_info['serial']
+        LOG.debug("Detaching disk from instance: %(instance_name)s. "
+                  "Disk path: %(disk_path)s. Disk serial tag: %(serial)s.",
                   dict(disk_path=disk_path,
+                       serial=serial,
                        instance_name=instance_name))
         self._vmutils.detach_vm_disk(instance_name, disk_path,
-                                     is_physical=self._is_block_dev)
+                                     is_physical=self._is_block_dev,
+                                     serial=serial)
 
     def _get_disk_ctrl_and_slot(self, instance_name, disk_bus):
         if disk_bus == constants.CTRL_TYPE_IDE:

compute_hyperv/tests/unit/test_volumeops.py

@@ -445,6 +445,7 @@ class VolumeOpsTestCase(test_base.HyperVBaseTestCase):
                  task_states.IMAGE_SNAPSHOT_PENDING])])
 
 
+@ddt.ddt
 class BaseVolumeDriverTestCase(test_base.HyperVBaseTestCase):
     """Unit tests for Hyper-V BaseVolumeDriver class."""
 
@@ -603,20 +604,29 @@ class BaseVolumeDriverTestCase(test_base.HyperVBaseTestCase):
     def test_attach_volume_block_dev(self):
         self._test_attach_volume(is_block_dev=True)
 
+    @ddt.data(True, False)
     @mock.patch.object(volumeops.BaseVolumeDriver,
                        'get_disk_resource_path')
-    def test_detach_volume(self, mock_get_disk_resource_path):
+    def test_detach_volume(self, is_block_dev, mock_get_disk_resource_path):
         connection_info = get_fake_connection_info()
+        self._base_vol_driver._is_block_dev = is_block_dev
 
         self._base_vol_driver.detach_volume(connection_info,
                                             mock.sentinel.instance_name)
 
-        mock_get_disk_resource_path.assert_called_once_with(
-            connection_info)
+        if is_block_dev:
+            exp_disk_res_path = None
+            self.assertFalse(mock_get_disk_resource_path.called)
+        else:
+            exp_disk_res_path = mock_get_disk_resource_path.return_value
+            mock_get_disk_resource_path.assert_called_once_with(
+                connection_info)
+
         self._vmutils.detach_vm_disk.assert_called_once_with(
             mock.sentinel.instance_name,
-            mock_get_disk_resource_path.return_value,
-            is_physical=self._base_vol_driver._is_block_dev)
+            exp_disk_res_path,
+            is_physical=is_block_dev,
+            serial=connection_info['serial'])
 
     def test_get_disk_ctrl_and_slot_ide(self):
         ctrl, slot = self._base_vol_driver._get_disk_ctrl_and_slot(