Juan Hernandez has posted comments on this change. Change subject: tools: engine-config bug fixes ......................................................................
Patch Set 3: (3 inline comments) .................................................... File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/kerberos/ConfigurationProvider.java Line 67: return fileName; I think it is better if you return the File object here instead of the String, that way the caller can use "getAbsolutePath()" to avoid problems if the path is not absolute. Also, will this create a world readable file? It does by default. In java 6 you will be forced to use Runtime.getRuntime().exec("/usr/bin/chmod", ...) to do it. In Java 7 you can use java.nio.file.Files.createTempFile(...). Line 81: " --admin-pass-file " + fileName) If the file name is not absolute this will work/fail depending on what is the current working directory of the tool. Should be use explicitly an absolute path. Line 82: + " -p " + engineConfigProperties); Same for the config, although I think it is an absolute path already. -- To view, visit http://gerrit.ovirt.org/6096 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3003f20a51bb5a62387593d914894d62b012c70b Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches