Maor Lipchuk has posted comments on this change. Change subject: core: Add alias and description for disk meta data ......................................................................
Patch Set 3: (4 comments) http://gerrit.ovirt.org/#/c/34163/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddImageFromScratchCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddImageFromScratchCommand.java: Line 126: AuditLogableBase auditLogableBase = new AuditLogableBase(); Line 127: auditLogableBase.addCustomValue("DataCenterName", getStoragePool().getName()); Line 128: auditLogableBase.addCustomValue("StorageDomainName", getStorageDomain().getName()); Line 129: auditLogableBase.addCustomValue("DiskName", getParameters().getDiskInfo().getDiskAlias()); Line 130: AuditLogDirector.log(auditLogableBase, AuditLogType.UPDATE_DESCRIPTION_FOR_DISK_FAILED); > there error here is in generating the description and not in updating it (w I didn't understood your comment, the audit log will be presented when we could not get map the disk info to json. I prefer to leave an audit log since we want to indicate to the user we could not update the meta data Line 131: return StringUtils.EMPTY; Line 132: } Line 133: } Line 134: Line 127: auditLogableBase.addCustomValue("DataCenterName", getStoragePool().getName()); Line 128: auditLogableBase.addCustomValue("StorageDomainName", getStorageDomain().getName()); Line 129: auditLogableBase.addCustomValue("DiskName", getParameters().getDiskInfo().getDiskAlias()); Line 130: AuditLogDirector.log(auditLogableBase, AuditLogType.UPDATE_DESCRIPTION_FOR_DISK_FAILED); Line 131: return StringUtils.EMPTY; > you can replace StringUtils.EMPTY with "", it's shorter. I prefer to leave that as it is Line 132: } Line 133: } Line 134: Line 135: @Override http://gerrit.ovirt.org/#/c/34163/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java: Line 843: dummy.getSnapshots().addAll(ImagesHandler.getAllImageSnapshots(dummy.getImageId())); Line 844: return dummy; Line 845: } Line 846: Line 847: public static String getJsonDiskDescription(String diskAlias, String diskDescription) throws IOException { > Not sure that i agree with this approach though I understand the motivation This is currently the best option for now, instead of maintaining two fields in the meta data Line 848: Map<String, Object> description = new HashMap<>(); Line 849: description.put(ImagesHandler.DISK_ALIAS, diskAlias); Line 850: description.put(ImagesHandler.DISK_DESCRIPTION, diskDescription); Line 851: return JsonHelper.mapToJson(description, false); http://gerrit.ovirt.org/#/c/34163/3/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java: Line 853: CREATE_OVF_STORE_FOR_STORAGE_DOMAIN_FAILED(191, AuditLogSeverity.WARNING), Line 854: CREATE_OVF_STORE_FOR_STORAGE_DOMAIN_INITIATE_FAILED(192), Line 855: DELETE_OVF_STORE_FOR_STORAGE_DOMAIN_FAILED(193), Line 856: UPDATE_DESCRIPTION_FOR_OVF_STORE_FAILED(1016), Line 857: UPDATE_DESCRIPTION_FOR_DISK_FAILED(1018, AuditLogSeverity.WARNING), > if you added it as 1018, please move it a line below after 1017 to try and done Line 858: RETRIEVE_OVF_STORE_FAILED(1017, AuditLogSeverity.WARNING), Line 859: Line 860: // Authentication Line 861: USER_ACCOUNT_DISABLED_OR_LOCKED(160, AuditLogSeverity.ERROR, -- To view, visit http://gerrit.ovirt.org/34163 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie2642ae7016579ead699509426e01ac2010bd374 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@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
