Juan Hernandez has posted comments on this change.
Change subject: bootstrap: send complete bootstrap from engine
......................................................................
Patch Set 9: (9 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsInstaller.java
Line 171: return initialCommand;
Line 172: }
Line 173:
Line 174: private boolean runBootstrapCommand(boolean doFinal) {
Line 175: Boolean fRes = false;
Is there any reason to use "Boolean" instead of plain "boolean".
Line 176: String command = _bootstrapCommand.replace("{RunFlag}",
doFinal ? "True" : "False");
Line 177: if (doFinal && !_rebootAfterInstallation) {
Line 178: command = command.replace(" -b ", " ");
Line 179: }
Line 204: }
Line 205: }
Line 206: }
Line 207:
Line 208: log.infoFormat(" RunScript ended:" + fRes.toString());
No need to call "toString()" here, the concatination operation already takes
care of that.
Line 209: return fRes;
Line 210: }
Line 211:
Line 212: private boolean isOverrideFirewallAllowed() {
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
Line 1416: "umask 0077 && " +
Line 1417: "rm -fr '/tmp/ovirt-{GUID}' && " +
Line 1418: "mkdir '/tmp/ovirt-{GUID}' && " +
Line 1419: "tar -C '/tmp/ovirt-{GUID}' --no-same-permissions -o -x && "
+
Line 1420: "/tmp/ovirt-{GUID}/setup -c
'ssl={server_SSL_enabled};management_port={management_port}' -O
'{OrganizationName}' -t {utc_time} {OverrideFirewall} -S {SSHKey} {EnginePort}
-b {virt-placeholder} {gluster-placeholder} {URL1} {URL1} {vds-server} {GUID}
{RunFlag}; " +
Can you split this long line?
Line 1421: "rm -fr '/tmp/ovirt-{GUID}'"
Line 1422: )
Line 1423: BootstrapCommand(373),
Line 1424:
....................................................
File backend/manager/modules/root/src/main/webapp/WEB-INF/web.xml
Line 105
Line 106
Line 107
Line 108
Line 109
Nice to see this go away :-) .
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/hostinstall/VdsInstallerSSH.java
Line 497: return ret;
Line 498: }
Line 499:
Line 500:
Line 501: static public String getEngineSSHKeyFingerprint() {
Typical order is "public static".
Line 502: String fingerprint = null;
Line 503:
Line 504: InputStream in = null;
Line 505: try {
Line 515: ks.getCertificate(
Line 516: Config.<String>GetValue(ConfigValues.CertAlias)
Line 517: ).getPublicKey(),
Line 518: "ovirt-engine"
Line 519: );
The "KeyStore.getCertificate()" method can return null if no such certificate
exits. Can you add a local variable, check for null and log/throw an
appropriate message?
Line 520: }
Line 521: catch (Exception e) {
Line 522: log.error(
Line 523: "Failed to send own public key from store",
Line 519: );
Line 520: }
Line 521: catch (Exception e) {
Line 522: log.error(
Line 523: "Failed to send own public key from store",
Can you add to this log message the absolute path of the keystore file and the
alias that triggered the failure? Something like this:
Failed to send own public key from store "/etc/pki/ovirt-engine/.keystore"
using alias "engine".
Line 524: e
Line 525: );
Line 526: }
Line 527: finally {
Line 529: try {
Line 530: in.close();
Line 531: }
Line 532: catch(IOException e) {
Line 533: log.error("Cannot close key store", e);
Can you add to the message the absolute path name of the file that can't be
closed?
Line 534: }
Line 535: }
Line 536: }
Line 537:
....................................................
File
backend/manager/modules/utils/src/test/java/org/ovirt/engine/core/utils/hostinstall/VdsInstallerSSHTest.java
Line 247: "rm -fr /tmp/vssh-%1$d"
Line 248: ),
Line 249: Thread.currentThread().getId()
Line 250: ),
Line 251: new
FileInputStream("src/test/resources/tarball.tar")
Where is this stream closed? If it isn't then this leaks one file descriptor.
Line 252: )
Line 253: );
Line 254: assertFalse(callbacks.connected);
Line 255: assertFalse(callbacks.endTransfer);
--
To view, visit http://gerrit.ovirt.org/6963
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6f4a09ca9e66f0c9f5f4f7b283a5f43986b7e603
Gerrit-PatchSet: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsl...@redhat.com>
Gerrit-Reviewer: Eyal Edri <ee...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Ofer Schreiber <oschr...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
