Sorry for the late. Following is my comments on storage motion:
1. We shouldn't touch volume state in virutalmachinemanager. I spend a lot of
time trying to move volume related operations from virtualmachinemanager to
storage subsystem, better to follow the same pattern.
2. The current storage subsystem apis, can only operate on one data object
(volume/snapshot or template) at one time. In storage migration case, it has to
deal with multiple volumes at one time.
My suggestion is that add a new method, like, migrateVolumes(Map<volumeInfo,
DataStore> volumeMaps), which can migrate a group of volumes at one time.
3. The code follow will look like:
virtualmachineManagerImpl:migrateWithStorage(change vm state) -> volumemanager:
migrateVolumes{do basic check} -> volumeService:migrateVolumes{which will
change volume state} -> datamotionservice:copyAsync(which will find a data
motion strategy which can handle storage migration for xenserver) ->
xenserverstoragemigrationstategy:copyAsync(send commands to xenserver resource )
You need to write a datamotion strategy for xenserver storage migration, and
need to add a method on both datamotionservice, volumeservice, volumemanager
and DataMotionStrategy, so that they can operate on multiple volumes at one
time.
4. Don't need to add a new api called migrateasync, copyasync has the same
meaning.
Does it make sense?
> -----Original Message-----
> From: Devdeep Singh [mailto:devdeep.si...@citrix.com]
> Sent: Friday, March 29, 2013 8:21 AM
> To: dev@cloudstack.apache.org
> Subject: Review request for storage motion on xenserver
>
> I have put the feature proposed [1] and developed in the feature branch [2]
> up for review. Code for this feature conforms to what was proposed in FS [3].
> The patch available at [4]. It includes marvin tests and unit tests for
> verifying
> the functionality. Please take a look at it and let me know your comments.
>
> [1] http://markmail.org/message/numdk6pdab2hekdp
> [2] https://github.com/devdeep/cloudstack/commits/sm3
> [3]
> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Enabling+Storag
> e+XenMotion+for+XenServer
> [4] https://reviews.apache.org/r/10196/
>
> Regards,
> Devdeep
