DaanHoogland commented on a change in pull request #4228:
URL: https://github.com/apache/cloudstack/pull/4228#discussion_r462482006
##########
File path:
server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java
##########
@@ -348,11 +362,19 @@ private void setupAgentSecurity(final Connection
sshConnection, final String age
_hostDao.saveDetails(connectedHost);
return resources;
} catch (DiscoveredWithErrorException e) {
+ s_logger.error("Exception: "+ e.getMessage());
throw e;
} catch (Exception e) {
String msg = " can't setup agent, due to " + e.toString() + " - "
+ e.getMessage();
s_logger.warn(msg);
} finally {
+ // set guid to null once host is added
Review comment:
is this comment correct? initialGuid might have a value other than null.
##########
File path:
server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java
##########
@@ -348,11 +362,19 @@ private void setupAgentSecurity(final Connection
sshConnection, final String age
_hostDao.saveDetails(connectedHost);
return resources;
} catch (DiscoveredWithErrorException e) {
+ s_logger.error("Exception: "+ e.getMessage());
Review comment:
this is not really saying it. what did we fail at for what reason with
(and this part is there) what message.
##########
File path:
server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java
##########
@@ -348,11 +362,19 @@ private void setupAgentSecurity(final Connection
sshConnection, final String age
_hostDao.saveDetails(connectedHost);
return resources;
} catch (DiscoveredWithErrorException e) {
+ s_logger.error("Exception: "+ e.getMessage());
throw e;
} catch (Exception e) {
String msg = " can't setup agent, due to " + e.toString() + " - "
+ e.getMessage();
s_logger.warn(msg);
} finally {
+ // set guid to null once host is added
Review comment:
exactly. it is not necessarily null. I think you'd better remove this
comment, it is confusing.
##########
File path:
server/src/main/java/com/cloud/hypervisor/kvm/discoverer/LibvirtServerDiscoverer.java
##########
@@ -348,11 +362,19 @@ private void setupAgentSecurity(final Connection
sshConnection, final String age
_hostDao.saveDetails(connectedHost);
return resources;
} catch (DiscoveredWithErrorException e) {
+ s_logger.error("Exception: "+ e.getMessage());
Review comment:
ok, than let's at least make the text describing our course of action:
```suggestion
s_logger.error("DiscoveredWithErrorException caught and
rethrowing, message; " + e.getMessage());
```
----------------------------------------------------------------
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]
