kvm: bind virtio-blk disks to an iothread, and let running SCSI disks win on attach

A disk with iothreads enabled got its iothread= binding only when its bus was
VIRTIO, although a virtio-blk disk is rendered with the same libvirt bus. A
virtio-blk VM with the iothreads detail therefore ran every disk with
io='threads' but no iothread, and data disks following a virtio-blk root now
lost it as well. Treat VIRTIOBLK like VIRTIO there.

When a data disk is attached without a controller of its own, the virtio-blk
root fallback ran before the scan of the running domain. The root detail can
be changed on a running VM, so a VM started on SCSI whose detail was later set
to virtio-blk would get a virtio disk hot-plugged next to its sd* ones. Scan
the running disks first and use the fallback only when no SCSI disk is there.

Author: calvix

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java

@@ -1203,7 +1203,7 @@ public String toString() {
                     diskBuilder.append(String.format("io='%s' ", ioDriver));
                 }
 
-                if (isIothreadsEnabled && _bus == DiskBus.VIRTIO) {
+                if (isIothreadsEnabled && (_bus == DiskBus.VIRTIO || _bus == DiskBus.VIRTIOBLK)) {
                     diskBuilder.append(String.format("iothread='%s' ", NUMBER_OF_IOTHREADS));
                 }
                 diskBuilder.append("/>\n");

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java

@@ -1538,15 +1538,6 @@ protected DiskDef.DiskBus getAttachDiskBusType(int deviceId, List<DiskDef> disks
                     diskController, controllerKey, deviceId);
             return busType;
         }
-        if (deviceId != 0 && DiskDef.DiskBus.VIRTIOBLK == DiskDef.DiskBus.fromValue(
-                MapUtils.getString(controllerInfo, VmDetailConstants.ROOT_DISK_CONTROLLER))) {
-            // A data disk with no controller of its own follows the root, as it already does when the
-            // VM starts. The scan below cannot help here: a virtio-blk disk is rendered with libvirt's
-            // 'virtio' bus, so it is indistinguishable from a plain virtio disk in the domain XML, and
-            // the disk would silently lose discard until the next stop/start.
-            logger.debug("Attaching disk (deviceId={}) with the root controller virtio-blk", deviceId);
-            return DiskDef.DiskBus.VIRTIOBLK;
-        }
         for (final DiskDef disk : disks) {
             if (disk.getDeviceType() != DeviceType.DISK) {
                 continue;
@@ -1557,6 +1548,17 @@ protected DiskDef.DiskBus getAttachDiskBusType(int deviceId, List<DiskDef> disks
                 return DiskDef.DiskBus.VIRTIOBLK;
             }
         }
+        if (deviceId != 0 && DiskDef.DiskBus.VIRTIOBLK == DiskDef.DiskBus.fromValue(
+                MapUtils.getString(controllerInfo, VmDetailConstants.ROOT_DISK_CONTROLLER))) {
+            // A data disk with no controller of its own follows the root, as it already does when the
+            // VM starts. The scan above cannot see this case: a virtio-blk disk is rendered with
+            // libvirt's 'virtio' bus, so it is indistinguishable from a plain virtio disk in the domain
+            // XML, and the disk would silently lose discard until the next stop/start. The scan still
+            // wins when the running VM is on SCSI, so a detail changed after the VM started cannot
+            // put a virtio disk next to its SCSI ones.
+            logger.debug("Attaching disk (deviceId={}) with the root controller virtio-blk", deviceId);
+            return DiskDef.DiskBus.VIRTIOBLK;
+        }
         return DiskDef.DiskBus.VIRTIO;
     }
 

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java

@@ -642,4 +642,14 @@ public void testDiskDefScsiIsUnchanged() {
         String xmlDef = disk.toString();
         assertTrue(xmlDef, xmlDef.contains("<target dev='sda' bus='scsi'/>"));
     }
+
+    @Test
+    public void testDiskDefVirtioBlkGetsAnIothreadLikeVirtio() {
+        DiskDef disk = new DiskDef();
+        disk.defFileBasedDisk("/var/lib/libvirt/images/disk.qcow2", 0, DiskDef.DiskBus.VIRTIOBLK, DiskDef.DiskFmtType.QCOW2);
+        disk.isIothreadsEnabled(true);
+
+        String xmlDef = disk.toString();
+        assertTrue(xmlDef, xmlDef.contains("iothread="));
+    }
 }

plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java

@@ -523,4 +523,13 @@ public void getAttachDiskBusTypeStillDetectsScsiFromTheRunningDisks() {
         Assert.assertEquals(LibvirtVMDef.DiskDef.DiskBus.SCSI,
                 storageProcessor.getAttachDiskBusType(1, List.of(diskWithBus(LibvirtVMDef.DiskDef.DiskBus.SCSI)), Map.of()));
     }
+
+    @Test
+    public void getAttachDiskBusTypePrefersTheRunningScsiDisksOverAVirtioBlkRootDetail() {
+        // the detail can be changed on a running VM; the disks it actually runs on win, so a SCSI VM
+        // does not get a virtio disk hot-plugged next to its sd* ones
+        Map<String, String> controllerInfo = Map.of(VmDetailConstants.ROOT_DISK_CONTROLLER, "virtio-blk");
+        Assert.assertEquals(LibvirtVMDef.DiskDef.DiskBus.SCSI,
+                storageProcessor.getAttachDiskBusType(1, List.of(diskWithBus(LibvirtVMDef.DiskDef.DiskBus.SCSI)), controllerInfo));
+    }
 }