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/>


Reply via email to