Idan Shaby has posted comments on this change. Change subject: core: add MemoryStorageHandler the ability to sort ......................................................................
Patch Set 1: (2 comments) https://gerrit.ovirt.org/#/c/40438/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 98: protected void sortStorageDomains(List<StorageDomain> domainsInPool, Collection<DiskImage> vmDisks) { Line 99: ComparatorChain comparatorChain = new ComparatorChain(); Line 100: // When there is more than one comparator, a nested sort is performed. Line 101: for (Comparator<StorageDomain> comparator : getStorageDomainComparators(domainsInPool, vmDisks)) { Line 102: comparatorChain.addComparator(comparator, true); > Why reverse the sorting? Because you want the "biggest" domains first. For example, StorageDomainNumberOfVmDisksComparator compares, like you would expect, the number of vm disks each storage domain holds. So if you sorted them the regular way (with reverse = false) you would have the domain with the smallest number of vm disks first. Thus, you need the flag reverse = true. Line 103: } Line 104: Collections.sort(domainsInPool, comparatorChain); Line 105: } Line 106: https://gerrit.ovirt.org/#/c/40438/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageTypeComparator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/storageDomainsComparators/StorageTypeComparator.java: Line 8: Line 9: @Override Line 10: public int compare(StorageDomain storageDomain, StorageDomain storageDomain2) { Line 11: return Boolean.compare(storageDomain.getStorageType().isFileDomain(), Line 12: storageDomain2.getStorageType().isFileDomain()); > AFAIK, this will give you the reverse order - we want file domains FIRST I designed the comparators to compare between storage domains the intuitive way ("lowest" before "biggest"), and then MemoryStorageHandler sorts the list of domains in descending order, because that's what we need there. In StorageDomainNumberOfVmDisksComparator it's more intuitive since we are talking about numbers. Here, what we compare is whether the domain is a file domain or not. Thus, file domains are "bigger" than block ones, and MemoryStorageHandler sorts them in descending order. Line 13: } -- To view, visit https://gerrit.ovirt.org/40438 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I00ad78fca97527a7dbf426a28ced7314a7bd921c 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: Arik Hadas <aha...@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
