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.tutkow...@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 <hieul...@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.tutkow...@solidfire.com o: 303.746.7302 Advancing the way the world uses the cloud <http://solidfire.com/solution/overview/?video=play>*™*
