Idan Shaby has posted comments on this change. Change subject: core: redesign MemoryStorageHandler to use filters ......................................................................
Patch Set 1: (3 comments) https://gerrit.ovirt.org/#/c/40437/1//COMMIT_MSG Commit Message: Line 8: Line 9: This patch doesn't change the behavior of MemoryStorageHandler. Line 10: Instead, it changes the way we select storage domains for memory Line 11: volumes to use filters. Each filter verifies that every domain meets a Line 12: specific condition, and removes those who don't from the list of > s/who/that/ Done Line 13: candidates. Line 14: This way, one can simply add a new filter to the list and add new logic Line 15: to the domain selection. Line 16: https://gerrit.ovirt.org/#/c/40437/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryStorageHandler.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryStorageHandler.java: Line 47: } Line 48: return storageDomainForMemory; Line 49: } Line 50: Line 51: public void updateDisksStorage(StorageDomain storageDomain, List<DiskImage> disksList) { > why? Because the main use is in StorageDomainSpaceRequirementsFilter. Unfortunately, I also must use it here since the methods that call findStorageDomainForMemory take into consideration that "disksList" is returned updated. Until now, we went over each domain, updated the disks, checked if the storage domain met the conditions , and if it did - returned the storage domain. Thus, the disks list was updated with the details of the returned storage domain. Now, we go over all the storage domains and update the disks list each time, but we don't necessarily return the last one. So before we return the domain, we need to update it again, and this is why I thought it's better to locate updateDisksStorage here. Line 52: for (DiskImage disk : disksList) { Line 53: disk.setStorageIds(new ArrayList<Guid>(Collections.singletonList(storageDomain.getId()))); Line 54: } Line 55: /* https://gerrit.ovirt.org/#/c/40437/1/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainFilterTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/storageDomainFilters/StorageDomainFilterTest.java: Line 31: Line 32: @Test Line 33: public void testThatFilterDoesntRemoveStorageDomainFromList() { Line 34: List<StorageDomain> filteredDomains = filterStorageDomain(false); Line 35: assertEquals(filteredDomains, Arrays.asList(storageDomain)); > This assumes both collections are the same type. Done Line 36: } Line 37: Line 38: private List<StorageDomain> filterStorageDomain(boolean removeStorageDomainFromList) { Line 39: filter = new StorageDomainFilter() { -- To view, visit https://gerrit.ovirt.org/40437 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iff7094a2351ab0755ff21ba3ccfc4b8bc5cfff6c Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Candace Sheremeta <csher...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
