diff --git a/api/src/main/java/com/cloud/vm/VmDetailConstants.java b/api/src/main/java/com/cloud/vm/VmDetailConstants.java index f7d87ab18f1e..f237e04d8e16 100644 --- a/api/src/main/java/com/cloud/vm/VmDetailConstants.java +++ b/api/src/main/java/com/cloud/vm/VmDetailConstants.java @@ -53,6 +53,9 @@ public interface VmDetailConstants { String NIC_MULTIQUEUE_NUMBER = "nic.multiqueue.number"; String NIC_PACKED_VIRTQUEUES_ENABLED = "nic.packed.virtqueues.enabled"; + // KVM specific, disk controllers + String KVM_WIN_SKIP_FORCE_DISK_CONTROLLER = "win.skip.force.disk.controller"; + // Mac OSX guest specific (internal) String SMC_PRESENT = "smc.present"; String FIRMWARE = "firmware"; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index b43c7dced0d8..b42f9a9d01b1 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -100,7 +100,6 @@ import org.xml.sax.InputSource; import org.xml.sax.SAXException; - import com.cloud.agent.api.Answer; import com.cloud.agent.api.Command; import com.cloud.agent.api.HostVmStateReportEntry; @@ -180,8 +179,8 @@ import com.cloud.network.Networks.RouterPrivateIpStrategy; import com.cloud.network.Networks.TrafficType; import com.cloud.resource.AgentStatusUpdater; -import com.cloud.resource.ResourceStatusUpdater; import com.cloud.resource.RequestWrapper; +import com.cloud.resource.ResourceStatusUpdater; import com.cloud.resource.ServerResource; import com.cloud.resource.ServerResourceBase; import com.cloud.storage.JavaStorageLayer; @@ -3010,6 +3009,42 @@ public static DiskDef.DiskType getDiskType(KVMPhysicalDisk physicalDisk) { return useBLOCKDiskType(physicalDisk) ? DiskDef.DiskType.BLOCK : DiskDef.DiskType.FILE; } + /** + * Defines the disk configuration for the default pool type based on the provided parameters. + * It determines the appropriate disk settings depending on whether the disk is a data disk, whether + * it's a Windows template, whether UEFI is enabled, and whether secure boot is active. + * + * @param disk The disk definition object that will be configured with the disk settings. + * @param volume The volume (disk) object, containing information about the type of disk. + * @param isWindowsTemplate Flag indicating whether the template is a Windows template. + * @param isUefiEnabled Flag indicating whether UEFI is enabled. + * @param isSecureBoot Flag indicating whether secure boot is enabled. + * @param physicalDisk The physical disk object that contains the path to the disk. + * @param devId The device ID for the disk. + * @param diskBusType The disk bus type to use if not skipping force disk controller. + * @param diskBusTypeData The disk bus type to use for data disks, if applicable. + * @param details A map of VM details containing additional configuration values, such as whether to skip force + * disk controller. + */ + protected void defineDiskForDefaultPoolType(DiskDef disk, DiskTO volume, boolean isWindowsTemplate, + boolean isUefiEnabled, boolean isSecureBoot, KVMPhysicalDisk physicalDisk, int devId, + DiskDef.DiskBus diskBusType, DiskDef.DiskBus diskBusTypeData, Map details) { + boolean skipForceDiskController = BooleanUtils.toBoolean(details.get( + VmDetailConstants.KVM_WIN_SKIP_FORCE_DISK_CONTROLLER)); + boolean isDataDiskWithoutForceController = volume.getType() == Volume.Type.DATADISK && + (!(isWindowsTemplate && isUefiEnabled) || skipForceDiskController); + + if (isDataDiskWithoutForceController) { + disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusTypeData, DiskDef.DiskFmtType.QCOW2); + } else { + if (isSecureBoot) { + disk.defFileBasedDisk(physicalDisk.getPath(), devId, DiskDef.DiskFmtType.QCOW2, isWindowsTemplate); + } else { + disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2); + } + } + } + public void createVbd(final Connect conn, final VirtualMachineTO vmSpec, final String vmName, final LibvirtVMDef vm) throws InternalErrorException, LibvirtException, URISyntaxException { final Map details = vmSpec.getDetails(); final List disks = Arrays.asList(vmSpec.getDisks()); @@ -3161,18 +3196,9 @@ public int compare(final DiskTO arg0, final DiskTO arg1) { disk.setDiscard(DiscardType.UNMAP); } } else { - if (volume.getType() == Volume.Type.DATADISK && !(isWindowsTemplate && isUefiEnabled)) { - disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusTypeData, DiskDef.DiskFmtType.QCOW2); - } else { - if (isSecureBoot) { - disk.defFileBasedDisk(physicalDisk.getPath(), devId, DiskDef.DiskFmtType.QCOW2, isWindowsTemplate); - } else { - disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2); - } - } - + defineDiskForDefaultPoolType(disk, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot, + physicalDisk,devId, diskBusType, diskBusTypeData, details); } - } if (data instanceof VolumeObjectTO) { diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index 9ef96deb6915..561aa6a6e725 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -56,9 +56,6 @@ import javax.xml.xpath.XPathExpressionException; import javax.xml.xpath.XPathFactory; -import com.cloud.utils.net.NetUtils; - -import com.cloud.vm.VmDetailConstants; import org.apache.cloudstack.api.ApiConstants.IoDriverPolicy; import org.apache.cloudstack.storage.command.AttachAnswer; import org.apache.cloudstack.storage.command.AttachCommand; @@ -217,13 +214,15 @@ import com.cloud.template.VirtualMachineTemplate.BootloaderType; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.script.Script; +import com.cloud.utils.net.NetUtils; import com.cloud.utils.script.OutputInterpreter.OneLineParser; +import com.cloud.utils.script.Script; import com.cloud.utils.ssh.SshHelper; import com.cloud.vm.DiskProfile; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachine.PowerState; import com.cloud.vm.VirtualMachine.Type; +import com.cloud.vm.VmDetailConstants; @RunWith(MockitoJUnitRunner.class) public class LibvirtComputingResourceTest { @@ -240,6 +239,19 @@ public class LibvirtComputingResourceTest { Connect connMock; @Mock LibvirtDomainXMLParser parserMock; + @Mock + private DiskDef diskDef; + @Mock + private DiskTO volume; + @Mock + private KVMPhysicalDisk physicalDisk; + @Mock + private Map details; + + private static final String PHYSICAL_DISK_PATH = "/path/to/disk"; + private static final int DEV_ID = 1; + private static final DiskDef.DiskBus DISK_BUS_TYPE = DiskDef.DiskBus.VIRTIO; + private static final DiskDef.DiskBus DISK_BUS_TYPE_DATA = DiskDef.DiskBus.SCSI; @Spy private LibvirtComputingResource libvirtComputingResourceSpy = Mockito.spy(new LibvirtComputingResource()); @@ -6421,4 +6433,68 @@ public void testGetDiskModelFromVMDetailVirtioBlk() { DiskDef.DiskBus diskBus = libvirtComputingResourceSpy.getDiskModelFromVMDetail(virtualMachineTO); assertEquals(DiskDef.DiskBus.VIRTIOBLK, diskBus); } + + @Test + public void testDefineDiskForDefaultPoolType_DataDiskNonWinSkipForceController() { + when(volume.getType()).thenReturn(Volume.Type.DATADISK); + when(details.get(VmDetailConstants.KVM_WIN_SKIP_FORCE_DISK_CONTROLLER)).thenReturn("true"); + when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH); + boolean isWindowsTemplate = false; + boolean isUefiEnabled = false; + boolean isSecureBoot = false; + libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot, + physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details); + verify(diskDef, times(1)).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2); + } + + @Test + public void testDefineDiskForDefaultPoolType_DataDiskWinSecuredSkipForceController() { + when(volume.getType()).thenReturn(Volume.Type.DATADISK); + when(details.get(VmDetailConstants.KVM_WIN_SKIP_FORCE_DISK_CONTROLLER)).thenReturn("true"); + when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH); + boolean isWindowsTemplate = true; + boolean isUefiEnabled = true; + boolean isSecureBoot = true; + libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot, + physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details); + verify(diskDef, times(1)).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2); + } + + @Test + public void testDefineDiskForDefaultPoolType_WinSecureBootEnabledForced() { + when(volume.getType()).thenReturn(Volume.Type.DATADISK); + when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH); + boolean isWindowsTemplate = true; + boolean isUefiEnabled = true; + boolean isSecureBoot = true; + libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot, + physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details); + verify(diskDef, times(1)).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DiskDef.DiskFmtType.QCOW2, isWindowsTemplate); + } + + @Test + public void testDefineDiskForDefaultPoolType_SecureBootDisabledForced() { + when(volume.getType()).thenReturn(Volume.Type.DATADISK); + when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH); + boolean isWindowsTemplate = true; + boolean isUefiEnabled = false; + boolean isSecureBoot = false; + libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot, + physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details); + verify(diskDef, times(1)).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2); + } + + @Test + public void testDefineDiskForDefaultPoolType_NotDataDisk() { + when(volume.getType()).thenReturn(Volume.Type.ROOT); + when(details.get(VmDetailConstants.KVM_WIN_SKIP_FORCE_DISK_CONTROLLER)).thenReturn("false"); + when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH); + boolean isWindowsTemplate = false; + boolean isUefiEnabled = false; + boolean isSecureBoot = false; + libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot, + physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details); + verify(diskDef, never()).defFileBasedDisk(PHYSICAL_DISK_PATH, PHYSICAL_DISK_PATH, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2); + verify(diskDef, times(1)).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE, DiskDef.DiskFmtType.QCOW2); + } }