----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15173/#review28084 -----------------------------------------------------------
api/src/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java <https://reviews.apache.org/r/15173/#comment54664> Is the customized flag required? Since you have made cpu/memory as optional. api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java <https://reviews.apache.org/r/15173/#comment54666> Please keep the cpu related parameters together for better readability api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java <https://reviews.apache.org/r/15173/#comment54665> typo feild api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java <https://reviews.apache.org/r/15173/#comment54667> Please follow identical naming convention. rootdisksize -> rootDiskSize api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java <https://reviews.apache.org/r/15173/#comment54668> Where is the check to see if the service offering is dynamic or not? If a static service offering is specified and also the values of cpu/memory is provided it should fail with exception api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java <https://reviews.apache.org/r/15173/#comment54672> For root disk size should there be any validation based on template size? engine/api/src/org/apache/cloudstack/engine/service/api/OrchestrationService.java <https://reviews.apache.org/r/15173/#comment54669> Can we have a consistent naming for the parameters? engine/components-api/src/com/cloud/configuration/ConfigurationManager.java <https://reviews.apache.org/r/15173/#comment54670> Why is vmId manadatory here? engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java <https://reviews.apache.org/r/15173/#comment54671> Isn't root disk size optional for deploy vm using template? From this change looks like mandatory. engine/schema/src/com/cloud/service/ServiceOfferingVO.java <https://reviews.apache.org/r/15173/#comment54673> Why do you need this field? If any of cpu/memory is not specified then 'isDynamic' can be inferred. engine/schema/src/com/cloud/service/ServiceOfferingVO.java <https://reviews.apache.org/r/15173/#comment54675> Again this is not required engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java <https://reviews.apache.org/r/15173/#comment54674> ordering of parameter is not correct. optional parameters should be at the end - Koushik Das On Nov. 1, 2013, 5:40 a.m., bharat kumar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15173/ > ----------------------------------------------------------- > > (Updated Nov. 1, 2013, 5:40 a.m.) > > > Review request for cloudstack, Kishan Kavala and Koushik Das. > > > Bugs: CLOUDSTACK-4738 > https://issues.apache.org/jira/browse/CLOUDSTACK-4738 > > > Repository: cloudstack-git > > > Description > ------- > > https://issues.apache.org/jira/browse/CLOUDSTACK-4738 > Dynamic compute Offering. > > Still need to test this. Facing some auto wiring problems when > UsageEventUtils bean is created. > > > Diffs > ----- > > api/src/com/cloud/offering/ServiceOffering.java 9f7bf8e > api/src/com/cloud/vm/UserVmService.java 0b142e8 > api/src/org/apache/cloudstack/api/ApiConstants.java b1bfcfb > > api/src/org/apache/cloudstack/api/command/admin/offering/CreateServiceOfferingCmd.java > decac29 > api/src/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java 8a6cea7 > > engine/api/src/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java > a773ac4 > > engine/api/src/org/apache/cloudstack/engine/service/api/OrchestrationService.java > 64ef063 > engine/components-api/src/com/cloud/configuration/ConfigurationManager.java > 03a549f > engine/components-api/src/com/cloud/event/UsageEventUtils.java b44ed32 > engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java > b74b4c5 > > engine/orchestration/src/org/apache/cloudstack/engine/orchestration/CloudOrchestrator.java > 2fd10b6 > > engine/orchestration/src/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java > 0821c81 > engine/schema/src/com/cloud/event/UsageEventDetailsVO.java PRE-CREATION > engine/schema/src/com/cloud/event/dao/UsageEventDetailsDao.java > PRE-CREATION > engine/schema/src/com/cloud/event/dao/UsageEventDetailsDaoImpl.java > PRE-CREATION > engine/schema/src/com/cloud/service/ServiceOfferingVO.java 9a262c5 > engine/schema/src/com/cloud/service/dao/ServiceOfferingDao.java 7da7208 > engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java f807f0d > > plugins/network-elements/internal-loadbalancer/src/org/apache/cloudstack/network/lb/InternalLoadBalancerVMManagerImpl.java > b6269eb > > server/src/com/cloud/agent/manager/allocator/impl/UserConcentratedAllocator.java > 0da2c92 > server/src/com/cloud/api/query/QueryManagerImpl.java f9d9c4f > server/src/com/cloud/capacity/CapacityManagerImpl.java 72905a7 > server/src/com/cloud/configuration/ConfigurationManagerImpl.java e3aa4fa > server/src/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java e82aaba > server/src/com/cloud/network/NetworkModelImpl.java df9f651 > server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java > a93480b > server/src/com/cloud/vm/UserVmManagerImpl.java 80a4036 > server/test/com/cloud/capacity/CapacityManagerTest.java 3faa32f > server/test/com/cloud/vpc/MockConfigurationManagerImpl.java 3147f1f > server/test/resources/createNetworkOffering.xml 9d684ba > setup/db/db/schema-421to430.sql 0de9dfd > > Diff: https://reviews.apache.org/r/15173/diff/ > > > Testing > ------- > > Not tested. > > > Thanks, > > bharat kumar > >
