----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14477/#review26694 -----------------------------------------------------------
Some questions and comments that probably need to be answered before this is committed. Thanks Chris! api/src/org/apache/cloudstack/api/command/user/snapshot/RevertSnapshotCmd.java <https://reviews.apache.org/r/14477/#comment51994> Are there any unit or integration tests for this new API call? I can't find any in this diff. core/src/org/apache/cloudstack/storage/command/ForgetObjectCmd.java <https://reviews.apache.org/r/14477/#comment51995> Are there any unit or integration tests for this new API call? I can't find any in this diff. core/src/org/apache/cloudstack/storage/command/IntroduceObjectCmd.java <https://reviews.apache.org/r/14477/#comment51996> Are there any unit or integration tests for this new API call? I can't find any in this diff. engine/storage/src/org/apache/cloudstack/storage/helper/HypervisorHelperImpl.java <https://reviews.apache.org/r/14477/#comment51997> I feel like this new code should have unit test for the if logic trees. plugins/hypervisors/simulator/src/com/cloud/resource/SimulatorStorageProcessor.java <https://reviews.apache.org/r/14477/#comment51998> Can we get rid of these TODOs? server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java <https://reviews.apache.org/r/14477/#comment52000> Unit tests for this if logic? - Chip Childers On Oct. 4, 2013, 12:57 a.m., Chris Suich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14477/ > ----------------------------------------------------------- > > (Updated Oct. 4, 2013, 12:57 a.m.) > > > Review request for cloudstack. > > > Repository: cloudstack-git > > > Description > ------- > > These changes are a joint effort between Edison and I to refactor some of the > code around snapshotting VM volumes and creating templates/volumes from VM > volume snapshots. In general, we were working towards allowing > PrimaryDataStoreDrivers to create snapshots on primary storage and not > requiring the snapshots to be transferred to secondary storage. > > High level changes: > -Added uuid to NfsTO, SwiftTO & S3TO to cut down on the requirement of > PrimaryDataStoreTO and ImageStoreTO which don't really serve much of a purpose > -Initial work towards enable reverting VM volume from snapshots > -Added hypervisor commands for introducing and forgetting new hypervisor > objects (snapshots, templates & volumes) > > > Diffs > ----- > > api/src/com/cloud/agent/api/to/DataStoreTO.java 9014f8e > api/src/com/cloud/agent/api/to/NfsTO.java 415c95c > api/src/com/cloud/agent/api/to/SwiftTO.java 7349d77 > api/src/com/cloud/event/EventTypes.java ec9604e > api/src/com/cloud/storage/snapshot/SnapshotApiService.java 23e6522 > > api/src/org/apache/cloudstack/api/command/admin/storage/ListStoragePoolsCmd.java > 26351bb > > api/src/org/apache/cloudstack/api/command/user/snapshot/RevertSnapshotCmd.java > PRE-CREATION > core/src/com/cloud/storage/resource/StorageProcessor.java 5fa9f8a > core/src/com/cloud/storage/resource/StorageSubsystemCommandHandlerBase.java > ab9aa2a > core/src/org/apache/cloudstack/storage/command/ForgetObjectCmd.java > PRE-CREATION > core/src/org/apache/cloudstack/storage/command/IntroduceObjectAnswer.java > PRE-CREATION > core/src/org/apache/cloudstack/storage/command/IntroduceObjectCmd.java > PRE-CREATION > core/src/org/apache/cloudstack/storage/to/ImageStoreTO.java 0037ea5 > core/src/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java 5e870df > > engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/EndPointSelector.java > ca0cc2c > > engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotService.java > d594a07 > > engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/SnapshotStrategy.java > 86ae532 > > engine/storage/datamotion/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java > 96d1f5a > > engine/storage/image/src/org/apache/cloudstack/storage/image/store/ImageStoreImpl.java > 855d8cb > > engine/storage/integration-test/test/org/apache/cloudstack/storage/test/SnapshotTestWithFakeData.java > 2aaabda > > engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java > 3ead93f > > engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/SnapshotStrategyBase.java > 1b57922 > > engine/storage/snapshot/src/org/apache/cloudstack/storage/snapshot/XenserverSnapshotStrategy.java > 60d9407 > > engine/storage/src/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java > fdc12bf > > engine/storage/src/org/apache/cloudstack/storage/helper/HypervisorHelper.java > PRE-CREATION > > engine/storage/src/org/apache/cloudstack/storage/helper/HypervisorHelperImpl.java > PRE-CREATION > > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java > 82fd2ce > > plugins/hypervisors/simulator/src/com/cloud/resource/SimulatorStorageProcessor.java > c7768aa > > plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java > 4982d87 > > plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServerStorageProcessor.java > 739b974 > server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 2297e6a > > services/secondary-storage/src/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java > 3ef950b > > Diff: https://reviews.apache.org/r/14477/diff/ > > > Testing > ------- > > > Thanks, > > Chris Suich > >
