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

Reply via email to