Chris, I am in-transit to CloudConnect. I will try to review the latest patch by COB today (21 Oct 2013).
Thanks, -John On Oct 21, 2013, at 9:31 AM, "SuichII, Christopher" <chris.su...@netapp.com> wrote: John, Will you be able to take a look at this revision soon? It is a small change to address your comments so it should not take very long. Thanks, Chris -- Chris Suich chris.su...@netapp.com NetApp Software Engineer Data Center Platforms – Cloud Solutions Citrix, Cisco & Red Hat On Oct 17, 2013, at 2:46 PM, Chris Suich <chris.su...@netapp.com> wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14522/ Review request for cloudstack, Brian Federle, edison su, John Burwell, and Mike Tutkowski. By Chris Suich. *Updated Oct. 17, 2013, 6:46 p.m.* Changes -Added context to the error messages for not finding DataMotionStrategies. However, it appears that the place where pickStrategy() is called for SnapshotStrategies is not able to actually throw error messages. I will open a separate issue for this. -Added TODOs for DRYing out the overloaded pickStrategy() methods. -Added context to the error message for failing to find the image store snapshot info for a snapshot response. However, this also is unable to throw a meaning full error message as the exception would be caught and replaced with a generic error. I will create a separate issue for this as well. *Repository: *cloudstack-git Description After the last batch of work to the revertSnapshot API, SnapshotServiceImpl was not tied into the workflow to be used by storage providers. I have added the logic in a similar fashion to takeSnapshot(), backupSnapshot() and deleteSnapshot(). I have also added a 'Revert to Snapshot' action to the volume snapshots list in the UI. Testing I have tested all of this locally with a custom storage provider. Unfortunately, I'm still in the middle of figuring out how to properly unit test this type of code. If anyone has any recommendations, please let me know. Diffs (updated) - engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriority.java (81034b1) - engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/DataMotionServiceImpl.java (2d31320) - server/src/com/cloud/api/ApiResponseHelper.java (f4ca112) - server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java (dade983) View Diff <https://reviews.apache.org/r/14522/diff/>
