> On Oct. 4, 2013, 8:23 p.m., Chip Childers wrote: > > api/src/org/apache/cloudstack/api/command/user/snapshot/RevertSnapshotCmd.java, > > line 36 > > <https://reviews.apache.org/r/14477/diff/1/?file=361268#file361268line36> > > > > Are there any unit or integration tests for this new API call? I can't > > find any in this diff.
There is no implementation for revertsnapshot yet in the current storage drivers. If any storage vendor wants to implement this feature, they can add their implementation in their storage driver. Also, if anybody is interested in implementing this feature in the default storage driver, for example, for Ceph, then it's the place to start with. > On Oct. 4, 2013, 8:23 p.m., Chip Childers wrote: > > core/src/org/apache/cloudstack/storage/command/ForgetObjectCmd.java, line 24 > > <https://reviews.apache.org/r/14477/diff/1/?file=361271#file361271line24> > > > > Are there any unit or integration tests for this new API call? I can't > > find any in this diff. It's not an api, it's just a simple command will send to hypervisor host, will be used by HypervisorHelper to introduce/forget an object on hypervisor host. > On Oct. 4, 2013, 8:23 p.m., Chip Childers wrote: > > engine/storage/src/org/apache/cloudstack/storage/helper/HypervisorHelperImpl.java, > > lines 35-76 > > <https://reviews.apache.org/r/14477/diff/1/?file=361287#file361287line35> > > > > I feel like this new code should have unit test for the if logic trees. The logic here is very simple: just send a command to hypervisor host. Not sure need unit test here or not. > On Oct. 4, 2013, 8:23 p.m., Chip Childers wrote: > > plugins/hypervisors/simulator/src/com/cloud/resource/SimulatorStorageProcessor.java, > > lines 226-236 > > <https://reviews.apache.org/r/14477/diff/1/?file=361289#file361289line226> > > > > Can we get rid of these TODOs? Yes, I can remove the TODO > On Oct. 4, 2013, 8:23 p.m., Chip Childers wrote: > > server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java, lines > > 264-283 > > <https://reviews.apache.org/r/14477/diff/1/?file=361292#file361292line264> > > > > Unit tests for this if logic? Again, straight forward code, not sure need add unit test here. - edison ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14477/#review26694 ----------------------------------------------------------- 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 > >
