Liron Aravot has posted comments on this change. Change subject: core: Add alias and description for disk meta data ......................................................................
Patch Set 16: (2 comments) http://gerrit.ovirt.org/#/c/34163/16/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 189: || validate(diskValidator.isDiskInterfaceSupported(getVm()))) && Line 190: setAndValidateDiskProfiles(); Line 191: } Line 192: Line 193: private void setStorageDomainValidator(DiskImage diskImage) { > No, we should not do it that way since the validator is per DiskImage there's only one disk here- so i don't understand what's the problem. Line 194: StorageDomain storageDomain = getStorageDomainDAO().getForStoragePool( Line 195: diskImage.getStorageIds().get(0), diskImage.getStoragePoolId()); Line 196: storageDomainValidator = new StorageDomainValidator(storageDomain); Line 197: } Line 437: getJsonDiskDescription()); Line 438: runVdsCommand(VDSCommandType.SetVolumeDescription, vdsCommandParameters); Line 439: } catch (Exception e) { Line 440: log.error("Exception while setting volume description for disk. ERROR: '{}'", e); Line 441: addAuditLog(storageDomain, diskImage); > No! We should not add a different audit log You should - this audit log says "Failed to update the meta data description of disk ${DiskName} (D ata Center ${DataCenterName}, Storage Domain ${StorageDomainName})." this is correct only for exception, if the domain is not up there should be warning to the user that you didn't attempt to update.. "Not updating the metadata of Disk..because the Storage Domain is not in STATUS status" Line 442: } Line 443: } Line 444: Line 445: private void addAuditLog(StorageDomain storageDomain, DiskImage diskImage) { -- 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: 16 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
