Maor Lipchuk has posted comments on this change. Change subject: core: Add alias and description for disk meta data ......................................................................
Patch Set 18: (4 comments) http://gerrit.ovirt.org/#/c/34163/18/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java: Line 184: || validate(diskValidator.isDiskInterfaceSupported(getVm()))) && Line 185: setAndValidateDiskProfiles(); Line 186: } Line 187: Line 188: protected StorageDomainValidator getStorageDomainValidator(DiskImage diskImage) { > thanks, I think this is the wrong way to implement all this solution, but just to get forward with it I will remove this. Line 189: if (storageDomainValidator == null) { Line 190: StorageDomain storageDomain = getStorageDomainDAO().getForStoragePool( Line 191: diskImage.getStorageIds().get(0), diskImage.getStoragePoolId()); Line 192: storageDomainValidator = new StorageDomainValidator(storageDomain); Line 187: Line 188: protected StorageDomainValidator getStorageDomainValidator(DiskImage diskImage) { Line 189: if (storageDomainValidator == null) { Line 190: StorageDomain storageDomain = getStorageDomainDAO().getForStoragePool( Line 191: diskImage.getStorageIds().get(0), diskImage.getStoragePoolId()); > let's do here setStorageDomain(storageDomain) or preferebly - export this l I prefer to leave it that way Line 192: storageDomainValidator = new StorageDomainValidator(storageDomain); Line 193: } Line 194: return storageDomainValidator; Line 195: } Line 413: Line 414: private void updateMetaDataDescription(DiskImage diskImage) { Line 415: StorageDomain storageDomain = Line 416: getStorageDomainDAO().getForStoragePool(diskImage.getStorageIds().get(0), Line 417: getVm().getStoragePoolId()); > it's preferred to do here getStorageDomain() to avoid the query.. (see the Using getStorageDomain() is prone error, you can't be sure which Storage domain was set there. Line 418: if (!isDomainExistAndActive()) { Line 419: auditLogForNoMetadataDescriptionUpdate(AuditLogType.UPDATE_DESCRIPTION_FOR_DISK_FAILED_SINCE_STORAGE_DOMAIN_NOT_ACTIVE, Line 420: storageDomain, Line 421: diskImage); http://gerrit.ovirt.org/#/c/34163/18/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties File backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties: Line 662: CREATE_OVF_STORE_FOR_STORAGE_DOMAIN_FAILED=Failed to create OVF store disk for Storage Domain ${StorageDomainName}.\n The Disk with the id ${DiskId} might be removed manually for automatic attempt to create new one. \n OVF updates won't be attempted on the created disk. Line 663: CREATE_OVF_STORE_FOR_STORAGE_DOMAIN_INITIATE_FAILED=Failed to create OVF store disk for Storage Domain ${StorageDomainName}. \n OVF data won't be updated meanwhile for that domain. Line 664: UPDATE_FOR_OVF_STORES_FAILED=Failed to update OVF disks ${DisksIds}, OVF data isn't updated on those OVF stores (Data Center ${DataCenterName}, Storage Domain ${StorageDomainName}). Line 665: UPDATE_DESCRIPTION_FOR_DISK_FAILED=Failed to update the meta data description of disk ${DiskName} (Data Center ${DataCenterName}, Storage Domain ${StorageDomainName}). Line 666: UPDATE_DESCRIPTION_FOR_DISK_FAILED_SINCE_STORAGE_DOMAIN_NOT_ACTIVE=Not updating the metadata of Disk ${DiskName} (Data Center ${DataCenterName}. Since the Storage Domain ${StorageDomainName} is not in active. > /s/active/Active status. I prefer to use active here, it is also valid IMHO. I will change the FAILED to SKPPED Line 667: DELETE_OVF_STORE_FOR_STORAGE_DOMAIN_FAILED=Failed to delete the OVF store disk for Storage Domain ${StorageDomainName}.\n In order to detach the domain please remove it manually or try to detach the domain again for another attempt. Line 668: IMPORTEXPORT_FAILED_TO_IMPORT_VM=Failed to read VM '${ImportedVmName}' OVF, it may be corrupted. Underlying error message: ${ErrorMessage} Line 669: IMPORTEXPORT_FAILED_TO_IMPORT_TEMPLATE=Failed to read Template '${Template}' OVF, it may be corrupted. Underlying error message: ${ErrorMessage} Line 670: CANNOT_HIBERNATE_RUNNING_VMS_AFTER_CLUSTER_CPU_UPGRADE=Hibernation of VMs after CPU upgrade of Cluster ${VdsGroup} is not supported. Please stop and restart those VMs in case you wish to hibernate them -- 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: 18 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
