incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chip Childers <chip.child...@sungard.com>
Subject Re: [MERGE] Support VM Snapshot
Date Wed, 30 Jan 2013 03:11:22 GMT
On Jan 29, 2013, at 9:20 PM, Mice Xia <mice_xia@tcloudcomputing.com> wrote:

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

We don't, other than a general consensus to improve coverage whenever
possible. ;)

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

I was thinking about the admin guide mostly. It's certainly not
required before bringing in the feature, but it should probably get
documented before the release.

Sounds like your ready to merge!

+1 from me

> 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