Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
Received-SPF: pass (nike.apache.org: domain of Chris.Suich@netapp.com
 designates 216.240.18.76 as permitted sender)
From: "SuichII, Christopher" <Chris.Suich@netapp.com>
To: John Burwell <jburwell@basho.com>, "<dev@cloudstack.apache.org>"
	<dev@cloudstack.apache.org>
Subject: Re: Review Request 14522: [CLOUDSTACK-4771] Support Revert VM Disk
 from Snapshot
Thread-Topic: Review Request 14522: [CLOUDSTACK-4771] Support Revert VM Disk
 from Snapshot
Thread-Index: AQHOw5vCWbkQWP6/jE+w71BZ/XzSKpn1TcWAgAKeZACAAdRkAIAF8VEA
Date: Mon, 21 Oct 2013 13:31:24 +0000
Message-ID: <9F005B7C-5E3F-4AE1-A604-0D52246F85CC@netapp.com>
References: <20131016145012.7309.7277@reviews.apache.org>
 <20131017184638.29093.8876@reviews.apache.org>
In-Reply-To: <20131017184638.29093.8876@reviews.apache.org>
Accept-Language: en-US
Content-Language: en-US
Content-Type: multipart/alternative;
	boundary="_000_9F005B7C5E3F4AE1A6040D52246F85CCnetappcom_"
MIME-Version: 1.0

--_000_9F005B7C5E3F4AE1A6040D52246F85CCnetappcom_
Content-Type: text/plain; charset="Windows-1252"
Content-Transfer-Encoding: quoted-printable

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 =96 Cloud Solutions
Citrix, Cisco & Red Hat

On Oct 17, 2013, at 2:46 PM, Chris Suich <Chris.Suich@netapp.com<mailto:Chr=
is.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 Snaps=
hotStrategies is not able to actually throw error messages. I will open a s=
eparate 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 sna=
pshot 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 w=
ith 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 add=
ed the logic in a similar fashion to takeSnapshot(), backupSnapshot() and d=
eleteSnapshot().

I have also added a 'Revert to Snapshot' action to the volume snapshots lis=
t 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 k=
now.


Diffs (updated)

  *   engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/Str=
ategyPriority.java (81034b1)
  *   engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/Da=
taMotionServiceImpl.java (2d31320)
  *   server/src/com/cloud/api/ApiResponseHelper.java (f4ca112)
  *   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java (dade9=
83)

View Diff<https://reviews.apache.org/r/14522/diff/>


--_000_9F005B7C5E3F4AE1A6040D52246F85CCnetappcom_--