-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14522/#review27298
-----------------------------------------------------------



engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/DataMotionServiceImpl.java
<https://reviews.apache.org/r/14522/#comment53128>

    Convert these lines to a StringUtils.join.  It should reduce the number of 
lines to one ...



server/src/com/cloud/api/ApiResponseHelper.java
<https://reviews.apache.org/r/14522/#comment53129>

    I recommend simplifying this block to the following:
    
    if (secondaryInfo == null) {
       new CloudRuntimeException("Unable to find info for image store snapshot 
with uuid '"+snapshot.getUuid()+"'");
    }
    
    snapshotResponse.setRevertable(secondaryInfo.isRevertable());
    
    // The rest of the method ...


- John Burwell


On Oct. 17, 2013, 2:46 p.m., Chris Suich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14522/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2013, 2:46 p.m.)
> 
> 
> Review request for cloudstack, Brian Federle, edison su, John Burwell, and 
> Mike Tutkowski.
> 
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   
> 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 
> 
> Diff: https://reviews.apache.org/r/14522/diff/
> 
> 
> 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.
> 
> 
> Thanks,
> 
> Chris Suich
> 
>

Reply via email to