Moti Asayag has posted comments on this change. Change subject: engine: errors in audit log- VDS_SET_NONOPERATIONAL_IFACE_DOWN ......................................................................
Patch Set 2: Code-Review-1 (6 comments) http://gerrit.ovirt.org/#/c/27720/2//COMMIT_MSG Commit Message: Line 18: Line 19: This patch fixes these errors and also slightly changes the format of the Line 20: error message. Line 21: Line 22: Since the bond status can be determined from the bond itself and shouldn't Does bond status reported since 3.0 ? Line 23: be calculated from the slaves status and most of the bugs were part of this Line 24: logic, this patch removes it. Line 25: Line 26: Change-Id: I9dfa5396c7622b5e689ee3e648fcccd0e75ed834 http://gerrit.ovirt.org/#/c/27720/2/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties File backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties: Line 467: interface/s whi i'm not a great fan of "interface/s" and "network/s" why not just interfaces and networks ? if concerned about singular vs plural, at worse we can duplicate the message or come up with a better phrasing. http://gerrit.ovirt.org/#/c/27720/2/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 29: brokenNicsToNetworks you can omit the type declaration on the right side. Line 41: } please add a space line :) http://gerrit.ovirt.org/#/c/27720/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 769: try { Line 770: String problematicNicsWithNetworksString = constructNicsWithNetworksString(problematicNicsWithNetworks); Line 771: Line 772: String message = Line 773: String.format( although not 100% related to this patch, but since you're already modifying it, please inline the message property and use log.infoFromat() instead formatting the 'message' string Line 774: "Host '%s' moved to Non-Operational state because interface/s which are down are needed by required network/s in the current cluster: '%s'", Line 775: _vds.getName(), Line 776: problematicNicsWithNetworksString); Line 777: Line 810: Line 811: String nicsWithNetworksString = nicsWithNetworksBuilder.toString(); Line 812: Line 813: return nicsWithNetworksBuilder.delete(nicsWithNetworksString.length() - 2, nicsWithNetworksString.length()) Line 814: .toString(); the following will get the same result with more readability IMO: List<String> reportedNics = new ArrayList<>(nicsWithNetworks.size()); for (Entry<String, Set<String>> nicToNetworks : nicsWithNetworks.entrySet()) { reportedNics.add(String.format("%s (%s)", nicToNetworks.getKey(), StringUtils.join(nicToNetworks.getValue(), ", "))); } return StringUtils.join(reportedNics, ", "); Line 815: } Line 816: Line 817: private void reportNicStatusChanges() { Line 818: List<VdsNetworkInterface> interfaces = _vds.getInterfaces(); -- 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: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@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
