rhtyd commented on a change in pull request #4548:
URL: https://github.com/apache/cloudstack/pull/4548#discussion_r547206184
##########
File path:
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -7239,7 +7239,9 @@ private static File fetchSystemVmKeyFile() {
instanceDisk.setController(DiskControllerType.getType(device.getClass().getSimpleName()).toString());
instanceDisk.setControllerUnit(((VirtualSCSIController) device).getBusNumber());
} else {
-
instanceDisk.setController(DiskControllerType.none.toString());
+ String msg = String.format("Controller of
disk %s is unsupported", instanceDisk.getLabel());
+ s_logger.error(msg);
+ throw new Exception(msg);
Review comment:
@DK101010 I think on failure you can decide to fail the entire VM
ingestion by (a) returning an empty list and let the user of the
`getUnmanageInstanceDisks` method handle the case (when disk list is empty
throw an exception), or (b) in the generic exception handler, instead of
Exception throw an `UnsupportedOperationException` add another handler for the
specific exception. cc @shwstppr
----------------------------------------------------------------
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:
[email protected]
