Martin Peřina has posted comments on this change. Change subject: backend: getLdapServers() use getCanonicalHostName ......................................................................
Patch Set 3: (3 comments) Maybe we should also test validness of ldap servers returned from DNS and not only those entered manually by -ldapServers. What do you think? .................................................... File backend/manager/tools/src/main/java/org/ovirt/engine/core/domains/ManageDomains.java Line 483: } Line 484: return result; Line 485: } Line 486: Line 487: for (String host: argValue.split(DOMAIN_SEPERATOR)) { Please, don't execute split method twice, convert entered LDAP server to List<String> before for cycle and use it in for cycle. Line 488: try { Line 489: InetAddress.getByName(host).getCanonicalHostName(); Line 490: } catch (UnknownHostException e) { Line 491: String warnMessage = "Warning: Cannot resolve FQDN: " + e.getMessage(); Line 485: } Line 486: Line 487: for (String host: argValue.split(DOMAIN_SEPERATOR)) { Line 488: try { Line 489: InetAddress.getByName(host).getCanonicalHostName(); Maybe I'm paranoid, but if ldap servers are entered as FQDNs, multiple IP addresses could be returned, so getAllByName would be better to test if all IP address are valid Line 490: } catch (UnknownHostException e) { Line 491: String warnMessage = "Warning: Cannot resolve FQDN: " + e.getMessage(); Line 492: log.warn(warnMessage); Line 493: System.out.println(warnMessage); Line 492: log.warn(warnMessage); Line 493: System.out.println(warnMessage); Line 494: } Line 495: } Line 496: return new ArrayList<String>(Arrays.asList(argValue.split(DOMAIN_SEPERATOR))); Please don't create new list instance twice: Arrays.asList creates new List instance, there's no need to pass it to another ArrayList constructor. Line 497: } Line 498: Line 499: public void addDomain(CLIParser parser) throws ManageDomainsResult { Line 500: String authMode = LdapAuthModeEnum.GSSAPI.name(); -- To view, visit http://gerrit.ovirt.org/19547 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibae81ccd6b2a402c856934839f1dbdad316bbc3d Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf <dougsl...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Douglas Schilling Landgraf <dougsl...@redhat.com> Gerrit-Reviewer: Eli Mesika <elimes...@gmail.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches