> On Feb. 8, 2013, 1:03 a.m., Prachi Damle wrote: > > Few comments: > > 1) In Allocators, it would be good to put the logic to check if host is > > suitable in separate methods that adding logic in-place. This will help > > readability. e.g. methods like checkIfHostIsDedicated() / > > checkIfHostIsFree() etc. > > 2) In Planner, check if host can be used prior to even looking for suitable > > volumes. This will save unnecessary execution of storage allocation when > > host is not suitable. > > 3) Should db changes be in schema 40to402 instead of 401? > > 4) Need to add unit-tests. Tests can mock the DAO responses and just run > > the allocator changes to check if they output the correct list of hosts. > > deepti dohare wrote: > Thanks Prachi, for the review. The requirements got updated recently. > Will soon submit a new patch.
In that case, please can you close this patch. - Prachi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9182/#review16312 ----------------------------------------------------------- On Feb. 7, 2013, 3:05 p.m., deepti dohare wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9182/ > ----------------------------------------------------------- > > (Updated Feb. 7, 2013, 3:05 p.m.) > > > Review request for cloudstack, Prachi Damle and Nitin Mehta. > > > Description > ------- > > Review Request for Implicit Dedication > ====================================== > > This feature is a part of Dedicated Resources: private pod, cluster or host. > > This feature allows to add a service offering that will indicate that the VM > needs to be on a host exclusive to an account. > > 1. A new check parameter called 'isDedicated' is added to the service > offerings. > 2. A dedicated Instance can be deployed with the Service Offering having > 'isDedicated' flag true. > 3. Dedicated instance will be placed in the host having dedicated VMs of the > same account. > 4. If space is not available in the dedicated host, dedicated VM will be > place in the new empty host. > 5. A non-dedicated instance will be placed in any host but not in the > dedicated host. > 6. If dedicated host gets empty, it will become available for all the > accounts. > > > This addresses bug CLOUDSTACK-718. > > > Diffs > ----- > > api/src/com/cloud/offering/ServiceOffering.java 4d71589 > api/src/org/apache/cloudstack/api/ApiConstants.java d29408e > > api/src/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java > ee1e1b2 > api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java > f35e87e > > plugins/host-allocators/random/src/com/cloud/agent/manager/allocator/impl/RandomAllocator.java > a672efd > > plugins/network-elements/elastic-loadbalancer/src/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java > 81039d1 > server/src/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java > a25e401 > server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 9795fef > server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java fe4a165 > server/src/com/cloud/configuration/ConfigurationManager.java 5c1b0d5 > server/src/com/cloud/configuration/ConfigurationManagerImpl.java 750b8b8 > server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java 168ac0e > server/src/com/cloud/deploy/FirstFitPlanner.java 66a24ac > server/src/com/cloud/migration/ServiceOffering21VO.java fdec30e > server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java > 1abca51 > server/src/com/cloud/server/ConfigurationServerImpl.java 5fa4c60 > server/src/com/cloud/service/ServiceOfferingVO.java c199a86 > server/src/com/cloud/service/dao/ServiceOfferingDao.java 589de7c > server/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java 062103e > server/src/com/cloud/storage/secondary/SecondaryStorageManagerImpl.java > fca89dc > server/src/com/cloud/test/DatabaseConfig.java 7c10f98 > server/src/com/cloud/vm/UserVmManagerImpl.java 662dab3 > server/src/com/cloud/vm/dao/VMInstanceDao.java d34b257 > server/src/com/cloud/vm/dao/VMInstanceDaoImpl.java 531c794 > server/test/com/cloud/vpc/MockConfigurationManagerImpl.java 574ce0a > setup/db/create-schema-view.sql f68a6ca > setup/db/create-schema.sql f89c885 > setup/db/db/schema-40to410.sql 6d5b262 > > Diff: https://reviews.apache.org/r/9182/diff/ > > > Testing > ------- > > Manual Testing Done > > Tests for DeployVirtualMachine > ============================== > Setup : Host1 and Host2 , Host 1 has system vms , Host 2 is empty. > Service Offering: SWD (Service offering with Dedication enabled), SND > (Service offering without Dedication enabled) > > 1. Root admin deploys a non-dedicated VM with Service Offering(SND), it went > to Host 2. > 2. Account a1 deploys a dedicated VM with service offering(SWD). VM deployed > in Host 1. > Note: Host having only systemVms is considered as empty host > 3. Now any non-dedicated VM (SND) placed in Host 2 > - vm created from Root account went to Host 2 - Tested > - vm created from a1 account went to Host 2 - Tested > - vm created from u1 account went to Host 2 - Tested > - vm created from different domains account : Host2 - Tested > 4. Deploying dedicated VMs with(SWD) of different Account failed, Since no > free host available. > 5. Any dedicated VM (SWD) of same Account a1 is placed on Host 1. > 6. If dedicated VMs in Host1 are deleted, the host becomes available for all > accounts i.e Account u1's VM used this Host1 . > > > Tests for Migration of dedicated VMs > ==================================== > 1. Migrate a non-dedicated VM: > - if destination host has dedicated vms, request failed. > - if destination host has no dedicated vms, migration to the destination > host successful. > 2. Migrate a dedicated VM: > - if destination host has dedicated vms for the same account, migration > to the destination host successful. > - if destination host has no dedicated vms, request failed. > > > Test for HostTags and isdedicated flag > ====================================== > 1. Host Tag is provided and isDedicated flag is true > - if Host has dedicated vms, vm is placed in that host > - if Host has no dedicated vms, request failed. > 2. Host tag is provided and isDedicated flag is false > - if Host has dedicated vms, request failed. > - if Host has no dedicated vms, vm is placed in that host (as cloudstack > is doing now) > > > Thanks, > > deepti dohare > >
