Vered Volansky has posted comments on this change.
Change subject: core: Extract SD validations from ImagesHandler
......................................................................
Patch Set 4: Looks good to me, but someone else must approve
(7 inline comments)
Please refer to (non-petty) comments.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidator.java
Line 53: });
Line 54: }
Line 55:
Line 56: /**
Line 57: * Validates that all the domains are withing free disk space
threshold.
/s/withing/within
Line 58: * @return {@link ValidationResult#VALID} if all the domains are
OK, or a {@link ValidationResult} with the first low space domain encountered.
Line 59: */
Line 60: public ValidationResult allDomainsWithinThresholds() {
Line 61: return validOrFirstFailure(new ValidatorPredicate() {
Line 56: /**
Line 57: * Validates that all the domains are withing free disk space
threshold.
Line 58: * @return {@link ValidationResult#VALID} if all the domains are
OK, or a {@link ValidationResult} with the first low space domain encountered.
Line 59: */
Line 60: public ValidationResult allDomainsWithinThresholds() {
That should be within range or upto threshold, but only if you're really petty.
Line 61: return validOrFirstFailure(new ValidatorPredicate() {
Line 62: @Override
Line 63: public ValidationResult evaluate(StorageDomainValidator
validator) {
Line 64: return validator.isDomainWithinThresholds();
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java
Line 136: Set<Guid> storageDomainIds =
ImagesHandler.getAllStorageIdsForImageIds(vmImages);
Line 137: MultipleStorageDomainsValidator
storageDomainValidator =
Line 138: new
MultipleStorageDomainsValidator(vm.getStoragePoolId(), storageDomainIds);
Line 139: if
(!validateAllDomainsUp(storageDomainValidator, message)) {
Line 140: retValue = false;
Why not return false here?
Line 141: }
Line 142:
Line 143: if (retValue && !vm.isAutoStartup()
Line 144: &&
!validateAllDomainsThresholds(storageDomainValidator, message)) {
Line 141: }
Line 142:
Line 143: if (retValue && !vm.isAutoStartup()
Line 144: &&
!validateAllDomainsThresholds(storageDomainValidator, message)) {
Line 145: retValue = false;
as above
Line 146: }
Line 147: }
Line 148:
Line 149: if (retValue) {
....................................................
Commit Message
Line 23: existence check. However, it is always equal to the storage domain id
Line 24: member of the DiskImage, so it is redundant as an additional
Line 25: parameter.
Line 26:
Line 27: The changes in the command were as follows:
/s/command/commands, /s/were/are
Line 28: * The parameters detailed above were removed from the
Line 29: ImagesHandler.PerformImagesCheck call.
Line 30: * Wherever appropriate, a direct call to StorageDomainValidator or
Line 31: MultipleStorageDomainsValidator was added.
Line 25: parameter.
Line 26:
Line 27: The changes in the command were as follows:
Line 28: * The parameters detailed above were removed from the
Line 29: ImagesHandler.PerformImagesCheck call.
/s/call/calls
Line 30: * Wherever appropriate, a direct call to StorageDomainValidator or
Line 31: MultipleStorageDomainsValidator was added.
Line 32: * In AddDiskCommand ImagesHanler.PerformImagesChecks was only used to
Line 33: validate storage domains, so once this logic was extracted, the call
Line 28: * The parameters detailed above were removed from the
Line 29: ImagesHandler.PerformImagesCheck call.
Line 30: * Wherever appropriate, a direct call to StorageDomainValidator or
Line 31: MultipleStorageDomainsValidator was added.
Line 32: * In AddDiskCommand ImagesHanler.PerformImagesChecks was only used to
Why? We can do this offline.
Line 33: validate storage domains, so once this logic was extracted, the call
Line 34: was removed.
Line 35:
Line 36: Change-Id: I4cfc895dceed4de520476ed5e3fa9c1c7cfd78f4
--
To view, visit http://gerrit.ovirt.org/12249
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4cfc895dceed4de520476ed5e3fa9c1c7cfd78f4
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[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: Vered Volansky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
