[GitHub] [cloudstack] slavkap commented on a change in pull request #4773: Fix deploy VM from ISOs with UEFI

[GitBox <gi...@apache.org>]

slavkap commented on a change in pull request #4773:
URL: https://github.com/apache/cloudstack/pull/4773#discussion_r591454081



##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -2564,16 +2565,9 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {
             final DiskDef disk = new DiskDef();
             int devId = volume.getDiskSeq().intValue();
             if (volume.getType() == Volume.Type.ISO) {
-                if (volPath == null) {
-                    if (isSecureBoot) {
-                        disk.defISODisk(null, devId,isSecureBoot,isWindowsTemplate);
-                    } else {
-                        /* Add iso as placeholder */
-                        disk.defISODisk(null, devId);
-                    }
-                } else {
-                    disk.defISODisk(volPath, devId);
-                }
+
+                disk.defISODisk(volPath, devId, isUefiEnabled);

Review comment:
       @sureshanaparti here we check if UEFI is enabled because cdrom disk controller should be SATA for q35 machines




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org