Moti Asayag has posted comments on this change. Change subject: engine: event on interface status change ......................................................................
Patch Set 2: (11 comments) Please add a third message to distinct slaves status change: The RFE asks for indication of the slave status. I'd merge the interface and bond to the same message, but you may leave it as is. In addition, i think it is reasonable to ask for events to subscribe to for notifications via the engine-notifier.g http://gerrit.ovirt.org/#/c/26104/2/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties File backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties: Line 217: VDS_ACTIVATE_FAILED=Failed to activate Host ${VdsName}.(User: ${UserName}). Line 218: VDS_ACTIVATE_FAILED_ASYNC=Failed to autorecover Host ${VdsName}. Line 219: HOST_REFRESHED_CAPABILITIES=Successfully refreshed the capabilities of host ${VdsName}. Line 220: HOST_REFRESH_CAPABILITIES_FAILED=Failed to refresh the capabilities of host ${VdsName}. Line 221: HOST_IFACE_DOWN=Interface '${InterfaceName}' on host ${VdsName}, changed state to down s/HOST_IFACE_DOWN/HOST_INTERFACE_STATE_DOWN Line 222: HOST_IFACE_UP=Interface '${InterfaceName}' on host ${VdsName}, changed state to up Line 223: HOST_BOND_DOWN=Bond '${InterfaceName}' on host ${VdsName}, changed state to down Line 224: HOST_BOND_UP=Bond '${InterfaceName}' on host ${VdsName}, changed state to up Line 225: VDS_DETECTED=Status of host ${VdsName} was set to ${HostStatus}. Line 218: VDS_ACTIVATE_FAILED_ASYNC=Failed to autorecover Host ${VdsName}. Line 219: HOST_REFRESHED_CAPABILITIES=Successfully refreshed the capabilities of host ${VdsName}. Line 220: HOST_REFRESH_CAPABILITIES_FAILED=Failed to refresh the capabilities of host ${VdsName}. Line 221: HOST_IFACE_DOWN=Interface '${InterfaceName}' on host ${VdsName}, changed state to down Line 222: HOST_IFACE_UP=Interface '${InterfaceName}' on host ${VdsName}, changed state to up s/HOST_IFACE_UP/HOST_INTERFACE_STATE_UP Line 223: HOST_BOND_DOWN=Bond '${InterfaceName}' on host ${VdsName}, changed state to down Line 224: HOST_BOND_UP=Bond '${InterfaceName}' on host ${VdsName}, changed state to up Line 225: VDS_DETECTED=Status of host ${VdsName} was set to ${HostStatus}. Line 226: VDS_FAILURE=Host ${VdsName} is non responsive. Line 219: HOST_REFRESHED_CAPABILITIES=Successfully refreshed the capabilities of host ${VdsName}. Line 220: HOST_REFRESH_CAPABILITIES_FAILED=Failed to refresh the capabilities of host ${VdsName}. Line 221: HOST_IFACE_DOWN=Interface '${InterfaceName}' on host ${VdsName}, changed state to down Line 222: HOST_IFACE_UP=Interface '${InterfaceName}' on host ${VdsName}, changed state to up Line 223: HOST_BOND_DOWN=Bond '${InterfaceName}' on host ${VdsName}, changed state to down s/HOST_BOND_DOWN/HOST_BOND_STATE_DOWN Line 224: HOST_BOND_UP=Bond '${InterfaceName}' on host ${VdsName}, changed state to up Line 225: VDS_DETECTED=Status of host ${VdsName} was set to ${HostStatus}. Line 226: VDS_FAILURE=Host ${VdsName} is non responsive. Line 227: VDS_MAINTENANCE=Host ${VdsName} was switched to Maintenance Mode. Line 220: HOST_REFRESH_CAPABILITIES_FAILED=Failed to refresh the capabilities of host ${VdsName}. Line 221: HOST_IFACE_DOWN=Interface '${InterfaceName}' on host ${VdsName}, changed state to down Line 222: HOST_IFACE_UP=Interface '${InterfaceName}' on host ${VdsName}, changed state to up Line 223: HOST_BOND_DOWN=Bond '${InterfaceName}' on host ${VdsName}, changed state to down Line 224: HOST_BOND_UP=Bond '${InterfaceName}' on host ${VdsName}, changed state to up s/HOST_BOND_UP/HOST_BOND_STATE_UP Line 225: VDS_DETECTED=Status of host ${VdsName} was set to ${HostStatus}. Line 226: VDS_FAILURE=Host ${VdsName} is non responsive. Line 227: VDS_MAINTENANCE=Host ${VdsName} was switched to Maintenance Mode. Line 228: VDS_MAINTENANCE_MANUAL_HA=Host ${VdsName} was switched to Maintenance mode, but Hosted Engine HA maintenance could not be enabled. Please enable it manually. http://gerrit.ovirt.org/#/c/26104/2/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java: Line 755: Set<VdsNetworkInterface> slaves = new HashSet<>(); Line 756: Map<String, InterfaceStatus> monitoredInterfaces = new HashMap<String, InterfaceStatus>(); Line 757: Line 758: for (VdsNetworkInterface iface : interfaces) { Line 759: if (iface.getBondName() != null) { it would be nice to expose new method in VdsNetworkInterface named isSlave which will encapsulate this logic (and replace any other places which perform the same check) Line 760: slaves.add(iface); Line 761: } Line 762: Line 763: String parentIfaceName = Line 760: slaves.add(iface); Line 761: } Line 762: Line 763: String parentIfaceName = Line 764: iface.getVlanId() == null ? iface.getName() : NetworkUtils.stripVlan(iface.getName()); no need for null check - see the javadoc of NetworkUtils.stripVlan(String): // method return interface name without vlan: // input: eth0.5 output eth0 // input" eth0 output eth0 Line 765: Line 766: // If the parent interface already marked as monitored- no need to check it again Line 767: if (monitoredInterfaces.containsKey(parentIfaceName)) { Line 768: continue; Line 769: } Line 770: Line 771: // The status of the interface should be monitored only if it has networks attached to it or has labels Line 772: if (StringUtils.isNotEmpty(iface.getNetworkName()) Line 773: || (iface.getLabels() != null && !iface.getLabels().isEmpty())) { use NetworkUtils.isLabeled(iface) instead Line 774: VdsNetworkInterface parentIface = iface; Line 775: // If vlan find the parent interface Line 776: if (iface.getVlanId() != null) { Line 777: for (VdsNetworkInterface tmpIface : interfaces) { Line 773: || (iface.getLabels() != null && !iface.getLabels().isEmpty())) { Line 774: VdsNetworkInterface parentIface = iface; Line 775: // If vlan find the parent interface Line 776: if (iface.getVlanId() != null) { Line 777: for (VdsNetworkInterface tmpIface : interfaces) { please replace the for loop with Entities.entitiesByName(interfaces) that will be saved to a local variable outside of the outer loop. Line 778: if (parentIfaceName.equals(tmpIface.getName())) { Line 779: parentIface = tmpIface; Line 780: } Line 781: } Line 778: if (parentIfaceName.equals(tmpIface.getName())) { Line 779: parentIface = tmpIface; Line 780: } Line 781: } Line 782: } please add a space line after the parenthesis Line 783: monitoredInterfaces.put(parentIface.getName(), parentIface.getStatistics().getStatus()); Line 784: } Line 785: } Line 786: Line 779: parentIface = tmpIface; Line 780: } Line 781: } Line 782: } Line 783: monitoredInterfaces.put(parentIface.getName(), parentIface.getStatistics().getStatus()); why not to use parentIfaceName as the key ? Line 784: } Line 785: } Line 786: Line 787: // Slaves should be monitored if the bond is monitored Line 797: && oldIface.getStatistics().getStatus() != status) { Line 798: AuditLogableBase logable = new AuditLogableBase(_vds.getId()); Line 799: logable.setCustomId(oldIface.getName()); Line 800: logable.addCustomValue("InterfaceName", oldIface.getName()); Line 801: if (oldIface.getBonded() != null && oldIface.getBonded()) { you could use: if (Boolean.TRUE.equals(oldIface.getBonded())) { ... } Line 802: auditLog(logable, status == InterfaceStatus.UP ? AuditLogType.HOST_BOND_UP Line 803: : AuditLogType.HOST_BOND_DOWN); Line 804: } else { Line 805: auditLog(logable, status == InterfaceStatus.UP ? AuditLogType.HOST_IFACE_UP -- To view, visit http://gerrit.ovirt.org/26104 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I76953e4b52e35d2e18b6bc68051fa7dd6f3db129 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
