GutoVeronezi commented on a change in pull request #4575:
URL: https://github.com/apache/cloudstack/pull/4575#discussion_r626882785
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2105,7 +2106,7 @@ private Host createHostAndAgent(final ServerResource
resource, final Map<String,
host = findHostByGuid(firstCmd.getGuidWithoutResource());
}
if (host != null && host.getRemoved() == null) { // host
already added, no need to add again
- s_logger.debug("Found the host " + host.getId() + " by
guid: " + firstCmd.getGuid() + ", old host reconnected as new");
+ s_logger.debug(String.format("Found host [id: %d, name:
%s] by guid: %s, old host reconnected as new", host.getId(), host.getName(),
firstCmd.getGuid()));
Review comment:
@GabrielBrascher We can log the entire object here, as it has
`toString()` implemented.
##########
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4339,13 +4343,13 @@ private void orchestrateMigrateForScale(final String
vmUuid, final long srcHostI
try {
_agentMgr.send(srcHostId, new
Commands(cleanup(vm.getInstanceName())), null);
} catch (final AgentUnavailableException e) {
- s_logger.error("AgentUnavailableException while
cleanup on source host: " + srcHostId);
+
s_logger.error(String.format("AgentUnavailableException while cleanup on source
host: %s", srcHost));
Review comment:
@GabrielBrascher We can pass the exception as param to `s_logger.error`
here.
##########
File path: server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
##########
@@ -2178,7 +2179,7 @@ private Host createHostAndAgentDeferred(final
ServerResource resource, final Map
// added, no
// need to add
// again
- s_logger.debug("Found the host " + host.getId() + " by
guid: " + firstCmd.getGuid() + ", old host reconnected as new");
+ s_logger.debug(String.format("Found host [id: %d, name:
%s] by guid %s, old host reconnected as new", host.getId(), host.getName(),
firstCmd.getGuid()));
Review comment:
@GabrielBrascher We can log the entire object here, as it has toString()
implemented.
##########
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4219,14 +4219,18 @@ private void orchestrateMigrateForScale(final String
vmUuid, final long srcHostI
vm.getServiceOfferingId();
final long dstHostId = dest.getHost().getId();
final Host fromHost = _hostDao.findById(srcHostId);
+ Host srcHost = _hostDao.findById(srcHostId);
if (fromHost == null) {
- s_logger.info("Unable to find the host to migrate from: " +
srcHostId);
- throw new CloudRuntimeException("Unable to find the host to
migrate from: " + srcHostId);
+ String logMessageUnableToFindHost = String.format("Unable to find
host to migrate from %s.", srcHost);
+ s_logger.info(logMessageUnableToFindHost);
+ throw new CloudRuntimeException(logMessageUnableToFindHost);
}
+ Host dstHost = _hostDao.findById(dstHostId);
if (fromHost.getClusterId().longValue() != dest.getCluster().getId()) {
Review comment:
@GabrielBrascher As this class has already been touched, what do you
think of doing a few refactors?
Like removing the explicit unboxing...
--
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]
