Shireesh Anjal has posted comments on this change. Change subject: engine: check and peer probe gluster servers ......................................................................
Patch Set 4: (4 inline comments) Few in-line comments. .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java Line 192: if (vdsList != null && vdsList.size() > 1) { Line 193: VDS upServer = null; Line 194: for (VDS vds : vdsList) { Line 195: if (!Guid.OpEquality(getVdsId(), vds.getId()) Line 196: && vds.getstatus() == VDSStatus.Up) { Since vdsList contains only UP servers, there is no need to check the status here. Line 197: upServer = vds; Line 198: break; Line 199: } Line 200: } Line 202: // If new server is not part of the existing gluster peers, add into peer group Line 203: if (upServer != null) { Line 204: if (!hostExists(getGlusterPeers(upServer.getId()), getVds())) { Line 205: String newServerName = Line 206: getVds().gethost_name().isEmpty() ? getVds().getManagmentIp() : getVds().gethost_name(); can gethost_name() return null? If yes, this line could potentially throw a NullPointerException Line 207: glusterPeerProbe(upServer.getId(), newServerName); Line 208: } Line 209: } Line 210: } Line 211: } Line 212: Line 213: private boolean hostExists(List<GlusterServerInfo> glusterServers, VDS server) { Line 214: for (GlusterServerInfo glusterServer : glusterServers) { Line 215: if (glusterServer.getHostName().equals(server.gethost_name()) I think getHostName() in GlusterServerInfo has been changed to getHostNameOrIp() in the corresponding patch. So a rebase is required. Line 216: || glusterServer.getHostName().equals(server.getManagmentIp())) { Line 217: return true; Line 218: } Line 219: } Line 222: Line 223: @SuppressWarnings("unchecked") Line 224: private List<GlusterServerInfo> getGlusterPeers(Guid upServerId) { Line 225: List<GlusterServerInfo> glusterServers = new ArrayList<GlusterServerInfo>(); Line 226: VDSReturnValue returnValue = Backend.getInstance() Now you can directly use runVdsCommand() instead of Backend.getInstance().getResourceManager().RunVdsCommand() Line 227: .getResourceManager() Line 228: .RunVdsCommand(VDSCommandType.GlusterServersList, Line 229: new VdsIdVDSCommandParametersBase(upServerId)); Line 230: -- To view, visit http://gerrit.ovirt.org/7243 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I13c8de35ac596d89d2a9c631c4a8ef26996ea860 Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Selvasundaram <sesub...@redhat.com> Gerrit-Reviewer: Dhandapani Gopal <dgo...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Selvasundaram <sesub...@redhat.com> Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches