incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mice Xia" <mice_...@tcloudcomputing.com>
Subject RE: [MERGE] Support VM Snapshot
Date Wed, 30 Jan 2013 02:20:00 GMT
Hi, Chip

1) You state that it's based off of 4.0.  Do you need to rebase against master?
[mice] already rebased against master

2) Are you using reviewboard as a method for easy review of the changes, but will be committing
to master via a merge in the repo?  Or will you pull the patch into master from reviewboard?
 I think that once you get consensus on the changes, you have commit rights to do either.
[mice] will commit to master via a merge in the repo

3) Are there any unit tests that test the new classes and methods?
[mice] Yes, VMSnapshotManagerTest.java for testing creation case in VMSnapshotManager. But
I'm not sure if this is sufficient, do we have some guideline about unit test coverage?

4) Are there any doc updates required?
[mice] test scenarios need to be elaborated in the spec, currently I only listed some fast
acceptance cases.

Regards
-Mice

-----Original Message-----
From: Chip Childers [mailto:chip.childers@sungard.com] 
Sent: Tuesday, January 29, 2013 10:34 PM
To: cloudstack-dev@incubator.apache.org
Subject: Re: [MERGE] Support VM Snapshot

On Mon, Jan 28, 2013 at 9:35 PM, Mice Xia <mice_xia@tcloudcomputing.com> wrote:
> I would like to request merge of feature VM snapshot to master
>
> Review board:
> https://reviews.apache.org/r/9118/
>
> Doc/spec
> https://cwiki.apache.org/confluence/display/CLOUDSTACK/VM+Snapshots
>
> Jira ticket
> https://issues.apache.org/jira/browse/CLOUDSTACK-684
>
> Branch
> vm-snapshot (which is based on 4.0.0)
>
> Potential impact and notes:
> * 4 new states are added to VM state Machine
> * the case 'migrating volume to other primary storage with associated 
> VM snapshots' is not yet handled
> * KVM support is not merged due to expected libvirt library has not released.
> * UI change is included.
>
>
> Regards
> Mice

I'd hope for one of the more knowledgeable engineers to review the implementation details,
but the patch looks reasonable.

Some questions though:

1) You state that it's based off of 4.0.  Do you need to rebase against master?
2) Are you using reviewboard as a method for easy review of the changes, but will be committing
to master via a merge in the repo?  Or will you pull the patch into master from reviewboard?
 I think that once you get consensus on the changes, you have commit rights to do either.
3) Are there any unit tests that test the new classes and methods?
4) Are there any doc updates required?

-chip
Mime
View raw message