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<mailto: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<mailto: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/>


Reply via email to