cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Suich" <chris.su...@netapp.com>
Subject Re: Review Request 14522: [CLOUDSTACK-4771] Support Revert VM Disk from Snapshot
Date Wed, 16 Oct 2013 14:50:12 GMT

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

(Updated Oct. 16, 2013, 2:50 p.m.)


Review request for cloudstack, Brian Federle, edison su, John Burwell, and Mike Tutkowski.


Changes
-------

In this new revision, I have:

-Renamed some of the members and methods
-Dropped the changes to the PrimaryDataStoreDriver
-Changed from sorting the strategies to simply choosing one of the highest priorities
-Started throwing an exception if there was an error determining whether the snapshot can
be reverted

After these changes, it appears there is only one remaining issue as the others have either
been addressed or the associated code has been dropped.


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 (updated)
-----

  api/src/org/apache/cloudstack/api/ApiConstants.java 62eed09 
  api/src/org/apache/cloudstack/api/response/SnapshotResponse.java e9cb109 
  engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/PrimaryDataStoreDriver.java
b124d83 
  engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotInfo.java 8d6b760

  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/DataMotionServiceImpl.java
2d31320 
  engine/storage/integration-test/test/org/apache/cloudstack/storage/test/FakePrimaryDataStoreDriver.java
810afd1 
  engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTest.java
81f77d6 
  engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotObject.java 3f35e1d

  engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java
6a874d6 
  plugins/storage/volume/default/src/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java
3eaeb1f 
  plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/driver/SamplePrimaryDataStoreDriverImpl.java
ece7b26 
  plugins/storage/volume/solidfire/src/org/apache/cloudstack/storage/datastore/driver/SolidfirePrimaryDataStoreDriver.java
8046b6c 
  server/src/com/cloud/api/ApiResponseHelper.java f4ca112 
  server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java dade983 
  ui/scripts/storage.js b16f4d4 

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


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