----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14823/#review27296 -----------------------------------------------------------
Darren, the design of all of this has already changed here: https://reviews.apache.org/r/14522/. Take a look at the newer revisions which haven't made it in yet (still waiting on a final review from John Burwell). Could you take a look there and see if those changes address your issue? In this new approach, we no longer create a sorted list. Instead, we simply chose one by iterating over the list of strategies once and returning the best strategy. - Chris Suich On Oct. 22, 2013, 3:34 a.m., Darren Shepherd wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14823/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2013, 3:34 a.m.) > > > Review request for cloudstack, Chris Suich and edison su. > > > Repository: cloudstack-git > > > Description > ------- > > I specifically ran into a problem in the spring modularization branch with > StrategyPriority.sortStrategies(). The lists of strategies are unmodifiable > collections so Collection.sort() can not be used. I went to do a quick fix > for this but then decided to try remove the duplicated logic in each code > that needs a SnapshotStrategy or DataMotionStrategy and then ended up > refactoring quite a bit. > > > Diffs > ----- > > > engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/DataMotionStrategy.java > 950f9e2 > > engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotStrategy.java > e4cecb6 > > engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StorageStrategyFactory.java > PRE-CREATION > > engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriority.java > 81034b1 > > engine/api/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java > 3d75279 > > engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java > 5f5f01e > > engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/DataMotionServiceImpl.java > 2d31320 > > engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java > 6a874d6 > > engine/storage/src/org/apache/cloudstack/storage/helper/StorageStrategyFactoryImpl.java > PRE-CREATION > > engine/storage/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java > PRE-CREATION > > plugins/hypervisors/xen/src/org/apache/cloudstack/storage/motion/XenServerStorageMotionStrategy.java > 8578a9a > server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java dade983 > > Diff: https://reviews.apache.org/r/14823/diff/ > > > Testing > ------- > > > Thanks, > > Darren Shepherd > >
