Yair Zaslavsky has posted comments on this change.
Change subject: engine-core: [autorecovery] Autorecovery logic
......................................................................
Patch Set 7: (3 inline comments)
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java
Line 21: VDS_MAINTENANCE_FAILED(17,
AuditLogTimeInterval.MINUTE.getValue()), // When VDS is transferred to
maintenance mode
What about storage domain activation?
I see we have only - USER_ACTIVATED_STORAGE_DOMAIN .
It's text is not good for us - we need a new type here (I will comment more on
this later).
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
Line 1086: @TypeConverterAttribute(String.class)
Are these needs anymore , now that you're using AuditLogType mechanism?
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java
Line 674: if (seconds > 0 && auditLogable.isInternalExecution()) {
IMHO,
Not entirely correct - have you taken into consideration the code in
AddStoragePoolWithStoragesCommand ? It calls an internal ActivateStorageDomain
command, but it is initiated by user, so it has to be logged anyway.
My suggestion is - Since commands are autiLoggable maybe add a new boolean
isIgnoringDuplicateLogging at AuditLoggableBase - it will return by default
true if the command is internal , but in case of ActivateStorageDomain it will
also check what is the parent command (you can obtain this from the parameters)
and if the parent command is AddStoragePoolWithStorages.
In addition, we still have the problem of the audit log type for this one (not
sure if PM will approve to change the text so it will match auto recovery as
well).
Actually, I see now that also VDS_ACTIVATE audit log type is problematic.
I will email you some ideas on how to solve this.
--
To view, visit http://gerrit.ovirt.org/2011
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I544ff69bad946fd3d4edd2cf815ad10392958d26
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
