Allon Mureinik has posted comments on this change.
Change subject: core: Extract SD validations from ImagesHandler
......................................................................
Patch Set 6: (5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidator.java
Line 22: */
Line 23: public class MultipleStorageDomainsValidator {
Line 24:
Line 25: /** The ID of the storage pool all the domains belong to */
Line 26: private NGuid spId;
Done
Line 27:
Line 28: /** A map from the ids of each domain being validated to its
validator */
Line 29: private Map<Guid, StorageDomainValidator> domainValidators;
Line 30:
Line 35: public MultipleStorageDomainsValidator(NGuid spId,
Collection<Guid> sdIds) {
Line 36: this.spId = spId;
Line 37: domainValidators = new HashMap<Guid,
StorageDomainValidator>(sdIds.size());
Line 38: for (Guid id : sdIds) {
Line 39: domainValidators.put(id, null);
I think this is a matter of taste - I prefer not holding two data structures if
I only need one
Line 40: }
Line 41: }
Line 42:
Line 43: /**
Line 68:
Line 69: /** @return The lazy-loaded validator for the given map entry */
Line 70: private StorageDomainValidator
getStorageDomainValidator(Map.Entry<Guid, StorageDomainValidator> entry) {
Line 71: if (entry.getValue() == null) {
Line 72: entry.setValue(new
StorageDomainValidator(getStorageDomainDAO().getForStoragePool(entry.getKey(),
spId)));
The constructor does not fail. This is exactly was allDomainsExistAndActive()
verifies.
Line 73: }
Line 74:
Line 75: return entry.getValue();
Line 76: }
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/MultipleStorageDomainsValidatorTest.java
Line 93: @Test
Line 94: public void testAllDomainsWithinThresholdsOneLacking() {
Line 95: domain1.getStorageDynamicData().setAvailableDiskSize(15);
Line 96: domain2.getStorageDynamicData().setAvailableDiskSize(7);
Line 97: ValidationResult result =
validator.allDomainsWithinThresholds();
It regards free space.
The name isn't great, but it's done to keep consistency with
StoragDomainValidator.
Perhaps it should be fixed, but I don't think this renaming is in the scope of
this patch.
Line 98: assertFalse("Both domains should not be withing thresholds",
result.isValid());
Line 99: assertEquals("Wrong validation error",
Line 100:
VdcBllMessages.ACTION_TYPE_FAILED_DISK_SPACE_LOW_ON_TARGET_STORAGE_DOMAIN,
Line 101: result.getMessage());
....................................................
Commit Message
Line 10: ImagesHandler.PerformImagesChecks, and made the relevant commands use
it
Line 11: directly.
Line 12:
Line 13: In order to do this, a new type of validator was introduced,
Line 14: MultipleStoroageDomainsValidator, which aggregates validations from
Done
Line 15: individual StorageDomainValidators in a fast-fail pattern. This is done
Line 16: for performance reasons, so there is no need to load all the storage
Line 17: domains from the database preemptively.
Line 18:
--
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: 6
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 Ar <[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
