Import Remote KVM VM logging improvements (#9284)

api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java

@@ -203,8 +203,8 @@ public class ImportUnmanagedInstanceCmd extends BaseAsyncCmd {
             for (Map<String, String> entry : (Collection<Map<String, String>>)nicNetworkList.values()) {
                 String nic = entry.get(VmDetailConstants.NIC);
                 String networkUuid = entry.get(VmDetailConstants.NETWORK);
-                if (LOGGER.isTraceEnabled()) {
+                if (LOGGER.isDebugEnabled()) {
-                    LOGGER.trace(String.format("nic, '%s', goes on net, '%s'", nic, networkUuid));
+                    LOGGER.debug(String.format("nic, '%s', goes on net, '%s'", nic, networkUuid));
                 }
                 if (StringUtils.isAnyEmpty(nic, networkUuid) || _entityMgr.findByUuid(Network.class, networkUuid) == null) {
                     throw new InvalidParameterValueException(String.format("Network ID: %s for NIC ID: %s is invalid", networkUuid, nic));
@@ -221,8 +221,8 @@ public class ImportUnmanagedInstanceCmd extends BaseAsyncCmd {
             for (Map<String, String> entry : (Collection<Map<String, String>>)nicIpAddressList.values()) {
                 String nic = entry.get(VmDetailConstants.NIC);
                 String ipAddress = StringUtils.defaultIfEmpty(entry.get(VmDetailConstants.IP4_ADDRESS), null);
-                if (LOGGER.isTraceEnabled()) {
+                if (LOGGER.isDebugEnabled()) {
-                    LOGGER.trace(String.format("nic, '%s', gets ip, '%s'", nic, ipAddress));
+                    LOGGER.debug(String.format("nic, '%s', gets ip, '%s'", nic, ipAddress));
                 }
                 if (StringUtils.isEmpty(nic)) {
                     throw new InvalidParameterValueException(String.format("NIC ID: '%s' is invalid for IP address mapping", nic));

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

@@ -17,7 +17,6 @@
 
 package com.cloud.agent.api;
 
-@LogLevel(LogLevel.Log4jLevel.Trace)
 public class CheckVolumeAnswer extends Answer {
 
     private long size;

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

@@ -21,7 +21,6 @@ package com.cloud.agent.api;
 
 import com.cloud.agent.api.to.StorageFilerTO;
 
-@LogLevel(LogLevel.Log4jLevel.Trace)
 public class CheckVolumeCommand extends Command {
 
     String srcFile;

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

@@ -17,7 +17,6 @@
 
 package com.cloud.agent.api;
 
-@LogLevel(LogLevel.Log4jLevel.Trace)
 public class CopyRemoteVolumeAnswer extends Answer {
 
     private String remoteIp;

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

@@ -21,10 +21,10 @@ package com.cloud.agent.api;
 
 import com.cloud.agent.api.to.StorageFilerTO;
 
-@LogLevel(LogLevel.Log4jLevel.Trace)
 public class CopyRemoteVolumeCommand extends Command {
     String remoteIp;
     String username;
+    @LogLevel(LogLevel.Log4jLevel.Off)
     String password;
     String srcFile;
     String tmpPath;

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

@@ -22,10 +22,10 @@ import org.apache.cloudstack.vm.UnmanagedInstanceTO;
 import java.util.HashMap;
 import java.util.List;
 
-@LogLevel(LogLevel.Log4jLevel.Trace)
 public class GetRemoteVmsAnswer extends Answer {
 
     private String remoteIp;
+    @LogLevel(LogLevel.Log4jLevel.Trace)
     private HashMap<String, UnmanagedInstanceTO> unmanagedInstances;
 
     List<String> vmNames;

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

@@ -19,11 +19,11 @@
 
 package com.cloud.agent.api;
 
-@LogLevel(LogLevel.Log4jLevel.Trace)
 public class GetRemoteVmsCommand extends Command {
 
     String remoteIp;
     String username;
+    @LogLevel(LogLevel.Log4jLevel.Off)
     String password;
 
     public GetRemoteVmsCommand(String remoteIp, String username, String password) {

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

@@ -21,10 +21,10 @@ import java.util.HashMap;
 
 import org.apache.cloudstack.vm.UnmanagedInstanceTO;
 
-@LogLevel(LogLevel.Log4jLevel.Trace)
 public class GetUnmanagedInstancesAnswer extends Answer {
 
     private String instanceName;
+    @LogLevel(LogLevel.Log4jLevel.Trace)
     private HashMap<String, UnmanagedInstanceTO> unmanagedInstances;
 
     GetUnmanagedInstancesAnswer() {

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

@@ -28,10 +28,10 @@ import org.apache.commons.collections.CollectionUtils;
  * All managed instances will be filtered while trying to find unmanaged instances.
  */
 
-@LogLevel(LogLevel.Log4jLevel.Trace)
 public class GetUnmanagedInstancesCommand extends Command {
 
     String instanceName;
+    @LogLevel(LogLevel.Log4jLevel.Trace)
     List<String> managedInstancesNames;
 
     public GetUnmanagedInstancesCommand() {

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

@@ -3771,14 +3771,14 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
     }
 
     public List<String> getAllVmNames(final Connect conn) {
-        final ArrayList<String> la = new ArrayList<String>();
+        final ArrayList<String> domainNames = new ArrayList<String>();
         try {
             final String names[] = conn.listDefinedDomains();
             for (int i = 0; i < names.length; i++) {
-                la.add(names[i]);
+                domainNames.add(names[i]);
             }
         } catch (final LibvirtException e) {
-            s_logger.warn("Failed to list Defined domains", e);
+            s_logger.warn("Failed to list defined domains", e);
         }
 
         int[] ids = null;
@@ -3786,14 +3786,14 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
             ids = conn.listDomains();
         } catch (final LibvirtException e) {
             s_logger.warn("Failed to list domains", e);
-            return la;
+            return domainNames;
         }
 
         Domain dm = null;
         for (int i = 0; i < ids.length; i++) {
             try {
                 dm = conn.domainLookupByID(ids[i]);
-                la.add(dm.getName());
+                domainNames.add(dm.getName());
             } catch (final LibvirtException e) {
                 s_logger.warn("Unable to get vms", e);
             } finally {
@@ -3807,7 +3807,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
             }
         }
 
-        return la;
+        return domainNames;
     }
 
     private HashMap<String, HostVmStateReportEntry> getHostVmStateReport() {

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

@@ -50,36 +50,37 @@ public final class LibvirtGetRemoteVmsCommandWrapper extends CommandWrapper<GetR
 
     @Override
     public Answer execute(final GetRemoteVmsCommand command, final LibvirtComputingResource libvirtComputingResource) {
-        String hypervisorURI = "qemu+tcp://" + command.getRemoteIp() + "/system";
+        String remoteIp = command.getRemoteIp();
+        String hypervisorURI = "qemu+tcp://" + remoteIp + "/system";
         HashMap<String, UnmanagedInstanceTO> unmanagedInstances = new HashMap<>();
         try {
             Connect conn = LibvirtConnection.getConnection(hypervisorURI);
             final List<String> allVmNames = libvirtComputingResource.getAllVmNames(conn);
+            s_logger.info(String.format("Found %d VMs on the remote host %s", allVmNames.size(), remoteIp));
             for (String name : allVmNames) {
                 final Domain domain = libvirtComputingResource.getDomain(conn, name);
-
                 final DomainInfo.DomainState ps = domain.getInfo().state;
                 final VirtualMachine.PowerState state = libvirtComputingResource.convertToPowerState(ps);
-                s_logger.debug("VM " + domain.getName() + " - powerstate: " + ps + ", state: " + state.toString());
+                s_logger.debug(String.format("Remote VM %s - powerstate: %s, state: %s", domain.getName(), ps.toString(), state.toString()));
 
                 if (state == VirtualMachine.PowerState.PowerOff) {
                     try {
                         UnmanagedInstanceTO instance = getUnmanagedInstance(libvirtComputingResource, domain, conn);
                         unmanagedInstances.put(instance.getName(), instance);
                     } catch (Exception e) {
-                        s_logger.error("Couldn't fetch VM " + domain.getName() + " details, due to: " + e.getMessage(), e);
+                        s_logger.error("Couldn't fetch remote VM " + domain.getName() + " details, due to: " + e.getMessage(), e);
                     }
                 }
                 domain.free();
             }
-            s_logger.debug("Found " + unmanagedInstances.size() + " stopped VMs on host " + command.getRemoteIp());
+            s_logger.debug("Found " + unmanagedInstances.size() + " stopped VMs on remote host " + remoteIp);
             return new GetRemoteVmsAnswer(command, "", unmanagedInstances);
         } catch (final LibvirtException e) {
-            s_logger.error("Failed to list stopped VMs on remote host " + command.getRemoteIp() + ", due to: " + e.getMessage(), e);
+            s_logger.error("Failed to list stopped VMs on remote host " + remoteIp + ", due to: " + e.getMessage(), e);
             if (e.getMessage().toLowerCase().contains("connection refused")) {
-                return new Answer(command, false, "Unable to connect to remote host " + command.getRemoteIp() + ", please check the libvirtd tcp connectivity and retry");
+                return new Answer(command, false, "Unable to connect to remote host " + remoteIp + ", please check the libvirtd tcp connectivity and retry");
             }
-            return new Answer(command, false, "Unable to list stopped VMs on remote host " + command.getRemoteIp() + ", due to: " + e.getMessage());
+            return new Answer(command, false, "Unable to list stopped VMs on remote host " + remoteIp + ", due to: " + e.getMessage());
         }
     }
 
@@ -105,8 +106,8 @@ public final class LibvirtGetRemoteVmsCommandWrapper extends CommandWrapper<GetR
 
             return instance;
         } catch (Exception e) {
-            s_logger.debug("Unable to retrieve unmanaged instance info,  due to: " + e.getMessage(), e);
+            s_logger.debug("Unable to retrieve remote unmanaged instance info,  due to: " + e.getMessage(), e);
-            throw new CloudRuntimeException("Unable to retrieve unmanaged instance info, due to: " + e.getMessage());
+            throw new CloudRuntimeException("Unable to retrieve remote unmanaged instance info, due to: " + e.getMessage());
         }
     }
 

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

@@ -48,7 +48,7 @@ public final class LibvirtGetUnmanagedInstancesCommandWrapper extends CommandWra
 
     @Override
     public GetUnmanagedInstancesAnswer execute(GetUnmanagedInstancesCommand command, LibvirtComputingResource libvirtComputingResource) {
-        LOGGER.info("Fetching unmanaged instance on host");
+        LOGGER.info("Fetching unmanaged instances on host");
 
         HashMap<String, UnmanagedInstanceTO> unmanagedInstances = new HashMap<>();
         try {

server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java

@@ -800,6 +800,7 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
         }
         int copyTimeoutInSecs = copyTimeout * 60;
         copyRemoteVolumeCommand.setWait(copyTimeoutInSecs);
+        LOGGER.error(String.format("Initiating copy remote volume %s from %s, timeout %d secs", path, remoteUrl, copyTimeoutInSecs));
         Answer answer = agentManager.easySend(dest.getHost().getId(), copyRemoteVolumeCommand);
         if (!(answer instanceof CopyRemoteVolumeAnswer)) {
             throw new CloudRuntimeException("Error while copying volume of remote instance: " + answer.getDetails());
@@ -2434,7 +2435,6 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
         return defaultNetwork;
     }
 
-    //generate unit test
     public ListResponse<UnmanagedInstanceResponse> listVmsForImport(ListVmsForImportCmd cmd) {
         final Account caller = CallContext.current().getCallingAccount();
         if (caller.getType() != Account.Type.ADMIN) {
@@ -2474,8 +2474,8 @@ public class UnmanagedVMsManagerImpl implements UnmanagedVMsManager {
     private HashMap<String, UnmanagedInstanceTO> getRemoteVmsOnKVMHost(long zoneId, String remoteHostUrl, String username, String password) {
         //ToDo: add option to list one Vm by name
         List<HostVO> hosts = resourceManager.listAllUpAndEnabledHostsInOneZoneByHypervisor(Hypervisor.HypervisorType.KVM, zoneId);
-        if(hosts.size() < 1) {
+        if (hosts.size() < 1) {
-            throw new CloudRuntimeException("No hosts available for VM import");
+            throw new CloudRuntimeException("No hosts available to list VMs on remote host " + remoteHostUrl);
         }
         HostVO host = hosts.get(0);
         GetRemoteVmsCommand getRemoteVmsCommand = new GetRemoteVmsCommand(remoteHostUrl, username, password);