server: Avoid device id collision between config drive ISO and data volumes on KVM

ConfigDriveNetworkElement hardcodes disk sequence 4 (CONFIGDRIVEDISKSEQ) for
the config drive ISO, but the volume device-id allocators
(VolumeApiServiceImpl.getDeviceId, KVMGuru.getNextAvailableDeviceId) only
look at persisted VolumeVO rows and have no knowledge that slot 4 is
reserved by the config drive. When a VM using ConfigDrive already has data
disks on device ids 1 and 2, attaching a 3rd data disk gets assigned device
id 4 as well, producing two disks with the same libvirt target device
(e.g. "sde") once both end up on the same bus namespace, as happens with
Q35/UEFI machine types.

Both allocators now skip CONFIGDRIVEDISKSEQ when the VM has a config drive
(detected via the existing CONFIG_DRIVE_LOCATION vm detail), the same way
they already permanently reserve device id 3 for the CD-ROM slot.

Author: weizhouapache

server/src/main/java/com/cloud/hypervisor/KVMGuru.java

@@ -27,6 +27,7 @@
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao;
 import com.cloud.hypervisor.kvm.dpdk.DpdkHelper;
+import com.cloud.network.element.ConfigDriveNetworkElement;
 import com.cloud.service.ServiceOfferingVO;
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.GuestOSHypervisorVO;
@@ -42,7 +43,9 @@
 import com.cloud.vm.VMInstanceVO;
 import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.VirtualMachineProfile;
+import com.cloud.vm.VmDetailConstants;
 import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.vm.dao.VMInstanceDetailsDao;
 import org.apache.cloudstack.backup.Backup;
 import org.apache.cloudstack.storage.command.CopyCommand;
 import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
@@ -71,6 +74,8 @@ public class KVMGuru extends HypervisorGuruBase implements HypervisorGuru {
     VolumeDao _volumeDao;
     @Inject
     HypervisorCapabilitiesDao _hypervisorCapabilitiesDao;
+    @Inject
+    VMInstanceDetailsDao _vmInstanceDetailsDao;
 
 
     @Override
@@ -86,7 +91,7 @@ protected KVMGuru() {
      * Get next free DeviceId for a KVM Guest
      */
 
-    protected Long getNextAvailableDeviceId(List<VolumeVO> vmVolumes) {
+    protected Long getNextAvailableDeviceId(long vmId, List<VolumeVO> vmVolumes) {
 
         int maxDataVolumesSupported;
         int maxDeviceId;
@@ -103,6 +108,9 @@ protected Long getNextAvailableDeviceId(List<VolumeVO> vmVolumes) {
             devIds.add(String.valueOf(i));
         }
         devIds.remove("3");
+        if (_vmInstanceDetailsDao.findDetail(vmId, VmDetailConstants.CONFIG_DRIVE_LOCATION) != null) {
+            devIds.remove(ConfigDriveNetworkElement.CONFIGDRIVEDISKSEQ.toString());
+        }
         for (VolumeVO vmVolume : vmVolumes) {
             devIds.remove(vmVolume.getDeviceId().toString().trim());
         }
@@ -371,7 +379,7 @@ public VirtualMachine importVirtualMachineFromBackup(long zoneId, long domainId,
                } else if (VMVolToRestore.getType() == Volume.Type.DATADISK) {
                    List<VolumeVO> vmVolumes = _volumeDao.findByInstance(vm.getId());
                    _volumeDao.update(volume.getId(), volume);
-                   _volumeDao.attachVolume(volume.getId(), vm.getId(), getNextAvailableDeviceId(vmVolumes));
+                   _volumeDao.attachVolume(volume.getId(), vm.getId(), getNextAvailableDeviceId(vm.getId(), vmVolumes));
                }
                UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_ATTACH, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(),
                        volume.getDiskOfferingId(), volume.getTemplateId(), volume.getSize(), Volume.class.getName(), volume.getUuid(), vm.getId(), volume.isDisplay());
@@ -389,7 +397,7 @@ public VirtualMachine importVirtualMachineFromBackup(long zoneId, long domainId,
         VolumeVO restoredVolume = _volumeDao.findByUuid(location);
         if (restoredVolume != null) {
             try {
-                _volumeDao.attachVolume(restoredVolume.getId(), vm.getId(), getNextAvailableDeviceId(vmVolumes));
+                _volumeDao.attachVolume(restoredVolume.getId(), vm.getId(), getNextAvailableDeviceId(vm.getId(), vmVolumes));
                 restoredVolume.setState(Volume.State.Ready);
                 _volumeDao.update(restoredVolume.getId(), restoredVolume);
                 UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_ATTACH, restoredVolume.getAccountId(), restoredVolume.getDataCenterId(), restoredVolume.getId(), restoredVolume.getName(),

server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java

@@ -142,7 +142,7 @@ public class ConfigDriveNetworkElement extends AdapterBase implements NetworkEle
     @Inject
     private HypervisorGuruManager _hvGuruMgr;
 
-    private final static Integer CONFIGDRIVEDISKSEQ = 4;
+    public final static Integer CONFIGDRIVEDISKSEQ = 4;
 
     private boolean canHandle(TrafficType trafficType) {
         return trafficType.equals(TrafficType.Guest);

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

@@ -160,6 +160,7 @@
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.hypervisor.HypervisorCapabilitiesVO;
 import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao;
+import com.cloud.network.element.ConfigDriveNetworkElement;
 import com.cloud.offering.DiskOffering;
 import com.cloud.org.Cluster;
 import com.cloud.org.Grouping;
@@ -5042,8 +5043,10 @@ private Long getDeviceId(UserVmVO vm, Long deviceId) {
         int maxDevices = getMaxDataVolumesSupported(vm) + 2; // add 2 to consider devices root volume and cdrom
         int maxDeviceId = maxDevices - 1;
         List<VolumeVO> vols = _volsDao.findByInstance(vm.getId());
+        boolean vmHasConfigDrive = vmInstanceDetailsDao.findDetail(vm.getId(), VmDetailConstants.CONFIG_DRIVE_LOCATION) != null;
         if (deviceId != null) {
-            if (deviceId.longValue() < 0 || deviceId.longValue() > maxDeviceId || deviceId.longValue() == 3) {
+            if (deviceId.longValue() < 0 || deviceId.longValue() > maxDeviceId || deviceId.longValue() == 3
+                    || (vmHasConfigDrive && deviceId.longValue() == ConfigDriveNetworkElement.CONFIGDRIVEDISKSEQ)) {
                 throw new RuntimeException("deviceId should be 0,1,2,4-" + maxDeviceId);
             }
             for (VolumeVO vol : vols) {
@@ -5058,6 +5061,9 @@ private Long getDeviceId(UserVmVO vm, Long deviceId) {
                 devIds.add(String.valueOf(i));
             }
             devIds.remove("3");
+            if (vmHasConfigDrive) {
+                devIds.remove(ConfigDriveNetworkElement.CONFIGDRIVEDISKSEQ.toString());
+            }
             for (VolumeVO vol : vols) {
                 devIds.remove(vol.getDeviceId().toString().trim());
             }