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:[email protected]] > Sent: Thursday, April 18, 2013 2:44 AM > To: Edison Su; Anthony Xu; Abhinandan Prateek ([email protected]); > Alex Huang; [email protected] > 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:[email protected]] 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/> >
