Liron Aravot has posted comments on this change.

Change subject: core: Add alias and description for disk meta data
......................................................................


Patch Set 16:

(6 comments)

http://gerrit.ovirt.org/#/c/34163/16/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 850: 
Line 851:     /**
Line 852:      * Creates and returns a Json string containing the disk alias 
and the disk description. The disk alias and
Line 853:      * description are preserved in the disk meta data. If the meta 
data will be added with more fields
Line 854:      * UpdateVmDiskCimmand should be changed accordingly.
/s/UpdateVmDiskCimmand/UpdateVmDiskCommand.shouldPerformMetadataUpdate

note that you wrote Cimmand instead of Command
Line 855:      */
Line 856:     public static String getJsonDiskDescription(String diskAlias, 
String diskDescription) throws IOException {
Line 857:         Map<String, Object> description = new HashMap<>();
Line 858:         description.put(ImagesHandler.DISK_ALIAS, diskAlias);


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 174:             }
Line 175:         }
Line 176: 
Line 177:         if (DiskStorageType.IMAGE == 
getOldDisk().getDiskStorageType()) {
Line 178:             setStorageDomainValidator((DiskImage)getNewDisk());
see related comment below about setStorageDomainValidator, after you'll change 
it this part of the change is unneeded.
Line 179: 
Line 180:             if (!validateCanResizeDisk()) {
Line 181:                 return false;
Line 182:             }


Line 189:                 || 
validate(diskValidator.isDiskInterfaceSupported(getVm()))) &&
Line 190:                 setAndValidateDiskProfiles();
Line 191:     }
Line 192: 
Line 193:     private void setStorageDomainValidator(DiskImage diskImage) {
please remove this method and have this method in the getter

if (storageDomainValidator == null) {

init the validator

return validator;
}
Line 194:         StorageDomain storageDomain = 
getStorageDomainDAO().getForStoragePool(
Line 195:                 diskImage.getStorageIds().get(0), 
diskImage.getStoragePoolId());
Line 196:         storageDomainValidator = new 
StorageDomainValidator(storageDomain);
Line 197:     }


Line 300:         return true;
Line 301:     }
Line 302: 
Line 303:     private boolean isDomainExistAndActive() {
Line 304:         if 
(!validate(getStorageDomainValidator().isDomainExistAndActive())) {
you replace this code with

return validate(getStorageDomainValidator().isDomainExistAndActive()).
Line 305:             return false;
Line 306:         }
Line 307:         return true;
Line 308:     }


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);
this should be a different audit log of error, while the other one should be of 
warning (not performing vs failed)
Line 442:         }
Line 443:     }
Line 444: 
Line 445:     private void addAuditLog(StorageDomain storageDomain, DiskImage 
diskImage) {


Line 441:             addAuditLog(storageDomain, diskImage);
Line 442:         }
Line 443:     }
Line 444: 
Line 445:     private void addAuditLog(StorageDomain storageDomain, DiskImage 
diskImage) {
/s/addAuditLog/auditLogForNoMetadataDescriptionUpdate()
Line 446:         AuditLogableBase auditLogableBase = new AuditLogableBase();
Line 447:         auditLogableBase.addCustomValue("DataCenterName", 
getStoragePool().getName());
Line 448:         auditLogableBase.addCustomValue("StorageDomainName", 
storageDomain.getName());
Line 449:         auditLogableBase.addCustomValue("DiskName", 
diskImage.getDiskAlias());


-- 
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

Reply via email to