Alon Bar-Lev has posted comments on this change.
Change subject: bootstrap: detach OVirtUpgrader from VdsInstaller into
OVirtNodeUpgrade
......................................................................
Patch Set 10: (9 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallerMessages.java
Line 32: case INFO:
Line 33: logType = AuditLogType.VDS_INSTALL_IN_PROGRESS;
Line 34: log.infoFormat("Installation {0}: {1}",
_vds.gethost_name(), text);
Line 35: break;
Line 36: default:
What is wrong with the format? break terminate the case just like compound.
Why throw? this will be runtime error author will not see. In this case issue a
warning is much more visible.
Line 37: case WARNING:
Line 38: logable.setCustomId(_sequence++);
Line 39: logType = AuditLogType.VDS_INSTALL_IN_PROGRESS_WARNING;
Line 40: log.warnFormat("Installation {0}: {1}",
_vds.gethost_name(), text);
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/OVirtNodeUpgrade.java
Line 22:
Line 23: /**
Line 24: * ovirt-node upgrade.
Line 25: */
Line 26: public class OVirtNodeUpgrade implements SSHDialog.Sink, Runnable {
I prefer the 3rd...
Thread = new Thread(
new Runnable {
},
"MyThread"
);
But it adds extra complexity, but I don't really care, will do.
Line 27:
Line 28: private static final int BUFFER_SIZE = 10 * 1024;
Line 29: private static final int THREAD_JOIN_TIMEOUT = 20 * 1000;
Line 30:
Line 79: */
Line 80: @Override
Line 81: public void finalize() {
Line 82: close();
Line 83: }
It is not used. it is fallback. I don't see any reason why not to have the
fallback.
Line 84:
Line 85: /**
Line 86: * Free resources.
Line 87: */
Line 84:
Line 85: /**
Line 86: * Free resources.
Line 87: */
Line 88: public void close() {
There is, if the user forgets... he should not be punished.
Line 89: stop();
Line 90: if (_dialog != null) {
Line 91: _dialog.disconnect();
Line 92: _dialog = null;
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/EngineSSHDialog.java
Line 70:
Line 71: entry = (KeyStore.PrivateKeyEntry)ks.getEntry(
Line 72: alias,
Line 73: new KeyStore.PasswordProtection(
Line 74: password.toCharArray()
This is fanny, as you have the string anyway, you copy it to char array, clear
the char array (if actually done as optimization will remove the clear), and
then you end of with at least 5 copies of the password because of other copies
to construct the string that is passed as parameter.
I don't think it worth the effort, but I will do this as it is not that
important.
Line 75: )
Line 76: );
Line 77: }
Line 78: catch (Exception e) {
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ssh/SSHDialog.java
Line 103: */
Line 104: @Override
Line 105: public void finalize() {
Line 106: disconnect();
Line 107: }
The reasoning had not changed.
Line 108:
Line 109: /**
Line 110: * Get session public key.
Line 111: * @return public key or null.
Line 226: _port,
Line 227: _hardTimeout,
Line 228: _softTimeout
Line 229: )
Line 230: );
Using string buffer? not thanks.
Line 231:
Line 232: try {
Line 233: if (_client != null) {
Line 234: throw new IOException("Already connected");
Line 289: stdinList = new LinkedList<InputStream>();
Line 290: }
Line 291: else {
Line 292: stdinList = new
LinkedList<InputStream>(Arrays.asList(initial));
Line 293: }
Done
Line 294: stdinList.add(pinStdin);
Line 295:
Line 296: try {
Line 297: sink.setControl(
....................................................
File
backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/ssh/SSHDialogTest.java
Line 22: import javax.naming.TimeLimitExceededException;
Line 23:
Line 24: import static org.junit.Assert.assertEquals;
Line 25: import static org.junit.Assert.assertTrue;
Line 26: import static org.junit.Assume.assumeTrue;
OK, for the eclipse users sake :)
Line 27: import org.junit.After;
Line 28: import org.junit.AfterClass;
Line 29: import org.junit.Before;
Line 30: import org.junit.BeforeClass;
--
To view, visit http://gerrit.ovirt.org/9174
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iff19fdb9f717d424f23bc5d4e5a8df8fce8a58bf
Gerrit-PatchSet: 10
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Shireesh Anjal <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
