Yevgeny Zaspitsky has posted comments on this change.

Change subject: engine: errors in audit log- VDS_SET_NONOPERATIONAL_IFACE_DOWN
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.ovirt.org/#/c/27720/1/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/NetworkMonitoringHelper.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/NetworkMonitoringHelper.java:

Line 23:      *            The cluster's networks.
Line 24:      * @return A pair of a list of the names of the NICs which are 
problematic, and a list of the names of the networks
Line 25:      *         which are required by these NICs.
Line 26:      */
Line 27:     public static Pair<Set<String>, Set<String>> 
determineProblematicNics(List<VdsNetworkInterface> interfaces,
Is order important here? Should the 2 collections correlate each to another?
Line 28:             List<Network> clusterNetworks) {
Line 29:         Set<String> networks = new HashSet<String>();
Line 30:         Set<String> brokenNics = new HashSet<String>();
Line 31: 


Line 50:     private static boolean isRequiredInterfaceDown(Map<String, 
Network> networksByName,
Line 51:             List<VdsNetworkInterface> interfaces,
Line 52:             VdsNetworkInterface iface) {
Line 53:         if (iface.getNetworkName() != null &&
Line 54:                 !isBaseInterfaceUp(iface, interfaces)
After the change isBaseInterfaceUp->isBaseInterfaceDown, please update the 
condition accordingly.
Line 55:                 && networksByName.containsKey(iface.getNetworkName())) 
{
Line 56: 
Line 57:             Network net = networksByName.get(iface.getNetworkName());
Line 58:             if (net.getCluster().getStatus() == 
NetworkStatus.OPERATIONAL && net.getCluster().isRequired()) {


Line 54:                 !isBaseInterfaceUp(iface, interfaces)
Line 55:                 && networksByName.containsKey(iface.getNetworkName())) 
{
Line 56: 
Line 57:             Network net = networksByName.get(iface.getNetworkName());
Line 58:             if (net.getCluster().getStatus() == 
NetworkStatus.OPERATIONAL && net.getCluster().isRequired()) {
As was discussed, please add check net != null and remove 
"networksByName.containsKey(iface.getNetworkName())"
Line 59:                 return true;
Line 60:             }
Line 61:         }
Line 62:         return false;


Line 61:         }
Line 62:         return false;
Line 63:     }
Line 64: 
Line 65:     private static boolean isBaseInterfaceUp(VdsNetworkInterface 
iface, List<VdsNetworkInterface> interfaces) {
Please change the method to isBaseInterfaceDown, as that is really needed here.
Line 66:         if (!NetworkUtils.isVlan(iface)) {
Line 67:             return iface.getStatistics().getStatus() == 
InterfaceStatus.UP;
Line 68:         }
Line 69: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9dfa5396c7622b5e689ee3e648fcccd0e75ed834
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Yevgeny Zaspitsky <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to