winterhazel commented on code in PR #10454:
URL: https://github.com/apache/cloudstack/pull/10454#discussion_r2241339652


##########
plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java:
##########
@@ -2101,18 +2101,11 @@ private Answer attachVolume(Command cmd, DiskTO disk, 
boolean isAttach, boolean
             AttachAnswer answer = new AttachAnswer(disk);
 
             if (isAttach) {
-                String rootDiskControllerDetail = 
DiskControllerType.ide.toString();
-                if (controllerInfo != null && 
StringUtils.isNotEmpty(controllerInfo.get(VmDetailConstants.ROOT_DISK_CONTROLLER)))
 {
-                    rootDiskControllerDetail = 
controllerInfo.get(VmDetailConstants.ROOT_DISK_CONTROLLER);
-                }
-                String dataDiskControllerDetail = 
getLegacyVmDataDiskController();
-                if (controllerInfo != null && 
StringUtils.isNotEmpty(controllerInfo.get(VmDetailConstants.DATA_DISK_CONTROLLER)))
 {
-                    dataDiskControllerDetail = 
controllerInfo.get(VmDetailConstants.DATA_DISK_CONTROLLER);
-                }
-
-                
VmwareHelper.validateDiskControllerDetails(rootDiskControllerDetail, 
dataDiskControllerDetail);
-                Pair<String, String> chosenDiskControllers = 
VmwareHelper.chooseRequiredDiskControllers(new Pair<>(rootDiskControllerDetail, 
dataDiskControllerDetail), vmMo, null, null);
-                String diskController = 
VmwareHelper.getControllerBasedOnDiskType(chosenDiskControllers, disk);
+                String rootDiskControllerDetail = 
controllerInfo.get(VmDetailConstants.ROOT_DISK_CONTROLLER);
+                String dataDiskControllerDetail = 
controllerInfo.get(VmDetailConstants.DATA_DISK_CONTROLLER);

Review Comment:
   `controllerInfo` can not be `null` here. It is always defined when creating 
the `AttachComand` in 
`com.cloud.storage.VolumeApiServiceImpl#sendAttachVolumeCommand`.
   
   As for the keys, if they are not present, 
`com.cloud.hypervisor.vmware.util.VmwareHelper#getDiskControllersFromVmSettings`
 will just return default values.
   
   I will add a comment with this information. I do not think that creating a 
new method is worth here.



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to