Shireesh Anjal has posted comments on this change.

Change subject: engine: Peer probe gluster servers on bootstrap
......................................................................


Patch Set 9: (5 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java
Line 223:                         }
Line 224:                     } catch (Exception e) {
Line 225:                         log.errorFormat("Could not peer probe the 
gluster server {0}. Error: {1}",
Line 226:                                 getVds().gethost_name(),
Line 227:                                 e.getMessage());
I would recommend to catch exception inside the glusterPeerProbe() method and 
return false from there, so that there is a single call to setNonOperational() 
here.
Line 228:                     }
Line 229:                 }
Line 230:             }
Line 231:         }


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GlusterServersListReturnForXmlRpc.java
Line 17:     private static final String UUID = "uuid";
Line 18: 
Line 19:     private static final String PEER_STATUS = "status";
Line 20: 
Line 21:     private List<GlusterServerInfo> servers;
I would recommend to initialize this with an empty list
Line 22: 
Line 23:     public List<GlusterServerInfo> getServers() {
Line 24:         return servers;
Line 25:     }


Line 23:     public List<GlusterServerInfo> getServers() {
Line 24:         return servers;
Line 25:     }
Line 26: 
Line 27:     public void setServer(List<GlusterServerInfo> servers) {
The setter is not really required.
Line 28:         this.servers = servers;
Line 29:     }
Line 30: 
Line 31:     @SuppressWarnings("unchecked")


Line 29:     }
Line 30: 
Line 31:     @SuppressWarnings("unchecked")
Line 32:     public GlusterServersListReturnForXmlRpc(Map<String, Object> 
innerMap) {
Line 33:         super(innerMap);
Do not try to parse the sever list if the command has failed (mStatus.code != 0)
Line 34:         List<GlusterServerInfo> glusterServers = new 
ArrayList<GlusterServerInfo>();
Line 35: 
Line 36:         Object[] serversArr = (Object[]) innerMap.get(GLUSTER_HOSTS);
Line 37: 


Line 31:     @SuppressWarnings("unchecked")
Line 32:     public GlusterServersListReturnForXmlRpc(Map<String, Object> 
innerMap) {
Line 33:         super(innerMap);
Line 34:         List<GlusterServerInfo> glusterServers = new 
ArrayList<GlusterServerInfo>();
Line 35: 
You can remove the setter, and directly populate this.servers here.
Line 36:         Object[] serversArr = (Object[]) innerMap.get(GLUSTER_HOSTS);
Line 37: 
Line 38:         if (serversArr != null) {
Line 39:             for (int i = 0; i < serversArr.length; i++) {


--
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: 9
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Selvasundaram <[email protected]>
Gerrit-Reviewer: Dhandapani Gopal <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Selvasundaram <[email protected]>
Gerrit-Reviewer: Shireesh Anjal <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to