Juan Hernandez has posted comments on this change.
Change subject: bootstrap: send firewall rules without creating temp file
......................................................................
Patch Set 5: (4 inline comments)
I should have marked the "close" comments as "extra picky" as well.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsInstaller.java
Line 259: case UploadScript: {
Line 260: String path = Config.resolveBootstrapInstallerPath();
Line 261: _executionSucceded = _wrapper.sendFile(path,
_remoteBootstrapRunningScriptPath);
Line 262: if (isOverrideFirewallAllowed() && _executionSucceded) {
Line 263: String ipTableConfig = Config.<String>
GetValue(ConfigValues.IPTablesConfig);
Thanks.
Line 264: if (StringUtils.isNotEmpty(ipTableConfig)) {
Line 265: _executionSucceded =
uploadStringAsFile(ipTableConfig, remoteFwRulesFilePath);
Line 266: }
Line 267:
Line 336:
Line 337: try {
Line 338: _wrapper.executeCommand(
Line 339: String.format("cat > '%1$s'", remote),
Line 340: new ByteArrayInputStream(str.getBytes("UTF-8"))
You need to close any stream you open. It is true that the current
implementation of ByteArrayInputStream doesn't hold any resources, so nothing
bad will happen if you don't close it. But I think you should not let your
brain get used to not close streams.
Line 341: );
Line 342: isUploaded = true;
Line 343: }
Line 344: catch(Exception e) {
Line 346: "Error during create remote file '{1}' error: '{2}'",
Line 347: remoteFwRulesFilePath,
Line 348: ExceptionUtils.getMessage(e)
Line 349: );
Line 350: log.debug(e);
Ok, no need to change now.
Line 351: }
Line 352: return isUploaded;
Line 353: }
Line 354:
....................................................
File
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/hostinstall/VdsInstallerSSH.java
Line 416: return ret;
Line 417: }
Line 418:
Line 419: public final boolean executeCommand(String command) {
Line 420: return executeCommand(command, new ByteArrayInputStream(new
byte[0]));
It is just a bad habit in my opinion. If one day you change this to other type
of stream you will probably forget to add the code to close it.
Line 421: }
Line 422:
Line 423: public final boolean receiveFile(String source, String
destination) {
Line 424:
--
To view, visit http://gerrit.ovirt.org/7028
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I61b0ee4c62382807d14c5476baf2898a08ee5892
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Sharad Mishra <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
