Allow detaching disks by serial tag

At the moment, we allow tagging attached passthrough disks. It is
assumed that this tag will uniquely indentify a disk (Nova uses the
Cinder volume id).

At the same time, a passthrough disk cannot be attached to multiple
instances hosted by the same node (as opposed to VHDSet images, for
example). Also, having to identify a disk path each time can be a
time consuming operation.

Having this in mind, it makes sense to allow detaching passthrough
disks from VMs by using their serial tag instead of relying on the
actual disk resource path.

Related-Bug: #1694671

Change-Id: Ic3775c3e81a5f05b20221ef52e393fdb54d0190c

os_win/tests/unit/utils/compute/test_vmutils.py

@@ -865,20 +865,26 @@ class VMUtilsTestCase(test_base.OsWinBaseTestCase):
         mock_disk = self._prepare_mock_disk()
 
         self._vmutils.detach_vm_disk(self._FAKE_VM_NAME,
-                                     self._FAKE_HOST_RESOURCE)
+                                     self._FAKE_HOST_RESOURCE,
+                                     serial=mock.sentinel.serial)
         self._vmutils._jobutils.remove_virt_resource.assert_called_once_with(
             mock_disk)
 
-    def test_get_mounted_disk_resource_from_path(self):
-        mock_disk_1 = mock.MagicMock()
-        mock_disk_2 = mock.MagicMock()
-        mock_disk_2.HostResource = [self._FAKE_MOUNTED_DISK_PATH]
-        self._vmutils._conn.query.return_value = [mock_disk_1, mock_disk_2]
+    @ddt.data(None, mock.sentinel.serial)
+    def test_get_mounted_disk_resource_from_path(self, serial):
+        mock_disk = mock.MagicMock()
+
+        if serial:
+            self._vmutils._conn.query.return_value = [mock_disk]
+        else:
+            mock_disk.HostResource = [self._FAKE_MOUNTED_DISK_PATH]
+            self._vmutils._conn.query.return_value = [
+                mock.MagicMock(), mock_disk]
 
         physical_disk = self._vmutils._get_mounted_disk_resource_from_path(
-            self._FAKE_MOUNTED_DISK_PATH, True)
+            self._FAKE_MOUNTED_DISK_PATH, True, serial=serial)
 
-        self.assertEqual(mock_disk_2, physical_disk)
+        self.assertEqual(mock_disk, physical_disk)
 
     def test_get_controller_volume_paths(self):
         self._prepare_mock_disk()

os_win/utils/compute/vmutils.py

@@ -784,10 +784,11 @@ class VMUtils(baseutils.BaseUtilsVirt):
                                                                   is_physical)
         return disk_resource is not None
 
-    def detach_vm_disk(self, vm_name, disk_path, is_physical=True):
+    def detach_vm_disk(self, vm_name, disk_path=None, is_physical=True,
+                       serial=None):
         # TODO(claudiub): remove vm_name argument, no longer used.
-        disk_resource = self._get_mounted_disk_resource_from_path(disk_path,
-                                                                  is_physical)
+        disk_resource = self._get_mounted_disk_resource_from_path(
+            disk_path, is_physical, serial=serial)
 
         if disk_resource:
             parent = self._conn.query("SELECT * FROM "
@@ -799,7 +800,8 @@ class VMUtils(baseutils.BaseUtilsVirt):
             if not is_physical:
                 self._jobutils.remove_virt_resource(parent)
 
-    def _get_mounted_disk_resource_from_path(self, disk_path, is_physical):
+    def _get_mounted_disk_resource_from_path(self, disk_path, is_physical,
+                                             serial=None):
         if is_physical:
             class_name = self._RESOURCE_ALLOC_SETTING_DATA_CLASS
         else:
@@ -814,9 +816,15 @@ class VMUtils(baseutils.BaseUtilsVirt):
                      'res_sub_type_virt': self._HARD_DISK_RES_SUB_TYPE,
                      'res_sub_type_dvd': self._DVD_DISK_RES_SUB_TYPE})
 
+        if serial:
+            query += " AND ElementName='%s'" % serial
+
         disk_resources = self._compat_conn.query(query)
 
         for disk_resource in disk_resources:
+            if serial:
+                return disk_resource
+
             if disk_resource.HostResource:
                 if disk_resource.HostResource[0].lower() == disk_path.lower():
                     return disk_resource

releasenotes/notes/detach_disk_by_tag-9d8d5b04f91fedd2.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - |
+    os-win now allows detaching VM disks by their unique tag. This considerably
+    reduces the time needed for this operation in case of passthrough disks
+    as the disk paths no longer have to be retrieved (which can be really time
+    consuming, especially under high load).