cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "SuichII, Christopher" <Chris.Su...@netapp.com>
Subject Re: Review Request 14522: [CLOUDSTACK-4771] Support Revert VM Disk from Snapshot
Date Mon, 21 Oct 2013 13:31:24 GMT
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.suich@netapp.com<mailto:chris.suich@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.Suich@netapp.com<mailto:Chris.Suich@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/>



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