cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Burwell <>
Subject Re: Review Request 14522: [CLOUDSTACK-4771] Support Revert VM Disk from Snapshot
Date Mon, 21 Oct 2013 13:44:29 GMT

I am in-transit to CloudConnect.  I will try to review the latest patch by
COB today (21 Oct 2013).


On Oct 21, 2013, at 9:31 AM, "SuichII, Christopher" <>


 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.

Chris Suich
NetApp Software Engineer
Data Center Platforms – Cloud Solutions
Citrix, Cisco & Red Hat

 On Oct 17, 2013, at 2:46 PM, Chris Suich <> wrote:

   This is an automatically generated e-mail. To reply, visit:
  Review request for cloudstack, Brian Federle, edison su, John Burwell,
and Mike Tutkowski.
By Chris Suich.

*Updated Oct. 17, 2013, 6:46 p.m.*

-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

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.


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/
   - engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/
   - server/src/com/cloud/api/ (f4ca112)
   - server/src/com/cloud/storage/snapshot/

View Diff <>

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message