Address review comments

Author: nvazquez

core/src/main/java/com/cloud/agent/api/CleanupConvertedInstanceDisksAnswer.java

@@ -51,7 +51,7 @@
 import java.util.UUID;
 import java.util.stream.Collectors;
 
-public abstract class LibvirtBaseConvertCommandWrapper <T extends Command, A extends Answer, R extends ServerResource> extends CommandWrapper<Command, Answer, ServerResource> {
+public abstract class LibvirtBaseConvertCommandWrapper <T extends Command, A extends Answer, R extends ServerResource> extends CommandWrapper<T, A, R> {
 
     protected KVMStoragePool getTemporaryStoragePool(DataStoreTO conversionTemporaryLocation, KVMStoragePoolManager storagePoolMgr) {
         if (conversionTemporaryLocation instanceof NfsTO) {
@@ -237,8 +237,10 @@ protected LibvirtDomainXMLParser parseMigratedVMXmlDomain(String installPath) th
             logger.error(err);
             throw new CloudRuntimeException(err);
         }
-        InputStream is = new BufferedInputStream(new FileInputStream(xmlPath));
-        String xml = IOUtils.toString(is, Charset.defaultCharset());
+        String xml;
+        try (InputStream is = new BufferedInputStream(new FileInputStream(xmlPath))) {
+            xml = IOUtils.toString(is, Charset.defaultCharset());
+        }
         final LibvirtDomainXMLParser parser = new LibvirtDomainXMLParser();
         try {
             parser.parseDomainXML(xml);

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtBaseConvertCommandWrapper.java

@@ -20,15 +20,13 @@
 
 import com.cloud.agent.api.Answer;
 import com.cloud.agent.api.CleanupConvertedInstanceDisksCommand;
-import com.cloud.agent.api.Command;
 import com.cloud.agent.api.to.DataStoreTO;
 import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
 import com.cloud.hypervisor.kvm.resource.LibvirtDomainXMLParser;
 import com.cloud.hypervisor.kvm.storage.KVMPhysicalDisk;
 import com.cloud.hypervisor.kvm.storage.KVMStoragePool;
 import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
 import com.cloud.resource.ResourceWrapper;
-import com.cloud.resource.ServerResource;
 
 import java.io.File;
 import java.util.List;
@@ -37,11 +35,9 @@
 public class LibvirtCleanupConvertedInstanceDisksCommandWrapper extends LibvirtBaseConvertCommandWrapper<CleanupConvertedInstanceDisksCommand, Answer, LibvirtComputingResource> {
 
     @Override
-    public Answer execute(Command command, ServerResource resource) {
-        CleanupConvertedInstanceDisksCommand cmd = (CleanupConvertedInstanceDisksCommand) command;
-        LibvirtComputingResource serverResource = (LibvirtComputingResource) resource;
-        DataStoreTO vmVolumesStore = cmd.getVmVolumesStore();
-        String vmVolumesPrefix = cmd.getVmVolumesPrefix();
+    public Answer execute(CleanupConvertedInstanceDisksCommand command, LibvirtComputingResource serverResource) {
+        DataStoreTO vmVolumesStore = command.getVmVolumesStore();
+        String vmVolumesPrefix = command.getVmVolumesPrefix();
 
         final KVMStoragePoolManager storagePoolMgr = serverResource.getStoragePoolMgr();
         KVMStoragePool conversionPool = getTemporaryStoragePool(vmVolumesStore, storagePoolMgr);
@@ -53,7 +49,7 @@ public Answer execute(Command command, ServerResource resource) {
             boolean xmlExists = new File(xmlPath).exists();
 
             LibvirtDomainXMLParser xmlParser = xmlExists ? parseMigratedVMXmlDomain(volumesBasePath) : null;
-            List<KVMPhysicalDisk> temporaryDisks = xmlExists ?
+            List<KVMPhysicalDisk> temporaryDisks = xmlExists && xmlParser != null ?
                     getTemporaryDisksFromParsedXml(conversionPool, xmlParser, volumesBasePath) :
                     getTemporaryDisksWithPrefixFromTemporaryPool(conversionPool, conversionPoolPath, vmVolumesPrefix);
 

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCleanupConvertedInstanceDisksCommandWrapper.java

@@ -20,8 +20,6 @@
 
 import java.util.List;
 
-import com.cloud.agent.api.Command;
-import com.cloud.resource.ServerResource;
 import org.apache.cloudstack.vm.UnmanagedInstanceTO;
 
 import com.cloud.agent.api.Answer;
@@ -42,9 +40,7 @@
 public class LibvirtImportConvertedInstanceCommandWrapper extends LibvirtBaseConvertCommandWrapper<ImportConvertedInstanceCommand, Answer, LibvirtComputingResource> {
 
     @Override
-    public Answer execute(Command command, ServerResource resource) {
-        ImportConvertedInstanceCommand cmd = (ImportConvertedInstanceCommand) command;
-        LibvirtComputingResource serverResource = (LibvirtComputingResource) resource;
+    public Answer execute(ImportConvertedInstanceCommand cmd, LibvirtComputingResource serverResource) {
         RemoteInstanceTO sourceInstance = cmd.getSourceInstance();
         Hypervisor.HypervisorType sourceHypervisorType = sourceInstance.getHypervisorType();
         String sourceInstanceName = sourceInstance.getInstanceName();

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtImportConvertedInstanceCommandWrapper.java

@@ -2248,7 +2248,11 @@ private UnmanagedInstanceTO convertAndImportToKVM(ConvertInstanceCommand convert
                 CleanupConvertedInstanceDisksCommand cleanupCommand =
                         new CleanupConvertedInstanceDisksCommand(temporaryConvertLocation, convertedDisksPrefix);
                 try {
-                    agentManager.send(convertHost.getId(), cleanupCommand);
+                    Answer cleanupAnswer = agentManager.send(convertHost.getId(), cleanupCommand);
+                    if (!cleanupAnswer.getResult()) {
+                        logger.warn("Failed to cleanup the converted disks for the VM {} through " +
+                                "the conversion host {}: {}", sourceVM, convertHost.getName(), cleanupAnswer.getDetails());
+                    }
                 } catch (AgentUnavailableException | OperationTimedoutException e) {
                     logger.error("Error cleaning up converted disks for VM {} through the conversion host {}",
                             sourceVM, convertHost.getName(), e);