Sorry, I don't realize you are not a committer yet. BTW, let me nominate you as 
a commiter.

> -----Original Message-----
> From: Devdeep Singh [mailto:devdeep.si...@citrix.com]
> Sent: Thursday, April 18, 2013 2:44 AM
> To: Edison Su; Anthony Xu; Abhinandan Prateek (aprat...@apache.org);
> Alex Huang; dev@cloudstack.apache.org
> Subject: RE: Review Request: Storage motion changes for xenserver
> 
> Hi,
> 
> The feature was given a “Ship It” after the review comments were addressed.
> Can someone apply and commit these changes to the master branch. I have
> verified that the patch applies cleanly to the latest master.
> 
> Regards,
> Devdeep
> 
> From: edison su [mailto:nore...@reviews.apache.org] On Behalf Of edison
> su
> Sent: Wednesday, April 17, 2013 11:53 AM
> To: Anthony Xu; Edison Su; Abhinandan Prateek; Alex Huang
> Cc: cloudstack; Devdeep Singh
> Subject: Re: Review Request: Storage motion changes for xenserver
> 
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10196/
> 
> 
> 
> Ship it!
> 
> Ship It!
> 
> 
> - edison
> 
> 
> On April 15th, 2013, 7:24 a.m., Devdeep Singh wrote:
> Review request for cloudstack, Abhinandan Prateek, edison su, Alex Huang,
> and anthony xu.
> By Devdeep Singh.
> 
> Updated April 15, 2013, 7:24 a.m.
> 
> Description
> 
> Storage motion for Xenserver. FS for the feature
> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Enabling+Storag
> e+XenMotion+for+XenServer
> 
>     1. Implemented Api findStoragePoolsForMigration. Added a new response
> objects to list storage pools available for migration.
> 
>     2. Updated migrateVolume api for allowing migrating volumes of running
> vms. These changes are integrated into the latest storage refactoring
> changes.
> 
>     3. Added the implementation for findHostsForMigration api. It lists the
> hosts to which an instance can be migrated, including hosts from within and
> across clusters to which an instance may be migrated with storage motion.
> The work of migrating a volume of a running vm is also done in copyAsync.
> 
>     4. Updated the listHosts api for backward compatibility.
> 
>     5. Added the implementation for migrateVirtualMachineWithVolume api. It
> migrates an instance with its volumes within a cluster and also across 
> clusters.
> Also introduced a new XenServerStorageMotionStrategy for migrating
> volumes of a vm. When a vm is being migrated with its volumes, the vm is
> put in migrating state and a request is send to the volume manager to
> migrate the vm and its volumes. Volume manager calls into the volume
> service which forwards the request to data motion service after moving all
> the volumes to migrating state. Data motion service enumerates the
> strategies and the request reaches the XenServerStorageMotionStrategy. It
> calls in to the resource to complete the operation.
> 
>     6. Resolved an issue where storage xenmotion of 2nd VM created from the
> same template to a host was failing with duplicate_vm exception. Made
> changes to remove the mac_seed key value pair from other_config when
> vms are created. This is was storage motion to fail.
> 
>     7. Updated the db upgrade schema script.
> 
>     8. Added the right permissions in commands.properties
> 
>     9. Marvin tests for testing storage motion. Following scenarios are 
> tested.
> 
>     9.1. A virtual machine is migrated to another host. Its volumes are also
> migrated to another storage pool.
> 
>     9.2. Just the volumes of a vm are migrated to another storage pool while
> the vm continues to run on the same host.
> 
>     10. Unit tests for testing migration of a vm with its volumes.
> 
> 
> Testing
> 
> 1. Unit tests for testing vm migration with volume. They test when a vm is
> migrated within a cluster or across cluster. Also added negative tests for the
> scenrios.
> 
> 2. Marvin tests to do functional testing. Including tests to varify vm 
> migration
> with volume across cluster.
> 
> 3. Marvin test for volume migration to another storage pool in the cluster
> while the vm continues to run on the same host.
> 
> 4. Also did additional manual testing for the following scenarios:
> 
> 4.1 VM migration with volumes within and across cluster.
> 
> 4.2 Tested both the scenarios when 'migrateto' optional parameter is passed
> to the migrate vm with volume api. When it isn't passed, cloudstack picks up
> a storage pool for migration. When it is passed, the volume is migrated to the
> pool passed in the parameter.
> 
> 4.3 Tested that storage tags are honored when a vm is migrated with its
> volumes.
> 
> 4.4 Tested volume migration when the vm stays on the same host.
> 
> 4.5 For volume migration verified that storage tags are honored.
> 
> 
> 
> Other tests done to verify patch:
> 
> 1. Verified that there are no rat failures.
> 
> 2. Applied the patch to verify it applies cleanly.
> 
> Bugs: https://issues.apache.org/jira/browse/CLOUDSTACK-659
> Diffs
> 
>   *   api/src/com/cloud/agent/api/MigrateWithStorageAnswer.java (PRE-
> CREATION)
>   *   api/src/com/cloud/agent/api/MigrateWithStorageCommand.java (PRE-
> CREATION)
>   *   api/src/com/cloud/agent/api/MigrateWithStorageCompleteAnswer.java
> (PRE-CREATION)
>   *
> api/src/com/cloud/agent/api/MigrateWithStorageCompleteCommand.java
> (PRE-CREATION)
>   *   api/src/com/cloud/agent/api/MigrateWithStorageReceiveAnswer.java
> (PRE-CREATION)
>   *   api/src/com/cloud/agent/api/MigrateWithStorageReceiveCommand.java
> (PRE-CREATION)
>   *   api/src/com/cloud/agent/api/MigrateWithStorageSendAnswer.java
> (PRE-CREATION)
>   *   api/src/com/cloud/agent/api/MigrateWithStorageSendCommand.java
> (PRE-CREATION)
>   *   api/src/com/cloud/agent/api/storage/MigrateVolumeAnswer.java (PRE-
> CREATION)
>   *   api/src/com/cloud/agent/api/storage/MigrateVolumeCommand.java
> (PRE-CREATION)
>   *   api/src/com/cloud/hypervisor/HypervisorCapabilities.java (aff81b0)
>   *   api/src/com/cloud/server/ManagementService.java (6e6dbc3)
>   *   api/src/com/cloud/vm/UserVmService.java (d963b74)
>   *   api/src/org/apache/cloudstack/api/ApiConstants.java (b08e992)
>   *   api/src/org/apache/cloudstack/api/ResponseGenerator.java (c0dd57e)
>   *
> api/src/org/apache/cloudstack/api/command/admin/host/FindHostsForMigr
> ationCmd.java (PRE-CREATION)
>   *
> api/src/org/apache/cloudstack/api/command/admin/host/ListHostsCmd.jav
> a (29844c3)
>   *
> api/src/org/apache/cloudstack/api/command/admin/storage/FindStoragePo
> olsForMigrationCmd.java (PRE-CREATION)
>   *
> api/src/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMac
> hineWithVolumeCmd.java (PRE-CREATION)
>   *
> api/src/org/apache/cloudstack/api/command/user/volume/MigrateVolume
> Cmd.java (287241a)
>   *
> api/src/org/apache/cloudstack/api/response/HostForMigrationResponse.jav
> a (PRE-CREATION)
>   *   api/src/org/apache/cloudstack/api/response/HostResponse.java
> (f5aa8f9)
>   *
> api/src/org/apache/cloudstack/api/response/StoragePoolForMigrationResp
> onse.java (PRE-CREATION)
>   *   api/src/org/apache/cloudstack/api/response/StoragePoolResponse.java
> (0b16226)
>   *   client/tomcatconf/applicationContext.xml.in (15cd6fe)
>   *   client/tomcatconf/commands.properties.in (798d226)
>   *   core/src/com/cloud/hypervisor/HypervisorCapabilitiesVO.java (fafc0a3)
>   *
> engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/Obje
> ctInDataStoreStateMachine.java (f619ef4)
>   *
> engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/Volu
> meService.java (102c471)
>   *
> engine/storage/imagemotion/src/org/apache/cloudstack/storage/image/m
> otion/DefaultImageMotionStrategy.java (a70fd8a)
>   *   engine/storage/integration-
> test/test/org/apache/cloudstack/storage/test/MockStorageMotionStrategy.
> java (b619ee9)
>   *
> engine/storage/src/org/apache/cloudstack/storage/motion/AncientDataMo
> tionStrategy.java (3602bb1)
>   *
> engine/storage/src/org/apache/cloudstack/storage/motion/DataMotionSer
> vice.java (db36f64)
>   *
> engine/storage/src/org/apache/cloudstack/storage/motion/DataMotionSer
> viceImpl.java (343140f)
>   *
> engine/storage/src/org/apache/cloudstack/storage/motion/DataMotionStra
> tegy.java (ba40c6d)
>   *
> engine/storage/volume/src/org/apache/cloudstack/storage/volume/Volum
> eObject.java (ceadb25)
>   *
> engine/storage/volume/src/org/apache/cloudstack/storage/volume/Volum
> eServiceImpl.java (32e7d27)
>   *   plugins/host-
> allocators/random/src/com/cloud/agent/manager/allocator/impl/RandomAll
> ocator.java (a672efd)
>   *
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixReso
> urceBase.java (4ef583a)
>   *
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServe
> r56FP1Resource.java (d64e173)
>   *
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServe
> r610Resource.java (8d267b1)
>   *
> plugins/hypervisors/xen/src/org/apache/cloudstack/storage/motion/XenSer
> verStorageMotionStrategy.java (PRE-CREATION)
>   *   server/src/com/cloud/agent/manager/allocator/HostAllocator.java
> (60027e7)
>   *
> server/src/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java
> (0091e43)
>   *
> server/src/com/cloud/agent/manager/allocator/impl/TestingAllocator.java
> (90bd956)
>   *   server/src/com/cloud/api/ApiDBUtils.java (303f328)
>   *   server/src/com/cloud/api/ApiResponseHelper.java (50c137a)
>   *   server/src/com/cloud/api/query/ViewResponseHelper.java (dc2727e)
>   *   server/src/com/cloud/api/query/dao/HostJoinDao.java (1a21299)
>   *   server/src/com/cloud/api/query/dao/HostJoinDaoImpl.java (1adff40)
>   *   server/src/com/cloud/api/query/dao/StoragePoolJoinDao.java (bbb0242)
>   *   server/src/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java
> (58968df)
>   *   server/src/com/cloud/server/ManagementServerImpl.java (d9a4317)
>   *   server/src/com/cloud/storage/VolumeManager.java (2101038)
>   *   server/src/com/cloud/storage/VolumeManagerImpl.java (1e8edaf)
>   *   server/src/com/cloud/vm/UserVmManagerImpl.java (d281e5b)
>   *   server/src/com/cloud/vm/VirtualMachineManager.java (4a30d97)
>   *   server/src/com/cloud/vm/VirtualMachineManagerImpl.java (4072531)
>   *   server/test/com/cloud/vm/MockUserVmManagerImpl.java (fd826d9)
>   *   server/test/com/cloud/vm/MockVirtualMachineManagerImpl.java
> (4917e77)
>   *   server/test/com/cloud/vm/VirtualMachineManagerImplTest.java
> (322f051)
>   *   setup/db/db/schema-410to420.sql (92b2d9c)
>   *   test/integration/component/test_storage_motion.py (PRE-CREATION)
>   *   tools/marvin/marvin/integration/lib/base.py (3df68ab)
> 
> View Diff<https://reviews.apache.org/r/10196/diff/>
> 

Reply via email to