Moti Asayag has posted comments on this change.

Change subject: engine: Refresh gluster data periodically
......................................................................


Patch Set 14: (4 inline comments)

....................................................
File backend/manager/dbscripts/network_sp.sql
Line 310:    AS $procedure$
Line 311: BEGIN
Line 312:    RETURN QUERY SELECT *
Line 313:    FROM vds_interface_view
Line 314:    WHERE addr = v_addr;
The where clause will filter hosts with ip address from all of the DCs in the 
system. Same address can be shared between more than one DC or even on same 
cluster (if boot protocol is static and ip addr is configured by the 
administrator).

In that case the list will include several hosts nics that weren't planned to 
appear in this list.
Line 315: 
Line 316: END; $procedure$
Line 317: LANGUAGE plpgsql;
Line 318: 


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterManager.java
Line 259: 
Line 260:     private List<String> getVdsIps(VDS vds) {
Line 261:         List<String> vdsIps = new ArrayList<String>();
Line 262:         for (VdsNetworkInterface iface : 
getInterfaceDao().getAllInterfacesForVds(vds.getId())) {
Line 263:             vdsIps.add(iface.getAddress());
iface.getAddress() might be null, therefore the list will include null element. 
It may result within a false positive in the comparison below (in fetchServers)

therefore the list should be filtered out null elements.
Line 264:         }
Line 265:         return vdsIps;
Line 266:     }
Line 267: 


....................................................
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/InterfaceDAODbFacadeImpl.java
Line 130:         MapSqlParameterSource parameterSource = 
getCustomMapSqlParameterSource()
Line 131:                 .addValue("addr", ipAddress);
Line 132: 
Line 133:         return 
getCallsHandler().executeReadList("Getinterface_viewByAddr",
Line 134:                 vdsNetworkInterfaceRowMapper, parameterSource);
this new sp should be followed with a unit test
Line 135:     }
Line 136: 
Line 137:     @SuppressWarnings("unchecked")
Line 138:     @Override


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/gluster/GlusterVolumesListReturnForXmlRpc.java
Line 170:         if(servers.size() > 0) {
Line 171:             return getServerOfCluster(clusterId, servers);
Line 172:         }
Line 173: 
Line 174:         List<VdsNetworkInterface> ifaces = 
getInterfaceDao().getAllInterfacesWithIpAddress(hostnameOrIp);
please see consideration about the return value of 
getAllInterfacesWithIpAddress in InterfaceDao.
Line 175:         if(ifaces.size() > 0) {
Line 176:             for(VdsNetworkInterface iface : ifaces) {
Line 177:                 VDS server = getVdsDao().get(iface.getVdsId());
Line 178:                 if(server.getvds_group_id().equals(clusterId)) {


--
To view, visit http://gerrit.ovirt.org/7288
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6b61eb6e93105e46e2706eac1d94bc10717224c2
Gerrit-PatchSet: 14
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shireesh Anjal <san...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@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