Moti Asayag has posted comments on this change.

Change subject: core: audit logging support for mac pools.
......................................................................


Patch Set 5: Code-Review-1

(3 comments)

http://gerrit.ovirt.org/#/c/29204/5/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogableBase.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogableBase.java:

Line 90:     private Guid quotaIdForLog;
Line 91:     private String quotaNameForLog;
Line 92:     private String callStack;
Line 93:     private String macPoolNameForLog;
Line 94:     private Guid macPoolIdForLog;
there is no reason to extend this class with those fields.
They are relevant only for a specific flows and not commonly used as 
user/dc/host/name.

there are alternative methods to provide values to the audit-log entity. i.e. 
see:

VnicProfileCommandBase.getVnicProfileName() and 
VnicProfileCommandBase.getNetworkName() which are used to translate the place 
holder in AuditLogMessages.properties

ADD_VNIC_PROFILE=VM network interface profile ${VnicProfileName} was added to 
network ${NetworkName} in Data Center: ${DataCenterName}. (User: ${UserName})

the ${VnicProfileName} is replaced by the value suggested by 
VnicProfileCommandBase.getVnicProfileName() and so ${NetworkName}.
Line 95: 
Line 96:     public AuditLogableBase() {
Line 97:     }
Line 98: 


http://gerrit.ovirt.org/#/c/29204/5/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AuditLogDAODbFacadeImpl.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/AuditLogDAODbFacadeImpl.java:

Line 138:                 .addValue("gluster_volume_id", 
event.getGlusterVolumeId())
Line 139:                 .addValue("gluster_volume_name", 
event.getGlusterVolumeName())
Line 140:                 .addValue("call_stack", event.getCallStack())
Line 141:                 .addValue("mac_pool_id", event.getMacPoolId())
Line 142:                 .addValue("mac_pool_name", event.getMacPoolName());
extending this table isn't required.
Line 143:     }
Line 144: 
Line 145:     private MapSqlParameterSource getExternalEventSqlMapper(AuditLog 
event) {
Line 146:         return getSqlMapper(event)


http://gerrit.ovirt.org/#/c/29204/5/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties
File 
backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties:

Line 835: KDUMP_FLOW_NOT_DETECTED_ON_VDS=Kdump flow not detected on host 
'${VdsName}'.
Line 836: KDUMP_FLOW_FINISHED_ON_VDS=Kdump flow finished on host '${VdsName}'.
Line 837: KDUMP_DETECTION_NOT_CONFIGURED_ON_VDS=Kdump detection is enabled for 
host '${VdsName}', but kdump is not configured properly on host.
Line 838: 
Line 839: MAC_POOL_ADD_SUCCESS=MAC Pool ${macPoolName} (id=${macPoolId}) 
created.
please add (User: ${UserName}) to the messages, so the user which triggered the 
event could be identified.

Please reformat the messages to comply with the rest of the messages format.
Line 840: MAC_POOL_ADD_FAILED=Creation of MAC Pool ${macPoolName} failed.
Line 841: MAC_POOL_EDIT_SUCCESS=MAC Pool ${macPoolName} (id=${macPoolId}) 
changed.
Line 842: MAC_POOL_EDIT_FAILED=Alteration of MAC Pool ${macPoolName} 
(id=${macPoolId}) failed.
Line 843: MAC_POOL_REMOVE_SUCCESS=MAC Pool ${macPoolName} (id=${macPoolId}) 
removed.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I376b89abc03657a7cd2eb1b06e21591e4cd944ad
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <mmu...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@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

Reply via email to