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+Storage+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/FindHostsForMigrationCmd.java (PRE-CREATION) * api/src/org/apache/cloudstack/api/command/admin/host/ListHostsCmd.java (29844c3) * api/src/org/apache/cloudstack/api/command/admin/storage/FindStoragePoolsForMigrationCmd.java (PRE-CREATION) * api/src/org/apache/cloudstack/api/command/admin/vm/MigrateVirtualMachineWithVolumeCmd.java (PRE-CREATION) * api/src/org/apache/cloudstack/api/command/user/volume/MigrateVolumeCmd.java (287241a) * api/src/org/apache/cloudstack/api/response/HostForMigrationResponse.java (PRE-CREATION) * api/src/org/apache/cloudstack/api/response/HostResponse.java (f5aa8f9) * api/src/org/apache/cloudstack/api/response/StoragePoolForMigrationResponse.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/ObjectInDataStoreStateMachine.java (f619ef4) * engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VolumeService.java (102c471) * engine/storage/imagemotion/src/org/apache/cloudstack/storage/image/motion/DefaultImageMotionStrategy.java (a70fd8a) * engine/storage/integration-test/test/org/apache/cloudstack/storage/test/MockStorageMotionStrategy.java (b619ee9) * engine/storage/src/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java (3602bb1) * engine/storage/src/org/apache/cloudstack/storage/motion/DataMotionService.java (db36f64) * engine/storage/src/org/apache/cloudstack/storage/motion/DataMotionServiceImpl.java (343140f) * engine/storage/src/org/apache/cloudstack/storage/motion/DataMotionStrategy.java (ba40c6d) * engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java (ceadb25) * engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java (32e7d27) * plugins/host-allocators/random/src/com/cloud/agent/manager/allocator/impl/RandomAllocator.java (a672efd) * plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java (4ef583a) * plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServer56FP1Resource.java (d64e173) * plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/XenServer610Resource.java (8d267b1) * plugins/hypervisors/xen/src/org/apache/cloudstack/storage/motion/XenServerStorageMotionStrategy.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/>