cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Hieu LE <hieul...@gmail.com>
Subject Re: Review Request 22799: Golden (Base) Primary Storage feature
Date Mon, 21 Jul 2014 06:40:21 GMT
Thank you, Sudha and Mike,

So I will implement some regression test on Marvin to ensure this feature.

BRs.


On Mon, Jul 21, 2014 at 11:30 AM, Mike Tutkowski <
mike.tutkowski@solidfire.com> wrote:

> Thanks for the note, Sudha.
>
> I was not aware we were treating Marvin tests are a requirement. That is
> interesting to know as I've not seen that rule followed all the time (as
> you say, perhaps people have been backing off on that informally).
>
> I agree, though, that automated tests are critical and going to become
> more so as additional features continue to be added to CloudStack.
>
>
>  On Sun, Jul 20, 2014 at 10:23 PM, Sudha Ponnaganti <
> sudha.ponnaganti@citrix.com> wrote:
>
>> Mike / Hieu,
>>
>> Thank you for your consideration. Even if it is not needed at the review
>> time, can Marvin tests ( if applicable, simulator tests) can be added
>> before you close on the feature. That would help to run regression tests on
>> this feature for all releases.
>>
>> Some sort of automated tests ( unit tests or Marvin tests in place of
>> unit tests) must be checked in for feature to be accepted. This was
>> followed closely in the earlier releases and slowly this is being ignored.
>> Can reviewers be more stringent regarding this merge criteria.
>>
>> Thanks
>> /Sudha
>>
>> -----Original Message-----
>> From: Mike Tutkowski [mailto:mike.tutkowski@solidfire.com]
>> Sent: Sunday, July 20, 2014 9:16 PM
>> To: dev@cloudstack.apache.org; Hieu LE
>> Cc: Tim Mackey
>> Subject: Re: Review Request 22799: Golden (Base) Primary Storage feature
>>
>> Thanks!
>>
>> Adding Marvin tests is not a prerequisite to your code being committed. I
>> just strongly recommend you consider such tests.
>>
>> Essentially it is ideal to have Marvin tests, but not required.
>>
>> I'm glad to see the list of tests you've performed manually. Thanks for
>> adding that to Review Board.
>>
>>
>> On Sun, Jul 20, 2014 at 9:59 PM, Hieu LE <hieulq19@gmail.com> wrote:
>>
>> >
>> >
>> > > On July 18, 2014, 4:16 a.m., Mike Tutkowski wrote:
>> > > > Hi,
>> > > >
>> > > > It's been a while since we've had any activity review wise on this
>> > feature.
>> > > >
>> > > > Can you guys tell me where we're currently at?
>> > > >
>> > > > Thanks!
>> > > > Mike
>> >
>> > Sorry Mike,
>> >
>> > There are some troubles with my machines last week.
>> >
>> > I have updated new diff and adding integration tests. Fooling around
>> > with marvin is great but may be I need more times with it.
>> >
>> > Thanks!
>> >
>> > Hieu LE
>> >
>> >
>> > - Hieu
>> >
>> >
>> > -----------------------------------------------------------
>> > This is an automatically generated e-mail. To reply, visit:
>> > https://reviews.apache.org/r/22799/#review48109
>> > -----------------------------------------------------------
>> >
>> >
>> > On July 21, 2014, 3:56 a.m., Hieu LE wrote:
>> > >
>> > > -----------------------------------------------------------
>> > > This is an automatically generated e-mail. To reply, visit:
>> > > https://reviews.apache.org/r/22799/
>> > > -----------------------------------------------------------
>> > >
>> > > (Updated July 21, 2014, 3:56 a.m.)
>> > >
>> > >
>> > > Review request for cloudstack, Mike Tutkowski and Tim Mackey.
>> > >
>> > >
>> > > Repository: cloudstack-git
>> > >
>> > >
>> > > Description
>> > > -------
>> > >
>> > > As discussed in mailing list, this patch is applied for golden
>> > > primary
>> > storage in [1].
>> > > I have changed the term from "golden" to "base" because there are
>> > > some
>> > functions and variables in CloudStack also use "base" for base image.
>> > > This patch only apply for Xen Server.
>> > >
>> > > [1]:
>> > https://cwiki.apache.org/confluence/display/CLOUDSTACK/Golden+Primary+
>> > Storage
>> > >
>> > >
>> > > Diffs
>> > > -----
>> > >
>> > >   api/src/com/cloud/deploy/DeployDestination.java
>> > 4ded5ebe7a18252da471ee25019856f2b2f772e0
>> > >   api/src/com/cloud/storage/StoragePool.java
>> > 8e03c3348f3a6dd3156ab9e440126ea317957dc0
>> > >   api/src/com/cloud/template/VirtualMachineTemplate.java
>> > 599212bb039fdbb78511019e8f0a6ea4b4a84440
>> > >   api/src/org/apache/cloudstack/api/ApiConstants.java
>> > ae5d6f05b6b52f60b151369a641cb11fcbb558af
>> > >   api/src/org/apache/cloudstack/api/BaseUpdateTemplateOrIsoCmd.java
>> > 2350f6b389203e2c6cc2182fe03fe9a95e936b81
>> > >
>> > api/src/org/apache/cloudstack/api/command/admin/storage/CreateStorageP
>> > oolCmd.java ae44bc9373232d242e4ebdcf76844969f0fe69fc
>> > >
>> > api/src/org/apache/cloudstack/api/command/admin/storage/ListStoragePoo
>> > lsCmd.java
>> > ed123db
>> > >
>> > api/src/org/apache/cloudstack/api/command/admin/storage/UpdateStorageP
>> > oolCmd.java 3d1a77353257c814efaf60875ffdf99603bc414e
>> > >
>> > api/src/org/apache/cloudstack/api/command/user/template/RegisterTempla
>> > teCmd.java f478c9bc8eebf867a03deb4add1bf695ac3ec0ad
>> > >
>> > > api/src/org/apache/cloudstack/api/response/StoragePoolResponse.java
>> > 3571866fe74dca9aa5fe0d11373313eab97e94ac
>> > >   api/src/org/apache/cloudstack/api/response/TemplateResponse.java
>> > 3e21043e339103c021d3c9e767acac8b3837f760
>> > >   core/src/com/cloud/agent/api/CheckPoolBelongToHostAnswer.java
>> > PRE-CREATION
>> > >   core/src/com/cloud/agent/api/CheckPoolBelongToHostCommand.java
>> > PRE-CREATION
>> > >   core/src/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java
>> > 29e53b0d9581f764a17ea285606213d2c045b029
>> > >   core/src/org/apache/cloudstack/storage/to/TemplateObjectTO.java
>> > b201c386f4975913f13c575d7685e50cedc7d92f
>> > >
>> > core/test/org/apache/cloudstack/api/agent/test/BackupSnapshotCommandTe
>> > st.java
>> > 33361e87265df05e00bfa6dba810d2b68ae8d923
>> > >
>> > core/test/org/apache/cloudstack/api/agent/test/CheckNetworkAnswerTest.
>> > java
>> > 66feaecb5ef20053db50956e2801fec096a350c9
>> > >
>> > core/test/org/apache/cloudstack/api/agent/test/SnapshotCommandTest.jav
>> > a
>> > 114c8854d1504436523aa99c78bf2b4d84a12077
>> > >
>> > engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/Prim
>> > aryDataStoreParameters.java
>> > 1dbff59a8911ad8f0933ef17a2c2b1d3e33523b9
>> > >
>> > engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/Stor
>> > agePoolAllocator.java
>> > dfdbd8ab92c47799f6ad23637fa63e030f0be968
>> > >
>> > engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/Volu
>> > meInfo.java
>> > f93f4efac83c565cd33eb7eb67dcaca335f1c226
>> > >
>> > engine/components-api/src/com/cloud/deploy/DeploymentPlanningManager.j
>> > ava ee6721ab445a5222d0087dc9170e0b58f9eef91a
>> > >
>> > > engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
>> > 4aa5fc80d9660d2f985db98124c33465bd99767f
>> > >
>> > engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api
>> > /VMEntityManagerImpl.java
>> > b1ac2f853374d6f1ddd9087919dbc16db0433f59
>> > >
>> > engine/orchestration/src/org/apache/cloudstack/engine/orchestration/Vo
>> > lumeOrchestrator.java 6256e2526ef9bd4632a5e3873c4d9531eb301c7f
>> > >   engine/schema/src/com/cloud/storage/VMTemplateVO.java
>> > 9a77cbf873aa9e422985fbcdc0ae7e18b8c78d4c
>> > >   engine/schema/src/com/cloud/storage/VolumeVO.java
>> > e328253a596891029c2b55bea81b7ead425251ee
>> > >
>> > engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDa
>> > taStoreDao.java
>> > a976bfbf6fe46306d20ad939c335bba6b9b7be54
>> > >
>> > engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDa
>> > taStoreDaoImpl.java
>> > 92793f1fb1a08a455a78667ba4a39ae162378360
>> > >
>> > engine/schema/src/org/apache/cloudstack/storage/datastore/db/StoragePo
>> > olVO.java
>> > 1508ce0b28c83968c25d9601b6dae34e1a73dbb0
>> > >
>> > engine/storage/image/src/org/apache/cloudstack/storage/image/store/Tem
>> > plateObject.java
>> > 7288d454c30fdb81445e43549145f1f2da8533e4
>> > >
>> > engine/storage/src/org/apache/cloudstack/storage/allocator/ClusterScop
>> > eStoragePoolAllocator.java
>> > ea084c7555468001a12376640d9785b1cf852948
>> > >
>> > engine/storage/src/org/apache/cloudstack/storage/allocator/LocalStorag
>> > ePoolAllocator.java 446e101141bafde28615d766fdffd3a36ee8f3ce
>> > >
>> > engine/storage/src/org/apache/cloudstack/storage/image/TemplateEntityI
>> > mpl.java
>> > c1aa8c2f0d49eb6bc6ff124dd4d87b7b714f62e9
>> > >
>> > engine/storage/src/org/apache/cloudstack/storage/volume/datastore/Prim
>> > aryDataStoreHelper.java
>> > 6b129755009413faae6685a62cfb3ae7b62b42f3
>> > >
>> > engine/storage/volume/src/org/apache/cloudstack/storage/datastore/Prim
>> > aryDataStoreImpl.java
>> > f3c9e790277a4dc27fa9e572138c5d932be87b74
>> > >
>> > engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeO
>> > bject.java
>> > f2b4c9532a62ae917b351574523cc8b3014a4394
>> > >
>> > engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeS
>> > erviceImpl.java
>> > 3a71147f8aabb791d0bfc10624496f35f04195d7
>> > >
>> > plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resou
>> > rce/CitrixResourceBase.java
>> > 1af4579c43e2ab3b2e2154e62b68ba9e43f4b040
>> > >
>> > plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resou
>> > rce/XenServerStorageProcessor.java
>> > 9c86fbed82d1e3789171377a7a2e3d117b49703b
>> > >
>> > plugins/storage-allocators/random/src/org/apache/cloudstack/storage/al
>> > locator/RandomStoragePoolAllocator.java
>> > 83c23c2a5367dc329d7fe1a523dccf5b134b7cd8
>> > >
>> > plugins/storage/volume/default/src/org/apache/cloudstack/storage/datas
>> > tore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
>> > 3c1b76a62d3e3380a014e78303fd8861cf0ccf95
>> > >   scripts/vm/hypervisor/xenserver/vmopsSnapshot
>> > 5fd69a633f8d72321010c8c9c261a24d1be26f5a
>> > >   server/src/com/cloud/api/query/QueryManagerImpl.java
>> > 1182be575a60d16f9f8bed091ee9934fbcc775ef
>> > >   server/src/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java
>> > 1d89b19305749e5661d88e827074c6fd190c35f6
>> > >   server/src/com/cloud/api/query/dao/TemplateJoinDaoImpl.java
>> > 80ef0f6ed7d905cce378ece77e7cea324341e9c9
>> > >   server/src/com/cloud/api/query/vo/StoragePoolJoinVO.java
>> > 565e290bd7044fc996ecd953d83e6f9443694574
>> > >   server/src/com/cloud/api/query/vo/TemplateJoinVO.java
>> > 834a9cedd07124583208005864e540350a09702f
>> > >   server/src/com/cloud/deploy/DeploymentPlanningManagerImpl.java
>> > db6fa5fee833c0d6e4c10d8c198a95445554eeb0
>> > >   server/src/com/cloud/server/ManagementServerImpl.java
>> > 790441bdb91ff6c29a67dcd34960eb0caa4620a4
>> > >   server/src/com/cloud/storage/StorageManagerImpl.java
>> > 3d8b2c1fb54a932b7e806a9825b128cad656633c
>> > >   server/src/com/cloud/storage/StoragePoolAutomationImpl.java
>> > 8becd75ef26419fb7856758d5511f516901dcb5f
>> > >   server/src/com/cloud/storage/TemplateProfile.java
>> > 81e34f3c12148a1417c6d23d7d9cdd20a5777643
>> > >   server/src/com/cloud/storage/listener/StoragePoolMonitor.java
>> > 9f6b5fb9d3e07e197b630412f6d040c39be76881
>> > >   server/src/com/cloud/template/TemplateAdapter.java
>> > a85e3379834d4c2ab7c477e65b175799b7bb7e52
>> > >   server/src/com/cloud/template/TemplateAdapterBase.java
>> > e2204daea61998b69623c8ec8693fd4407f6fe34
>> > >   server/src/com/cloud/template/TemplateManagerImpl.java
>> > 51d09ef6cf6eda8b82ff89f759c6c9133923505e
>> > >   setup/db/db/schema-440to450.sql ee419a2
>> > >   ui/scripts/instances.js 93a40fc
>> > >   ui/scripts/storage.js 93fe79a
>> > >   ui/scripts/system.js 7e3b4573062b8620f8566620ee85d3ba61e2324b
>> > >   ui/scripts/templates.js e12927c538ad0608337af3ef3d2ec3cf1523ff40
>> > >
>> > > Diff: https://reviews.apache.org/r/22799/diff/
>> > >
>> > >
>> > > Testing
>> > > -------
>> > >
>> > > Environment: 2 nodes XenServer 6.2.0, CS code 4.5 branch, 2 Normal
>> > Primary Storage (NFS) + 1 Base Primary Storage (Local Storage - Node
>> > 2) + 1 Base Primary Storage (NFS), 1 Base Template (Ubuntu 12.04) and
>> > 1 normal template (Win 7)
>> > > Test case:
>> > > - Create new normal and base primary storage: Success.
>> > > - Register new normal and base template: Success.
>> > > - Deploy 1 normal VM and 1 base VM: Success.
>> > > - Start/Stop/Reboot/Ping VM: Success.
>> > > - Migrate normal stopped VM to another normal pool: Success.
>> > > - Live migrate normal running VM to another host: Success.
>> > > - Migrate stopped base VM to another pool:
>> > >     + List all normal pool that can communicate with base pool:
>> Success.
>> > >     + Migrate to selected pool: Success.
>> > > - Live migrate running VM to another host: Failed.
>> > >
>> > > (Checking result with command: vhd-util query -p -n <path to vhd
>> > > file>
>> > or vhd-util check -n <path to vhd file> to see the parent VHD disk
>> > location. )
>> > >
>> > >
>> > > Thanks,
>> > >
>> > > Hieu LE
>> > >
>> > >
>> >
>> >
>>
>>
>> --
>> *Mike Tutkowski*
>> *Senior CloudStack Developer, SolidFire Inc.*
>> e: mike.tutkowski@solidfire.com
>> o: 303.746.7302
>> Advancing the way the world uses the cloud
>> <http://solidfire.com/solution/overview/?video=play>*™*
>>
>
>
>
> --
> *Mike Tutkowski*
>  *Senior CloudStack Developer, SolidFire Inc.*
> e: mike.tutkowski@solidfire.com
> o: 303.746.7302
> Advancing the way the world uses the cloud
> <http://solidfire.com/solution/overview/?video=play>*™*
>



-- 
-----BEGIN GEEK CODE BLOCK-----
Version: 3.1
GCS/CM/IT/M/MU d-@? s+(++):+(++) !a C++++(++++)$ ULC++++(++)$ P L++(+++)$ E
!W N* o+ K w O- M V- PS+ PE++ Y+ PGP+ t 5 X R tv+ b+(++)>+++ DI- D+ G
e++(+++) h-- r(++)>+++ y-
------END GEEK CODE BLOCK------

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message