> On Oct. 8, 2013, 8:18 p.m., edison su wrote:
> > ui/scripts/storage.js, line 1763
> > <https://reviews.apache.org/r/14522/diff/1/?file=362033#file362033line1763>
> >
> >     The ui change here, is there way to disable it from ui, if the storage 
> > provider is not NetApp? Or move the ui change into your plugin?

This raises the question of whether people expect to see the revert snapshot 
functionality for hypervisors or just storage providers. I figured that the 
hypervisor functionality would be desired, but it sounds like that may not be 
the case for all hypervisors.

Has there been any thoughts to allow storage providers to indicate which 
features they support? Maybe part of the VolumeResponse can be a set of flags 
for which operations are supported (take snapshot, revert snapshot, etc.). This 
way, the UI can dynamically show/hide supported actions without knowing who the 
volume's storage provider actually is. This should be a fairly straight forward 
UI change, but would require adding methods to the storage provider interface. 
If we don't want to always load this information just for the VolumeResponse, 
we could expose new APIs to query which operations are supported for a given 
volume, but we may not want to go exposing APIs for this.

Any thoughts?


- Chris


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


On Oct. 7, 2013, 8:26 p.m., Chris Suich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14522/
> -----------------------------------------------------------
> 
> (Updated Oct. 7, 2013, 8:26 p.m.)
> 
> 
> Review request for cloudstack, Brian Federle and edison su.
> 
> 
> 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
> -----
> 
>   
> api/src/org/apache/cloudstack/api/command/user/snapshot/RevertSnapshotCmd.java
>  946eebd 
>   client/WEB-INF/classes/resources/messages.properties f92b85a 
>   client/tomcatconf/commands.properties.in 58c770d 
>   
> engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java
>  c09adca 
>   server/src/com/cloud/server/ManagementServerImpl.java 0a0fcdc 
>   server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 0b53cfd 
>   ui/dictionary.jsp f93f9dc 
>   ui/scripts/storage.js 88fb9f2 
> 
> 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