Noam Slomianko has posted comments on this change.
Change subject: engine:Trusted Compute Pools - Open Attestation integration
with oVirt engine
......................................................................
Patch Set 3: (3 inline comments)
Patch seems good.
Logging wasn't covered in the feature page, so I think we should think if maybe
some of the messages should be warnings.
for example: is loading trusted template in to an untrusted cluster important
enough to be a warning?
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java
Line 749: //watchdog
Line 750: WATCHDOG_EVENT(9901),
Line 751:
Line 752: //trusted service
Line 753: VDS_UNTRUSTED(10000, AuditLogTimeInterval.MINUTE.getValue()),
maybe change to
IMPORTEXPORT_IMPORT_TRUSTED_TEMPLATE_IN_UNTRUSTED_VDS_GROUP
IMPORTEXPORT_IMPORT_UNTRUSTED_TEMPLATE_IN_TRUSTED_VDS_GROUP
Line 754: IMPORTEXPORT_IMPORT_TEMPLATE_FROM_TRUSTED_TO_UNTRUSTED(10007),
Line 755: IMPORTEXPORT_IMPORT_TEMPLATE_FROM_UNTRUSTED_TO_TRUSTED(10008),
Line 756: USER_ADD_VM_TEMPLATE_FROM_TRUSTED_TO_UNTRUSTED(10009),
Line 757: USER_ADD_VM_TEMPLATE_FROM_UNTRUSTED_TO_TRUSTED(10010),
....................................................
File
backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties
Line 521: IMPORTEXPORT_REMOVE_TEMPLATE_FAILED=Failed to remove Template
${VmTemplateName} from ${StorageDomainName}
Line 522: IMPORTEXPORT_STARTING_REMOVE_VM=Starting to remove Vm ${VmName}
remove from ${StorageDomainName}
Line 523: IMPORTEXPORT_REMOVE_VM=Vm ${VmName} was removed from
${StorageDomainName}
Line 524: IMPORTEXPORT_REMOVE_VM_FAILED=Failed to remove Vm ${VmName} remove
from ${StorageDomainName}
Line 525: IMPORTEXPORT_GET_VMS_INFO_FAILED=Failed to retrieve VM/Templates
information from export domain ${StorageDomainName}
Looking at: if (getVmTemplate().isTrustedService() &&
!getVdsGroup().supportsTrustedService())
It means that the template requires trusted cluster and the cluster isn't
trusted
so maybe the message should be: "...trusted template was imported in an
untrusted cluster" ?
(same with the opposite one)
Line 526: IMPORTEXPORT_IMPORT_TEMPLATE_FROM_TRUSTED_TO_UNTRUSTED=the Template
${VmTemplateName} was created in trusted cluster and imported into a
non-trusted cluster
Line 527: IMPORTEXPORT_IMPORT_TEMPLATE_FROM_UNTRUSTED_TO_TRUSTED=the Template
${VmTemplateName} was created in non-trusted cluster and imported into a
trusted cluster
Line 528: USER_ADD_ROLE_WITH_ACTION_GROUP=Role ${RoleName} was added by
${UserName}
Line 529: USER_ADD_ROLE_WITH_ACTION_GROUP_FAILED=Failed to add role ${RoleName}
....................................................
Commit Message
Line 7: engine:Trusted Compute Pools - Open Attestation integration with oVirt
engine
Line 8:
Line 9: Detailed description: http://wiki.ovirt.org/Trusted_compute_pools
Line 10:
Line 11: In this patchset, follow changes has been introduced:
I would suggest changing to:
" Added logging when:
1) Creating a template with different trust value from the original vm
2) Importing trusted template to an untrusted cluster
3) changing the trust value of a template"
Line 12: 1.Make a template from from a trusted/untrusted vm to a
untrusted/trusted one, and make an audit event.
Line 13: 2.Import trusted template into a untrusted cluster and vice versa,
make an audit event.
Line 14: 3.Edit a template from a trusted/untrusted one to a untrusted/trusted
one, then make an audit event.
Line 15:
--
To view, visit http://gerrit.ovirt.org/16600
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf866224c6bdddb3e4cbf56666a8b340213f7c83
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gang Wei <[email protected]>
Gerrit-Reviewer: Dave Chen <[email protected]>
Gerrit-Reviewer: Emily Zhang <[email protected]>
Gerrit-Reviewer: Noam Slomianko <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-Reviewer: ofri masad <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
