> On Oct. 22, 2013, 1:19 p.m., Chris Suich wrote: > > 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.
That being said, your changes appear to make things cleaner. Maybe if mine are accepted we can rebase yours on top of mine (i.e. replace sort() with the new approach)? - Chris ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14823/#review27296 ----------------------------------------------------------- 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 > >
