Omer Frenkel has posted comments on this change.
Change subject: core: Add 5 new Events to AuditLog
......................................................................
Patch Set 2: (3 inline comments)
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
Line 265: Integer maxUsedPercentageThreshold =
Config.GetValue(ConfigValues.LogMaxPhysicalMemoryUsedThresholdInPercentage);
Line 266: if (stat.getmem_available() < minAvailableThreshold
Line 267: || stat.getusage_mem_percent() >
maxUsedPercentageThreshold) {
Line 268: AuditLogableBase logable = new AuditLogableBase();
Line 269: logable.setVdsId(stat.getId());
i know its not part of this change, but you can create the logable object with
vdsId in the constructor instead of setting it separatly
this apply to all next changes here as well
Line 270: logable.AddCustomValue("HostName", _vds.getvds_name());
Line 271: logable.AddCustomValue("AvailableMemory",
stat.getmem_available().toString());
Line 272: logable.AddCustomValue("UsedMemory",
stat.getusage_mem_percent().toString());
Line 273: logable.AddCustomValue("Threshold",
stat.getmem_available() < minAvailableThreshold ?
Line 274: minAvailableThreshold.toString() :
Line 275: maxUsedPercentageThreshold.toString());
Line 276: auditLog(logable, stat.getmem_available() <
minAvailableThreshold ? AuditLogType.VDS_LOW_MEM :
Line 277: AuditLogType.VDS_HIGH_MEM_USE);
Line 278: }
in my opinion, the code looks a little complicated,
i think its simpler to compare once and act according to the result,
something like:
if (..mem.. < minTreshold)
valueToLog=..
addCustomValue(..)
else if (..mem.. > maxTreshold)
valueToLog=..
addCustomValue(..)
if (valueToLog != null)
log()
Line 279: }
Line 280:
Line 281: /**
Line 282: * check if value is less than configurable threshold , if yes ,
generated event log message
Line 333: logable.AddCustomValue("AvailableSwapMemory",
stat.getswap_free().toString());
Line 334: logable.AddCustomValue("Threshold", stat.getswap_free() <
minAvailableThreshold ?
Line 335: minAvailableThreshold.toString() :
maxUsedPercentageThreshold.toString());
Line 336: auditLog(logable, stat.getswap_free() <
minAvailableThreshold ?
Line 337: AuditLogType.VDS_LOW_SWAP :
AuditLogType.VDS_HIGH_SWAP_USE);
same here, i think this code can be nicer
Line 338: }
Line 339: }
Line 340:
Line 341: public VdsUpdateRunTimeInfo(VdsManager vdsManager, VDS vds) {
--
To view, visit http://gerrit.ovirt.org/10865
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id382c4688bdcf46f7b343ff0661893a97b5dc726
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: ofri masad <oma...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: ofri masad <oma...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
