Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/#review29557 --- Ship it! master - 68406ba29d7dc31dcfd9ef2cefc673ff32cfa514 - Koushik Das On Nov. 29, 2013, 7:46 a.m., bharat kumar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 29, 2013, 7:46 a.m.) Review request for cloudstack and Koushik Das. Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs - api/src/org/apache/cloudstack/api/ApiConstants.java 7b87264 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java a7c864d api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 1357a7d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 6cb49c1 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 3dfcdf9 api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java 7ec739e engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70 engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java c02e0ad engine/schema/src/com/cloud/event/UsageEventVO.java 719a79b engine/schema/src/com/cloud/service/ServiceOfferingVO.java d968de5 engine/schema/src/com/cloud/service/dao/ServiceOfferingDao.java 10d6616 engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java f1f97fc engine/schema/src/com/cloud/storage/DiskOfferingVO.java 8a9dd3d engine/schema/src/com/cloud/usage/UsageVMInstanceVO.java cd746c2 server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 4f351eb server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 2260e1e server/src/com/cloud/capacity/CapacityManagerImpl.java c733c03 server/src/com/cloud/configuration/ConfigurationManagerImpl.java 3770eb8 server/src/com/cloud/server/ManagementServerImpl.java d34da4f server/src/com/cloud/vm/UserVmManager.java 8463326 server/src/com/cloud/vm/UserVmManagerImpl.java c0b0031 server/test/com/cloud/vm/UserVmManagerTest.java 71bbebe usage/src/com/cloud/usage/UsageManagerImpl.java 81e7892 Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 29, 2013, 12:35 p.m.) Review request for cloudstack and Koushik Das. Changes --- patch for 4.3 branch Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs (updated) - api/src/org/apache/cloudstack/api/ApiConstants.java 6f919c1 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java 212f129 api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 738b15d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 44f5575 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 161131b api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java e305ee9 engine/api/src/com/cloud/vm/VirtualMachineManager.java 00393bf engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java b92b41f engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java e784295 engine/schema/src/com/cloud/event/UsageEventVO.java 6fad8c9 engine/schema/src/com/cloud/service/ServiceOfferingVO.java 66ab836 engine/schema/src/com/cloud/service/dao/ServiceOfferingDao.java c5c4cff engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java 917eaef engine/schema/src/com/cloud/storage/DiskOfferingVO.java b5b3451 engine/schema/src/com/cloud/usage/UsageVMInstanceVO.java 1178cc8 server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 3bc6c78 server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 3c843bb server/src/com/cloud/capacity/CapacityManagerImpl.java 854e27a server/src/com/cloud/configuration/ConfigurationManagerImpl.java 21651ad server/src/com/cloud/server/ManagementServerImpl.java d083c11 server/src/com/cloud/vm/UserVmManager.java 485e633 server/src/com/cloud/vm/UserVmManagerImpl.java 485a345 server/test/com/cloud/vm/UserVmManagerTest.java 0a3ed3c usage/src/com/cloud/usage/UsageManagerImpl.java ea04dd0 Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/#review29558 --- Ship it! 4.3 - 72e03546619f50fa24abc372eb35438ef02713f8 - Koushik Das On Nov. 29, 2013, 12:35 p.m., bharat kumar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 29, 2013, 12:35 p.m.) Review request for cloudstack and Koushik Das. Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs - api/src/org/apache/cloudstack/api/ApiConstants.java 6f919c1 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java 212f129 api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 738b15d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 44f5575 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 161131b api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java e305ee9 engine/api/src/com/cloud/vm/VirtualMachineManager.java 00393bf engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java b92b41f engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java e784295 engine/schema/src/com/cloud/event/UsageEventVO.java 6fad8c9 engine/schema/src/com/cloud/service/ServiceOfferingVO.java 66ab836 engine/schema/src/com/cloud/service/dao/ServiceOfferingDao.java c5c4cff engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java 917eaef engine/schema/src/com/cloud/storage/DiskOfferingVO.java b5b3451 engine/schema/src/com/cloud/usage/UsageVMInstanceVO.java 1178cc8 server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 3bc6c78 server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 3c843bb server/src/com/cloud/capacity/CapacityManagerImpl.java 854e27a server/src/com/cloud/configuration/ConfigurationManagerImpl.java 21651ad server/src/com/cloud/server/ManagementServerImpl.java d083c11 server/src/com/cloud/vm/UserVmManager.java 485e633 server/src/com/cloud/vm/UserVmManagerImpl.java 485a345 server/test/com/cloud/vm/UserVmManagerTest.java 0a3ed3c usage/src/com/cloud/usage/UsageManagerImpl.java ea04dd0 Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 28, 2013, 10:29 a.m.) Review request for cloudstack and Koushik Das. Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs (updated) - api/src/org/apache/cloudstack/api/ApiConstants.java 7b87264 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java a7c864d api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 1357a7d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 6cb49c1 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 3dfcdf9 api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java 7ec739e engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70 engine/schema/src/com/cloud/event/UsageEventVO.java 719a79b engine/schema/src/com/cloud/service/ServiceOfferingVO.java d968de5 engine/schema/src/com/cloud/service/dao/ServiceOfferingDao.java 10d6616 engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java f1f97fc engine/schema/src/com/cloud/usage/UsageVMInstanceVO.java cd746c2 server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 4f351eb server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 2260e1e server/src/com/cloud/capacity/CapacityManagerImpl.java c733c03 server/src/com/cloud/configuration/ConfigurationManagerImpl.java 3770eb8 server/src/com/cloud/server/ManagementServerImpl.java d34da4f server/src/com/cloud/vm/UserVmManager.java 8463326 server/src/com/cloud/vm/UserVmManagerImpl.java c0b0031 server/test/com/cloud/vm/UserVmManagerTest.java 71bbebe usage/src/com/cloud/usage/UsageManagerImpl.java 81e7892 Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 29, 2013, 5:09 a.m.) Review request for cloudstack and Koushik Das. Changes --- patch for master Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs (updated) - api/src/org/apache/cloudstack/api/ApiConstants.java 7b87264 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java a7c864d api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 1357a7d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 6cb49c1 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 3dfcdf9 api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java 7ec739e engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70 engine/schema/src/com/cloud/event/UsageEventVO.java 719a79b engine/schema/src/com/cloud/service/ServiceOfferingVO.java d968de5 engine/schema/src/com/cloud/service/dao/ServiceOfferingDao.java 10d6616 engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java f1f97fc engine/schema/src/com/cloud/usage/UsageVMInstanceVO.java cd746c2 server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 4f351eb server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 2260e1e server/src/com/cloud/capacity/CapacityManagerImpl.java c733c03 server/src/com/cloud/configuration/ConfigurationManagerImpl.java 3770eb8 server/src/com/cloud/server/ManagementServerImpl.java d34da4f server/src/com/cloud/vm/UserVmManager.java 8463326 server/src/com/cloud/vm/UserVmManagerImpl.java c0b0031 server/test/com/cloud/vm/UserVmManagerTest.java 71bbebe usage/src/com/cloud/usage/UsageManagerImpl.java 81e7892 Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 29, 2013, 7:46 a.m.) Review request for cloudstack and Koushik Das. Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs (updated) - api/src/org/apache/cloudstack/api/ApiConstants.java 7b87264 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java a7c864d api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 1357a7d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 6cb49c1 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 3dfcdf9 api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java 7ec739e engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70 engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java c02e0ad engine/schema/src/com/cloud/event/UsageEventVO.java 719a79b engine/schema/src/com/cloud/service/ServiceOfferingVO.java d968de5 engine/schema/src/com/cloud/service/dao/ServiceOfferingDao.java 10d6616 engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java f1f97fc engine/schema/src/com/cloud/storage/DiskOfferingVO.java 8a9dd3d engine/schema/src/com/cloud/usage/UsageVMInstanceVO.java cd746c2 server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 4f351eb server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 2260e1e server/src/com/cloud/capacity/CapacityManagerImpl.java c733c03 server/src/com/cloud/configuration/ConfigurationManagerImpl.java 3770eb8 server/src/com/cloud/server/ManagementServerImpl.java d34da4f server/src/com/cloud/vm/UserVmManager.java 8463326 server/src/com/cloud/vm/UserVmManagerImpl.java c0b0031 server/test/com/cloud/vm/UserVmManagerTest.java 71bbebe usage/src/com/cloud/usage/UsageManagerImpl.java 81e7892 Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 27, 2013, 9:33 a.m.) Review request for cloudstack and Koushik Das. Changes --- Incorporated review comments. Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs (updated) - api/src/org/apache/cloudstack/api/ApiConstants.java 6f919c1 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java a7c864d api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 1357a7d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 6cb49c1 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 3dfcdf9 api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java 7ec739e engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70 engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java f1f97fc engine/schema/src/com/cloud/usage/UsageVMInstanceVO.java 06a7bee framework/db/src/com/cloud/utils/db/GenericDaoBase.java 8e6f6a4 server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 4f351eb server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 2260e1e server/src/com/cloud/configuration/ConfigurationManagerImpl.java 3770eb8 server/src/com/cloud/server/ManagementServerImpl.java d34da4f server/src/com/cloud/vm/UserVmManager.java b7b4bd5 server/src/com/cloud/vm/UserVmManagerImpl.java 00d8063 server/test/com/cloud/vm/UserVmManagerTest.java 71bbebe Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
On Nov. 26, 2013, 6:27 p.m., Nitin Mehta wrote: api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java, line 63 https://reviews.apache.org/r/15832/diff/3/?file=391144#file391144line63 All this is boilerplate code. Have an abstract class and make all these classes extend this abstract class. There are some examples to get an idea for doing this. Hi Nitin, The scalevm, scalesystemvm, upgradesystemvm and upgradevm all have similar API parameters. we need to move all these into a base class. I have created a bug for this. https://issues.apache.org/jira/browse/CLOUDSTACK-5286. On Nov. 26, 2013, 6:27 p.m., Nitin Mehta wrote: server/src/com/cloud/vm/UserVmManagerImpl.java, line 812 https://reviews.apache.org/r/15832/diff/3/?file=391157#file391157line812 I would suggest writing a util function for adding and removing these details. I think it can be used while deploying vm as well. Added functions saveCusotmOfferingDetails and removeCustomOfferingDetails. On Nov. 26, 2013, 6:27 p.m., Nitin Mehta wrote: server/src/com/cloud/vm/UserVmManagerImpl.java, line 866 https://reviews.apache.org/r/15832/diff/3/?file=391157#file391157line866 can you please break it down ? or write comments please Added a comment. On Nov. 26, 2013, 6:27 p.m., Nitin Mehta wrote: server/src/com/cloud/configuration/ConfigurationManagerImpl.java, line 1938 https://reviews.apache.org/r/15832/diff/3/?file=391154#file391154line1938 check for !(cpuNumber == null cpuSpeed == null memory == null) Hi Nitin, the intention is to check if any one of the params (cpu men etc) is null then all should be null. - bharat --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/#review29445 --- On Nov. 27, 2013, 9:33 a.m., bharat kumar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 27, 2013, 9:33 a.m.) Review request for cloudstack and Koushik Das. Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs - api/src/org/apache/cloudstack/api/ApiConstants.java 6f919c1 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java a7c864d api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 1357a7d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 6cb49c1 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 3dfcdf9 api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java 7ec739e engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70 engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java f1f97fc engine/schema/src/com/cloud/usage/UsageVMInstanceVO.java 06a7bee framework/db/src/com/cloud/utils/db/GenericDaoBase.java 8e6f6a4 server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 4f351eb server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 2260e1e server/src/com/cloud/configuration/ConfigurationManagerImpl.java 3770eb8 server/src/com/cloud/server/ManagementServerImpl.java d34da4f server/src/com/cloud/vm/UserVmManager.java b7b4bd5 server/src/com/cloud/vm/UserVmManagerImpl.java 00d8063 server/test/com/cloud/vm/UserVmManagerTest.java 71bbebe Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/#review29479 --- engine/schema/src/com/cloud/usage/UsageVMInstanceVO.java https://reviews.apache.org/r/15832/#comment56777 This is already there. check. Avoid duplicating the enum between cloud and usage db. framework/db/src/com/cloud/utils/db/GenericDaoBase.java https://reviews.apache.org/r/15832/#comment56769 '\' got added in the name, will result in build errors. server/src/com/cloud/configuration/ConfigurationManagerImpl.java https://reviews.apache.org/r/15832/#comment56770 Update the FS with this information. Fix typos in comments and exception messages. server/src/com/cloud/vm/UserVmManager.java https://reviews.apache.org/r/15832/#comment56785 Why is this needed in the interface? server/src/com/cloud/vm/UserVmManagerImpl.java https://reviews.apache.org/r/15832/#comment56783 Below I see some save/remove custom details methods? Shouldn't you just call those methods here? server/src/com/cloud/vm/UserVmManagerImpl.java https://reviews.apache.org/r/15832/#comment56782 Why is there a dependency to UsageVMInstanceVO? Isn't that a table in usage server? server/src/com/cloud/vm/UserVmManagerImpl.java https://reviews.apache.org/r/15832/#comment56778 There are 2 things happening here: validating custom parameters and updating service offering. Separate them into separate functions. If validation is successful then update service offering. server/src/com/cloud/vm/UserVmManagerImpl.java https://reviews.apache.org/r/15832/#comment56779 remove this check as this is always called from internal code and custom parameters will be non null. server/src/com/cloud/vm/UserVmManagerImpl.java https://reviews.apache.org/r/15832/#comment56780 This is also not required as custom parameter map cannot be null server/src/com/cloud/vm/UserVmManagerImpl.java https://reviews.apache.org/r/15832/#comment56781 Put some comment to explain the logic server/src/com/cloud/vm/UserVmManagerImpl.java https://reviews.apache.org/r/15832/#comment56784 I see the usage event publishing code getting repeated. Move them to a separate method. - Koushik Das On Nov. 27, 2013, 9:33 a.m., bharat kumar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 27, 2013, 9:33 a.m.) Review request for cloudstack and Koushik Das. Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs - api/src/org/apache/cloudstack/api/ApiConstants.java 6f919c1 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java a7c864d api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 1357a7d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 6cb49c1 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 3dfcdf9 api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java 7ec739e engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70 engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java f1f97fc engine/schema/src/com/cloud/usage/UsageVMInstanceVO.java 06a7bee framework/db/src/com/cloud/utils/db/GenericDaoBase.java 8e6f6a4 server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 4f351eb server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 2260e1e server/src/com/cloud/configuration/ConfigurationManagerImpl.java 3770eb8 server/src/com/cloud/server/ManagementServerImpl.java d34da4f server/src/com/cloud/vm/UserVmManager.java b7b4bd5 server/src/com/cloud/vm/UserVmManagerImpl.java 00d8063 server/test/com/cloud/vm/UserVmManagerTest.java 71bbebe Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 28, 2013, 2:04 a.m.) Review request for cloudstack and Koushik Das. Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs (updated) - api/src/org/apache/cloudstack/api/ApiConstants.java 7b87264 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java a7c864d api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 1357a7d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 6cb49c1 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 3dfcdf9 api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java 7ec739e engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70 engine/schema/src/com/cloud/event/UsageEventVO.java 719a79b engine/schema/src/com/cloud/service/ServiceOfferingVO.java d968de5 engine/schema/src/com/cloud/service/dao/ServiceOfferingDao.java 10d6616 engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java f1f97fc server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 4f351eb server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 2260e1e server/src/com/cloud/capacity/CapacityManagerImpl.java c733c03 server/src/com/cloud/configuration/ConfigurationManagerImpl.java 3770eb8 server/src/com/cloud/server/ManagementServerImpl.java d34da4f server/src/com/cloud/vm/UserVmManager.java 8463326 server/src/com/cloud/vm/UserVmManagerImpl.java c0b0031 server/test/com/cloud/vm/UserVmManagerTest.java 71bbebe Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 28, 2013, 2:26 a.m.) Review request for cloudstack and Koushik Das. Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs (updated) - api/src/org/apache/cloudstack/api/ApiConstants.java 7b87264 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java a7c864d api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 1357a7d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 6cb49c1 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 3dfcdf9 api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java 7ec739e engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70 engine/schema/src/com/cloud/event/UsageEventVO.java 719a79b engine/schema/src/com/cloud/service/ServiceOfferingVO.java d968de5 engine/schema/src/com/cloud/service/dao/ServiceOfferingDao.java 10d6616 engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java f1f97fc server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 4f351eb server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 2260e1e server/src/com/cloud/capacity/CapacityManagerImpl.java c733c03 server/src/com/cloud/configuration/ConfigurationManagerImpl.java 3770eb8 server/src/com/cloud/server/ManagementServerImpl.java d34da4f server/src/com/cloud/vm/UserVmManager.java 8463326 server/src/com/cloud/vm/UserVmManagerImpl.java c0b0031 server/test/com/cloud/vm/UserVmManagerTest.java 71bbebe Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 28, 2013, 6:18 a.m.) Review request for cloudstack and Koushik Das. Changes --- Rebased with master. Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs (updated) - api/src/org/apache/cloudstack/api/ApiConstants.java 7b87264 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java a7c864d api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 1357a7d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 6cb49c1 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 3dfcdf9 api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java 7ec739e engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70 engine/schema/src/com/cloud/event/UsageEventVO.java 719a79b engine/schema/src/com/cloud/service/ServiceOfferingVO.java d968de5 engine/schema/src/com/cloud/service/dao/ServiceOfferingDao.java 10d6616 engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java f1f97fc engine/schema/src/com/cloud/usage/UsageVMInstanceVO.java cd746c2 server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 4f351eb server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 2260e1e server/src/com/cloud/capacity/CapacityManagerImpl.java c733c03 server/src/com/cloud/configuration/ConfigurationManagerImpl.java 3770eb8 server/src/com/cloud/server/ManagementServerImpl.java d34da4f server/src/com/cloud/vm/UserVmManager.java 8463326 server/src/com/cloud/vm/UserVmManagerImpl.java c0b0031 server/test/com/cloud/vm/UserVmManagerTest.java 71bbebe usage/src/com/cloud/usage/UsageManagerImpl.java 81e7892 Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 26, 2013, 11:37 a.m.) Review request for cloudstack and Koushik Das. Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs (updated) - engine/schema/src/com/cloud/service/ServiceOfferingVO.java 67fea00 server/src/com/cloud/vm/UserVmManagerImpl.java 00d8063 Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 26, 2013, 11:38 a.m.) Review request for cloudstack and Koushik Das. Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs (updated) - api/src/org/apache/cloudstack/api/ApiConstants.java 6f919c1 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java a7c864d api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 1357a7d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 6cb49c1 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 3dfcdf9 api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java 7ec739e engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70 engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java f1f97fc engine/schema/src/com/cloud/usage/UsageVMInstanceVO.java 06a7bee server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 4f351eb server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 2260e1e server/src/com/cloud/configuration/ConfigurationManagerImpl.java 3770eb8 server/src/com/cloud/server/ManagementServerImpl.java d34da4f server/src/com/cloud/vm/UserVmManager.java b7b4bd5 server/src/com/cloud/vm/UserVmManagerImpl.java 00d8063 server/test/com/cloud/vm/UserVmManagerTest.java 71bbebe Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar
Re: Review Request 15832: enable custom offering support for scalevm
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/#review29445 --- api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java https://reviews.apache.org/r/15832/#comment56657 All this is boilerplate code. Have an abstract class and make all these classes extend this abstract class. There are some examples to get an idea for doing this. api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java https://reviews.apache.org/r/15832/#comment56658 it should be name. Also better to give example how to specify them server/src/com/cloud/configuration/ConfigurationManagerImpl.java https://reviews.apache.org/r/15832/#comment56659 check for !(cpuNumber == null cpuSpeed == null memory == null) server/src/com/cloud/vm/UserVmManagerImpl.java https://reviews.apache.org/r/15832/#comment56661 I would suggest writing a util function for adding and removing these details. I think it can be used while deploying vm as well. server/src/com/cloud/vm/UserVmManagerImpl.java https://reviews.apache.org/r/15832/#comment56663 can you please break it down ? or write comments please server/src/com/cloud/vm/UserVmManagerImpl.java https://reviews.apache.org/r/15832/#comment56662 boiler plate code. - Nitin Mehta On Nov. 26, 2013, 11:38 a.m., bharat kumar wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15832/ --- (Updated Nov. 26, 2013, 11:38 a.m.) Review request for cloudstack and Koushik Das. Bugs: CLOUDSTACK-5161 https://issues.apache.org/jira/browse/CLOUDSTACK-5161 Repository: cloudstack-git Description --- enable scaling of a vm using custom offering CLOUDSTACK-5161 Diffs - api/src/org/apache/cloudstack/api/ApiConstants.java 6f919c1 api/src/org/apache/cloudstack/api/command/admin/systemvm/ScaleSystemVMCmd.java a7c864d api/src/org/apache/cloudstack/api/command/admin/systemvm/UpgradeSystemVMCmd.java 1357a7d api/src/org/apache/cloudstack/api/command/user/vm/ScaleVMCmd.java 6cb49c1 api/src/org/apache/cloudstack/api/command/user/vm/UpgradeVMCmd.java 3dfcdf9 api/src/org/apache/cloudstack/api/response/ServiceOfferingResponse.java 7ec739e engine/api/src/com/cloud/vm/VirtualMachineManager.java c78942f engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java 3a3de70 engine/schema/src/com/cloud/service/dao/ServiceOfferingDaoImpl.java f1f97fc engine/schema/src/com/cloud/usage/UsageVMInstanceVO.java 06a7bee server/src/com/cloud/api/query/dao/ServiceOfferingJoinDaoImpl.java 4f351eb server/src/com/cloud/api/query/vo/ServiceOfferingJoinVO.java 2260e1e server/src/com/cloud/configuration/ConfigurationManagerImpl.java 3770eb8 server/src/com/cloud/server/ManagementServerImpl.java d34da4f server/src/com/cloud/vm/UserVmManager.java b7b4bd5 server/src/com/cloud/vm/UserVmManagerImpl.java 00d8063 server/test/com/cloud/vm/UserVmManagerTest.java 71bbebe Diff: https://reviews.apache.org/r/15832/diff/ Testing --- Tested on master. Thanks, bharat kumar