cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Koushik Das" <koushik....@citrix.com>
Subject Re: Review Request 15173: Dynamic compute offering.
Date Mon, 04 Nov 2013 08:32:55 GMT

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


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