[GitHub] [cloudstack] rhtyd commented on a change in pull request #3184: Fixed Decision Making Logic when selecting which secondary storage to use from a selection

[GitBox <gi...@apache.org>]

rhtyd commented on a change in pull request #3184: Fixed Decision Making Logic when selecting which secondary storage to use from a selection
URL: https://github.com/apache/cloudstack/pull/3184#discussion_r262820491
 
 

 ##########
 File path: engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java
 ##########
 @@ -146,7 +146,13 @@ public ImageStoreEntity getImageStore(String uuid) {
     @Override
     public DataStore getImageStore(List<DataStore> imageStores) {
         if (imageStores.size() > 1) {
-            Collections.shuffle(imageStores); // Randomize image store list.
+            Collections.sort(imageStores, new Comparator<DataStore>() {
+                @Override
 
 Review comment:
   This still does not cleanly solve the design issue. The `getImageStore` may be used in several places and simply randomized return (whether shuffled or sorted by capacity) does not make much difference. Instead, a check may be performed preferably via a different method that checks if image store has enough capacity, that check method may be used either by this method or the consumers of `getImageStore`.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services