Alon Bar-Lev has posted comments on this change. Change subject: notifier: modify the oid schema for SNMP notification. ......................................................................
Patch Set 5: (3 comments) usually, minor comments. are you sure you do not need to define the enterprise specific # for snmpv1 within the mib if you want to achieve backward compat? http://gerrit.ovirt.org/#/c/32951/5/backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/transport/snmp/Snmp.java File backend/manager/tools/src/main/java/org/ovirt/engine/core/notifier/transport/snmp/Snmp.java: Line 200: addString(v2pdu, auditObjects, STORAGE_DOMAIN_NAME, event.getStorageDomainName(), false); Line 201: addUuid(v2pdu, auditObjects, STORAGE_DOMAIN_ID, event.getStorageDomainId()); Line 202: } Line 203: Line 204: private void addString(PDU v2pdu, OID prefix,int suffix, String val, boolean allowEmpty) { String val -> Object val and call val.toString()? Line 205: if (!StringUtils.isEmpty(val)){ Line 206: v2pdu.add(new VariableBinding(new OID(prefix).append(suffix), new OctetString(val))); Line 207: } else { Line 208: if (allowEmpty) { Line 206: v2pdu.add(new VariableBinding(new OID(prefix).append(suffix), new OctetString(val))); Line 207: } else { Line 208: if (allowEmpty) { Line 209: v2pdu.add(new VariableBinding(new OID(prefix).append(suffix), new OctetString())); Line 210: } not sure what is the difference between new OctetString() and new OctetString("") if there is no difference then, the above can be shorter? if (allowEmpty || !StringUtils.isEmpty(val)) { v2pdu.add(new VariableBinding(new OID(prefix).append(suffix), new OctetString(val == null ? "" : val))); } Line 211: } Line 212: } Line 213: Line 214: private void addInt(PDU v2pdu, OID prefix,int suffix, Integer val, boolean allowEmpty) { Line 222: } Line 223: Line 224: private void addUuid(PDU v2pdu, OID prefix,int suffix, Guid val) { Line 225: if (val != null && !val.equals(Guid.Empty)){ Line 226: v2pdu.add(new VariableBinding(prefix.append(suffix), new OctetString(val.toString()))); call addString? if (!Guid.Empty.equals(val)) { addString(v2pdu, prefix, suffix, val); } Line 227: } Line 228: } Line 229: Line 230: private static class Host { -- To view, visit http://gerrit.ovirt.org/32951 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic8a06063f8ebbbe4d05783cb25036e870a6be3b7 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: mooli tayer <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: mooli tayer <[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
