Yaniv Bronhaim has posted comments on this change.
Change subject: core: Fixes logging in SshSoftFencingCommand
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(2 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SshSoftFencingCommand.java
Line 74: sshClient.useDefaultKeyPair();
Line 75: sshClient.connect();
Line 76: sshClient.authenticate();
Line 77: cmdOut = new ByteArrayOutputStream();
Line 78: cmdErr = new ByteArrayOutputStream();
I would split it to two try scopes, one for the ssh connection and one for the
command, this can gives more detailed log, and you won't print null if the
exception was before you set cmdOut and cmdErr..
Line 79: sshClient.executeCommand(Config.<String>
GetValue(ConfigValues.SshSoftFencingCommand, version),
Line 80: null,
Line 81: cmdOut,
Line 82: cmdErr);
Line 83: log.infoFormat("SSH Soft Fencing command executed on host
{0}", getVds().getHostName());
Line 84: } catch (Exception ex) {
Line 85: log.errorFormat("SSH Soft Fencing command failed on host
{0}: {1}", getVds().getHostName(), ex);
Line 86: logCommandOutput(cmdOut, "SSH Soft Fencing command
standard output: {0}");
Line 87: logCommandOutput(cmdErr, "SSH Soft Fencing command error
output: {0}");
Too much for logging. just do:
log.errorFormat(""SSH Soft Fencing command failed on host {0}: {1}. stdout:
{2}, strerr: {3}", getVds().getHostName(), ex, cmdOut, cmdErr)
and i think that on error cmdErr is enough.. check if you really need all those
values, it might repeats itself
the logCommandOutput is redundant.
Line 88: result = false;
Line 89: } finally {
Line 90: closeSshConnection(sshClient);
Line 91: }
--
To view, visit http://gerrit.ovirt.org/16619
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6ea12003bd44f6500e3878176e98debd102da056
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Peřina <[email protected]>
Gerrit-Reviewer: Martin Peřina <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
