cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Devdeep Singh <devdeep.si...@citrix.com>
Subject RE: Review request for storage motion on xenserver
Date Thu, 11 Apr 2013 11:19:38 GMT
Hi Edison,

I have updated the patch [1] after incorporating your review comments. It also includes the
changes to address the review comments given by Alex. Kindly take a look at the changes and
let me know your comments.

[1] https://reviews.apache.org/r/10196/

Regards,
Devdeep

> -----Original Message-----
> From: Devdeep Singh [mailto:devdeep.singh@citrix.com]
> Sent: Friday, April 05, 2013 3:37 PM
> To: dev@cloudstack.apache.org
> Subject: RE: Review request for storage motion on xenserver
> 
> Hi Edison,
> 
> Thanks a lot for looking at the changes. I am going through your feedback and
> working on a patch which addresses your review comments. I'll get back to
> you if I have any queries.
> 
> Regards,
> Devdeep
> 
> > -----Original Message-----
> > From: Edison Su [mailto:Edison.su@citrix.com]
> > Sent: Thursday, April 04, 2013 3:21 AM
> > To: dev@cloudstack.apache.org
> > Subject: RE: Review request for storage motion on xenserver
> >
> > 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.singh@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+Stor
> > > ag
> > > e+XenMotion+for+XenServer
> > > [4] https://reviews.apache.org/r/10196/
> > >
> > > Regards,
> > > Devdeep

Mime
View raw message