Re: Review Request: multiple ip address per vm nic changes for isolated and vpc networks changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/ --- (Updated March 1, 2013, 10:08 a.m.) Review request for cloudstack. Changes --- incorporated review comments Description --- Using this feature we can reserve secondary ip addresses for guest vm nic. Administrator will configure the these ip addresses on the nic manually. This addresses bug CLOUDSTACK-24. Diffs (updated) - api/src/com/cloud/network/IpAddress.java 47df4d6 api/src/com/cloud/network/NetworkService.java 95bcc42 api/src/com/cloud/network/rules/RulesService.java 921a86e api/src/com/cloud/vm/Nic.java 9d21130 api/src/com/cloud/vm/NicSecondaryIp.java PRE-CREATION api/src/org/apache/cloudstack/api/ApiConstants.java 4fdc91e api/src/org/apache/cloudstack/api/ResponseGenerator.java 267238a api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java 39ab812 api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java ce6ea16 api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListNicsCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/response/AddIpToVmNicResponse.java PRE-CREATION api/src/org/apache/cloudstack/api/response/IPAddressResponse.java 251b2dd api/src/org/apache/cloudstack/api/response/NicResponse.java a7d1a0d api/src/org/apache/cloudstack/api/response/NicSecondaryIpResponse.java PRE-CREATION api/test/org/apache/cloudstack/api/command/test/AddIpToVmNicTest.java PRE-CREATION client/tomcatconf/commands.properties.in f03e8d5 server/src/com/cloud/api/ApiDBUtils.java ffee22f server/src/com/cloud/api/ApiResponseHelper.java eafee8a server/src/com/cloud/network/NetworkManager.java 2904183 server/src/com/cloud/network/NetworkManagerImpl.java f527b73 server/src/com/cloud/network/NetworkModelImpl.java ac1bc87 server/src/com/cloud/network/NetworkServiceImpl.java 1708224 server/src/com/cloud/network/addr/PublicIp.java 61351a6 server/src/com/cloud/network/dao/IPAddressDao.java 9cdb975 server/src/com/cloud/network/dao/IPAddressDaoImpl.java e7067d9 server/src/com/cloud/network/dao/IPAddressVO.java 00da5eb server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java 531a428 server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 0e2eb63 server/src/com/cloud/network/rules/RulesManagerImpl.java 614d308 server/src/com/cloud/network/rules/dao/PortForwardingRulesDao.java 91f08e7 server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java 5406ab6 server/src/com/cloud/server/ManagementServerImpl.java e80d48c server/src/com/cloud/vm/NicVO.java 8e2edda server/src/com/cloud/vm/UserVmManagerImpl.java ce53c45 server/src/com/cloud/vm/dao/NicDao.java 762048b server/src/com/cloud/vm/dao/NicDaoImpl.java 5cf152f server/src/com/cloud/vm/dao/NicSecondaryIpDao.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpDaoImpl.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpVO.java PRE-CREATION server/test/com/cloud/network/MockNetworkManagerImpl.java 3568da5 server/test/com/cloud/network/MockRulesManagerImpl.java ba3dd41 server/test/com/cloud/vpc/MockNetworkManagerImpl.java 828a555 setup/db/db/schema-410to420.sql 4349bd0 Diff: https://reviews.apache.org/r/9396/diff/ Testing --- Tested add adding, deleting and listing addresses on the nic using APIs Thanks, Jayapal Reddy
Re: Review Request: multiple ip address per vm nic changes for isolated and vpc networks changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/ --- (Updated March 1, 2013, 11:04 a.m.) Review request for cloudstack. Changes --- removed trailing white space chars Description --- Using this feature we can reserve secondary ip addresses for guest vm nic. Administrator will configure the these ip addresses on the nic manually. This addresses bug CLOUDSTACK-24. Diffs (updated) - api/src/com/cloud/network/IpAddress.java 47df4d6 api/src/com/cloud/network/NetworkService.java 95bcc42 api/src/com/cloud/network/rules/RulesService.java 921a86e api/src/com/cloud/vm/Nic.java 9d21130 api/src/com/cloud/vm/NicSecondaryIp.java PRE-CREATION api/src/org/apache/cloudstack/api/ApiConstants.java 4fdc91e api/src/org/apache/cloudstack/api/ResponseGenerator.java 267238a api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java 39ab812 api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java ce6ea16 api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListNicsCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/response/AddIpToVmNicResponse.java PRE-CREATION api/src/org/apache/cloudstack/api/response/IPAddressResponse.java 251b2dd api/src/org/apache/cloudstack/api/response/NicResponse.java a7d1a0d api/src/org/apache/cloudstack/api/response/NicSecondaryIpResponse.java PRE-CREATION api/test/org/apache/cloudstack/api/command/test/AddIpToVmNicTest.java PRE-CREATION client/tomcatconf/commands.properties.in f03e8d5 server/src/com/cloud/api/ApiDBUtils.java ffee22f server/src/com/cloud/api/ApiResponseHelper.java eafee8a server/src/com/cloud/network/NetworkManager.java 2904183 server/src/com/cloud/network/NetworkManagerImpl.java f527b73 server/src/com/cloud/network/NetworkModelImpl.java ac1bc87 server/src/com/cloud/network/NetworkServiceImpl.java 1708224 server/src/com/cloud/network/addr/PublicIp.java 61351a6 server/src/com/cloud/network/dao/IPAddressDao.java 9cdb975 server/src/com/cloud/network/dao/IPAddressDaoImpl.java e7067d9 server/src/com/cloud/network/dao/IPAddressVO.java 00da5eb server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java 531a428 server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 0e2eb63 server/src/com/cloud/network/rules/RulesManagerImpl.java 614d308 server/src/com/cloud/network/rules/dao/PortForwardingRulesDao.java 91f08e7 server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java 5406ab6 server/src/com/cloud/server/ManagementServerImpl.java e80d48c server/src/com/cloud/vm/NicVO.java 8e2edda server/src/com/cloud/vm/UserVmManagerImpl.java ce53c45 server/src/com/cloud/vm/dao/NicDao.java 762048b server/src/com/cloud/vm/dao/NicDaoImpl.java 5cf152f server/src/com/cloud/vm/dao/NicSecondaryIpDao.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpDaoImpl.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpVO.java PRE-CREATION server/test/com/cloud/network/MockNetworkManagerImpl.java 3568da5 server/test/com/cloud/network/MockRulesManagerImpl.java ba3dd41 server/test/com/cloud/vpc/MockNetworkManagerImpl.java 828a555 setup/db/db/schema-410to420.sql 4349bd0 Diff: https://reviews.apache.org/r/9396/diff/ Testing --- Tested add adding, deleting and listing addresses on the nic using APIs Thanks, Jayapal Reddy
Re: Review Request: multiple ip address per vm nic changes for isolated and vpc networks changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/#review17228 --- Remove white spaces and spellcheck and then it is ready for shipping. - Abhinandan Prateek On Feb. 28, 2013, 5:24 a.m., Jayapal Reddy wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/ --- (Updated Feb. 28, 2013, 5:24 a.m.) Review request for cloudstack. Description --- Using this feature we can reserve secondary ip addresses for guest vm nic. Administrator will configure the these ip addresses on the nic manually. This addresses bug CLOUDSTACK-24. Diffs - api/src/com/cloud/network/IpAddress.java 47df4d6 api/src/com/cloud/network/NetworkService.java 95bcc42 api/src/com/cloud/network/rules/RulesService.java 921a86e api/src/com/cloud/vm/Nic.java 9d21130 api/src/com/cloud/vm/NicSecondaryIp.java PRE-CREATION api/src/org/apache/cloudstack/api/ApiConstants.java a26b468 api/src/org/apache/cloudstack/api/ResponseGenerator.java 267238a api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java 39ab812 api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java e0f9bcd api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java ce6ea16 api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListNicsCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/response/IPAddressResponse.java 251b2dd api/src/org/apache/cloudstack/api/response/NicResponse.java a7d1a0d api/src/org/apache/cloudstack/api/response/NicSecondaryIpResponse.java PRE-CREATION api/test/org/apache/cloudstack/api/command/test/AddIpToVmNicTest.java PRE-CREATION client/tomcatconf/commands.properties.in f03e8d5 server/src/com/cloud/api/ApiDBUtils.java ffee22f server/src/com/cloud/api/ApiResponseHelper.java eafee8a server/src/com/cloud/network/NetworkManager.java 2904183 server/src/com/cloud/network/NetworkManagerImpl.java 82893c4 server/src/com/cloud/network/NetworkModelImpl.java f978ac5 server/src/com/cloud/network/NetworkServiceImpl.java ce527b7 server/src/com/cloud/network/addr/PublicIp.java 7336c9c server/src/com/cloud/network/dao/IPAddressDao.java 9cdb975 server/src/com/cloud/network/dao/IPAddressDaoImpl.java e7067d9 server/src/com/cloud/network/dao/IPAddressVO.java 00da5eb server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java abb4973 server/src/com/cloud/network/rules/RulesManagerImpl.java 614d308 server/src/com/cloud/network/rules/dao/PortForwardingRulesDao.java 91f08e7 server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java 5406ab6 server/src/com/cloud/server/ManagementServerImpl.java e80d48c server/src/com/cloud/vm/NicVO.java 8e2edda server/src/com/cloud/vm/UserVmManagerImpl.java ce53c45 server/src/com/cloud/vm/dao/NicDao.java 762048b server/src/com/cloud/vm/dao/NicDaoImpl.java 5cf152f server/src/com/cloud/vm/dao/NicSecondaryIpDao.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpDaoImpl.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpVO.java PRE-CREATION server/test/com/cloud/network/MockNetworkManagerImpl.java 3568da5 server/test/com/cloud/network/MockRulesManagerImpl.java ba3dd41 server/test/com/cloud/vpc/MockNetworkManagerImpl.java 828a555 setup/db/db/schema-410to420.sql 4349bd0 Diff: https://reviews.apache.org/r/9396/diff/ Testing --- Tested add adding, deleting and listing addresses on the nic using APIs Thanks, Jayapal Reddy
Re: Review Request: multiple ip address per vm nic changes for isolated and vpc networks changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/ --- (Updated Feb. 28, 2013, 5:24 a.m.) Review request for cloudstack. Changes --- Removed white space chars and commented code. Description --- Using this feature we can reserve secondary ip addresses for guest vm nic. Administrator will configure the these ip addresses on the nic manually. This addresses bug CLOUDSTACK-24. Diffs (updated) - api/src/com/cloud/network/IpAddress.java 47df4d6 api/src/com/cloud/network/NetworkService.java 95bcc42 api/src/com/cloud/network/rules/RulesService.java 921a86e api/src/com/cloud/vm/Nic.java 9d21130 api/src/com/cloud/vm/NicSecondaryIp.java PRE-CREATION api/src/org/apache/cloudstack/api/ApiConstants.java a26b468 api/src/org/apache/cloudstack/api/ResponseGenerator.java 267238a api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java 39ab812 api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java e0f9bcd api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java ce6ea16 api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListNicsCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/response/IPAddressResponse.java 251b2dd api/src/org/apache/cloudstack/api/response/NicResponse.java a7d1a0d api/src/org/apache/cloudstack/api/response/NicSecondaryIpResponse.java PRE-CREATION api/test/org/apache/cloudstack/api/command/test/AddIpToVmNicTest.java PRE-CREATION client/tomcatconf/commands.properties.in f03e8d5 server/src/com/cloud/api/ApiDBUtils.java ffee22f server/src/com/cloud/api/ApiResponseHelper.java eafee8a server/src/com/cloud/network/NetworkManager.java 2904183 server/src/com/cloud/network/NetworkManagerImpl.java 82893c4 server/src/com/cloud/network/NetworkModelImpl.java f978ac5 server/src/com/cloud/network/NetworkServiceImpl.java ce527b7 server/src/com/cloud/network/addr/PublicIp.java 7336c9c server/src/com/cloud/network/dao/IPAddressDao.java 9cdb975 server/src/com/cloud/network/dao/IPAddressDaoImpl.java e7067d9 server/src/com/cloud/network/dao/IPAddressVO.java 00da5eb server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java abb4973 server/src/com/cloud/network/rules/RulesManagerImpl.java 614d308 server/src/com/cloud/network/rules/dao/PortForwardingRulesDao.java 91f08e7 server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java 5406ab6 server/src/com/cloud/server/ManagementServerImpl.java e80d48c server/src/com/cloud/vm/NicVO.java 8e2edda server/src/com/cloud/vm/UserVmManagerImpl.java ce53c45 server/src/com/cloud/vm/dao/NicDao.java 762048b server/src/com/cloud/vm/dao/NicDaoImpl.java 5cf152f server/src/com/cloud/vm/dao/NicSecondaryIpDao.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpDaoImpl.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpVO.java PRE-CREATION server/test/com/cloud/network/MockNetworkManagerImpl.java 3568da5 server/test/com/cloud/network/MockRulesManagerImpl.java ba3dd41 server/test/com/cloud/vpc/MockNetworkManagerImpl.java 828a555 setup/db/db/schema-410to420.sql 4349bd0 Diff: https://reviews.apache.org/r/9396/diff/ Testing --- Tested add adding, deleting and listing addresses on the nic using APIs Thanks, Jayapal Reddy
Re: Review Request: multiple ip address per vm nic changes for isolated and vpc networks changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/ --- (Updated Feb. 26, 2013, 4:47 p.m.) Review request for cloudstack. Changes --- Incorporated review comments Description --- Using this feature we can reserve secondary ip addresses for guest vm nic. Administrator will configure the these ip addresses on the nic manually. This addresses bug CLOUDSTACK-24. Diffs (updated) - api/src/com/cloud/network/IpAddress.java 47df4d6 api/src/com/cloud/network/NetworkService.java 95bcc42 api/src/com/cloud/network/rules/RulesService.java 921a86e api/src/com/cloud/vm/Nic.java 9d21130 api/src/com/cloud/vm/NicSecondaryIp.java PRE-CREATION api/src/org/apache/cloudstack/api/ApiConstants.java 2a09de8 api/src/org/apache/cloudstack/api/ResponseGenerator.java 267238a api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java 39ab812 api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java e0f9bcd api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java ce6ea16 api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListNicsCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveNicFromVMCmd.java b1a870e api/src/org/apache/cloudstack/api/response/IPAddressResponse.java 251b2dd api/src/org/apache/cloudstack/api/response/NicResponse.java a7d1a0d api/src/org/apache/cloudstack/api/response/NicSecondaryIpResponse.java PRE-CREATION api/test/org/apache/cloudstack/api/command/test/AddIpToVmNicTest.java PRE-CREATION client/tomcatconf/commands.properties.in f03e8d5 server/src/com/cloud/api/ApiDBUtils.java ffee22f server/src/com/cloud/api/ApiResponseHelper.java eafee8a server/src/com/cloud/network/NetworkManager.java 2904183 server/src/com/cloud/network/NetworkManagerImpl.java 82893c4 server/src/com/cloud/network/NetworkModelImpl.java 7b3717a server/src/com/cloud/network/NetworkServiceImpl.java ce527b7 server/src/com/cloud/network/addr/PublicIp.java 7336c9c server/src/com/cloud/network/dao/IPAddressDao.java 9cdb975 server/src/com/cloud/network/dao/IPAddressDaoImpl.java e7067d9 server/src/com/cloud/network/dao/IPAddressVO.java 00da5eb server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java 531a428 server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 980d482 server/src/com/cloud/network/rules/RulesManagerImpl.java 614d308 server/src/com/cloud/network/rules/dao/PortForwardingRulesDao.java 91f08e7 server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java 5406ab6 server/src/com/cloud/server/ManagementServerImpl.java e80d48c server/src/com/cloud/vm/NicVO.java 8e2edda server/src/com/cloud/vm/UserVmManagerImpl.java c2bba63 server/src/com/cloud/vm/dao/NicDao.java 762048b server/src/com/cloud/vm/dao/NicDaoImpl.java 5cf152f server/src/com/cloud/vm/dao/NicSecondaryIpDao.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpDaoImpl.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpVO.java PRE-CREATION server/test/com/cloud/network/MockNetworkManagerImpl.java 3568da5 server/test/com/cloud/network/MockRulesManagerImpl.java ba3dd41 server/test/com/cloud/vpc/MockNetworkManagerImpl.java 828a555 setup/db/db/schema-410to420.sql 4349bd0 Diff: https://reviews.apache.org/r/9396/diff/ Testing --- Tested add adding, deleting and listing addresses on the nic using APIs Thanks, Jayapal Reddy
Re: Review Request: multiple ip address per vm nic changes for isolated and vpc networks changes.
On Feb. 25, 2013, 8:18 a.m., Murali Reddy wrote: api/src/com/cloud/network/NetworkService.java, line 166 https://reviews.apache.org/r/9396/diff/2/?file=259809#file259809line166 can you add a comment what each of the service interface does Removed listSecondaryIps, using listnics to list nic secondary ip details On Feb. 25, 2013, 8:18 a.m., Murali Reddy wrote: api/src/com/cloud/network/rules/RulesService.java, line 70 https://reviews.apache.org/r/9396/diff/2/?file=259810#file259810line70 can you rename the new parameter to guest IP or vm guest Ip Renamed On Feb. 25, 2013, 8:18 a.m., Murali Reddy wrote: api/src/org/apache/cloudstack/api/ApiConstants.java, line 446 https://reviews.apache.org/r/9396/diff/2/?file=259813#file259813line446 rename to vm guest ip done On Feb. 25, 2013, 8:18 a.m., Murali Reddy wrote: api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java, line 43 https://reviews.apache.org/r/9396/diff/2/?file=259817#file259817line43 Return NicSecondaryIpResponse instead of AddIpToVmNicResponse. I dont see reason why we need AddIpToVmNicResponse, unless you have reason dont add this response changed to NicSecondaryIpResponse On Feb. 25, 2013, 8:18 a.m., Murali Reddy wrote: api/src/org/apache/cloudstack/api/command/user/vm/ListSecondaryIPToNicCmd.java, line 44 https://reviews.apache.org/r/9396/diff/2/?file=259819#file259819line44 Description is wrong Should return NicSecondaryIpResponse, not AddIpToVmNicResponse changed to NicSecondaryIpResponse On Feb. 25, 2013, 8:18 a.m., Murali Reddy wrote: api/src/org/apache/cloudstack/api/response/ListNicSecondaryIpResponse.java, line 29 https://reviews.apache.org/r/9396/diff/2/?file=259824#file259824line29 This is not List. Keep this as just NicSecondaryIpResponse Removed this using NicSecondaryIpResponse On Feb. 25, 2013, 8:18 a.m., Murali Reddy wrote: api/src/org/apache/cloudstack/api/response/NicResponse.java, line 178 https://reviews.apache.org/r/9396/diff/2/?file=259825#file259825line178 Nic response has all the necessary details, do we still see need for ListSecondaryIPToNicCmd? One cas do just listNics and can get both the default IP and secondary IP list We are not using this so removed On Feb. 25, 2013, 8:18 a.m., Murali Reddy wrote: client/tomcatconf/commands.properties.in, line 331 https://reviews.apache.org/r/9396/diff/2/?file=259827#file259827line331 listSecondaryIpToNic API is missing Not required, removed listSecondaryIpToNic command On Feb. 25, 2013, 8:18 a.m., Murali Reddy wrote: server/src/com/cloud/network/NetworkServiceImpl.java, line 496 https://reviews.apache.org/r/9396/diff/2/?file=259833#file259833line496 there need to be check if caller can access the Nic, Network etc added access check for network On Feb. 25, 2013, 8:18 a.m., Murali Reddy wrote: server/src/com/cloud/vm/dao/NicDaoImpl.java, line 210 https://reviews.apache.org/r/9396/diff/2/?file=259846#file259846line210 I do not see any use of this search by secondary IP set, when you are passing the Nic id removed - Jayapal --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/#review17023 --- On Feb. 26, 2013, 4:47 p.m., Jayapal Reddy wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/ --- (Updated Feb. 26, 2013, 4:47 p.m.) Review request for cloudstack. Description --- Using this feature we can reserve secondary ip addresses for guest vm nic. Administrator will configure the these ip addresses on the nic manually. This addresses bug CLOUDSTACK-24. Diffs - api/src/com/cloud/network/IpAddress.java 47df4d6 api/src/com/cloud/network/NetworkService.java 95bcc42 api/src/com/cloud/network/rules/RulesService.java 921a86e api/src/com/cloud/vm/Nic.java 9d21130 api/src/com/cloud/vm/NicSecondaryIp.java PRE-CREATION api/src/org/apache/cloudstack/api/ApiConstants.java 2a09de8 api/src/org/apache/cloudstack/api/ResponseGenerator.java 267238a api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java 39ab812 api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java e0f9bcd api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java ce6ea16 api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListNicsCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java
Re: Review Request: multiple ip address per vm nic changes for isolated and vpc networks changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/#review17030 --- Also, ensure your code does not have tabs, properly indented, has no trailing white spaces. i see some commented out code, Plz remove them. - Murali Reddy On Feb. 19, 2013, 4:19 p.m., Jayapal Reddy wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/ --- (Updated Feb. 19, 2013, 4:19 p.m.) Review request for cloudstack. Description --- Using this feature we can reserve secondary ip addresses for guest vm nic. Administrator will configure the these ip addresses on the nic manually. This addresses bug CLOUDSTACK-24. Diffs - api/src/com/cloud/network/IpAddress.java 47df4d6 api/src/com/cloud/network/NetworkService.java ace1bb6 api/src/com/cloud/network/rules/RulesService.java 921a86e api/src/com/cloud/vm/Nic.java 9d21130 api/src/com/cloud/vm/NicSecondaryIp.java PRE-CREATION api/src/org/apache/cloudstack/api/ApiConstants.java cd7d700 api/src/org/apache/cloudstack/api/ResponseGenerator.java 267238a api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java 39ab812 api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java ce6ea16 api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListNicsCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListSecondaryIPToNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveNicFromVMCmd.java b1a870e api/src/org/apache/cloudstack/api/response/AddIpToVmNicResponse.java PRE-CREATION api/src/org/apache/cloudstack/api/response/IPAddressResponse.java 251b2dd api/src/org/apache/cloudstack/api/response/ListNicSecondaryIpResponse.java PRE-CREATION api/src/org/apache/cloudstack/api/response/NicResponse.java a7d1a0d api/test/org/apache/cloudstack/api/command/test/AddIpToVmNicTest.java PRE-CREATION client/tomcatconf/commands.properties.in e1d0fb2 server/src/com/cloud/api/ApiDBUtils.java e6b1bf1 server/src/com/cloud/api/ApiResponseHelper.java a94e935 server/src/com/cloud/network/NetworkManager.java 2904183 server/src/com/cloud/network/NetworkManagerImpl.java f586865 server/src/com/cloud/network/NetworkModelImpl.java beebb87 server/src/com/cloud/network/NetworkServiceImpl.java 37b4903 server/src/com/cloud/network/addr/PublicIp.java 7336c9c server/src/com/cloud/network/dao/IPAddressDao.java 9cdb975 server/src/com/cloud/network/dao/IPAddressDaoImpl.java e7067d9 server/src/com/cloud/network/dao/IPAddressVO.java 00da5eb server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 1abca51 server/src/com/cloud/network/rules/RulesManagerImpl.java 614d308 server/src/com/cloud/network/rules/dao/PortForwardingRulesDao.java 91f08e7 server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java 5406ab6 server/src/com/cloud/server/ManagementServerImpl.java d381206 server/src/com/cloud/vm/NicVO.java 8e2edda server/src/com/cloud/vm/UserVmManagerImpl.java df97609 server/src/com/cloud/vm/dao/NicDao.java 762048b server/src/com/cloud/vm/dao/NicDaoImpl.java 5cf152f server/src/com/cloud/vm/dao/NicSecondaryIpDao.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpDaoImpl.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpVO.java PRE-CREATION server/test/com/cloud/network/MockNetworkManagerImpl.java 4a24f9a server/test/com/cloud/network/MockRulesManagerImpl.java ba3dd41 server/test/com/cloud/vpc/MockNetworkManagerImpl.java bcaaa26 setup/db/db/schema-410to420.sql 65add75 Diff: https://reviews.apache.org/r/9396/diff/ Testing --- Tested add adding, deleting and listing addresses on the nic using APIs Thanks, Jayapal Reddy
Re: Review Request: multiple ip address per vm nic changes for isolated and vpc networks changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/ --- (Updated Feb. 19, 2013, 4:19 p.m.) Review request for cloudstack. Changes --- Added unit test cases and updated the sql file in the new patch Description --- Using this feature we can reserve secondary ip addresses for guest vm nic. Administrator will configure the these ip addresses on the nic manually. This addresses bug CLOUDSTACK-24. Diffs (updated) - api/src/com/cloud/network/IpAddress.java 47df4d6 api/src/com/cloud/network/NetworkService.java ace1bb6 api/src/com/cloud/network/rules/RulesService.java 921a86e api/src/com/cloud/vm/Nic.java 9d21130 api/src/com/cloud/vm/NicSecondaryIp.java PRE-CREATION api/src/org/apache/cloudstack/api/ApiConstants.java cd7d700 api/src/org/apache/cloudstack/api/ResponseGenerator.java 267238a api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java 39ab812 api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java ce6ea16 api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListNicsCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListSecondaryIPToNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveNicFromVMCmd.java b1a870e api/src/org/apache/cloudstack/api/response/AddIpToVmNicResponse.java PRE-CREATION api/src/org/apache/cloudstack/api/response/IPAddressResponse.java 251b2dd api/src/org/apache/cloudstack/api/response/ListNicSecondaryIpResponse.java PRE-CREATION api/src/org/apache/cloudstack/api/response/NicResponse.java a7d1a0d api/test/org/apache/cloudstack/api/command/test/AddIpToVmNicTest.java PRE-CREATION client/tomcatconf/commands.properties.in e1d0fb2 server/src/com/cloud/api/ApiDBUtils.java e6b1bf1 server/src/com/cloud/api/ApiResponseHelper.java a94e935 server/src/com/cloud/network/NetworkManager.java 2904183 server/src/com/cloud/network/NetworkManagerImpl.java f586865 server/src/com/cloud/network/NetworkModelImpl.java beebb87 server/src/com/cloud/network/NetworkServiceImpl.java 37b4903 server/src/com/cloud/network/addr/PublicIp.java 7336c9c server/src/com/cloud/network/dao/IPAddressDao.java 9cdb975 server/src/com/cloud/network/dao/IPAddressDaoImpl.java e7067d9 server/src/com/cloud/network/dao/IPAddressVO.java 00da5eb server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 1abca51 server/src/com/cloud/network/rules/RulesManagerImpl.java 614d308 server/src/com/cloud/network/rules/dao/PortForwardingRulesDao.java 91f08e7 server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java 5406ab6 server/src/com/cloud/server/ManagementServerImpl.java d381206 server/src/com/cloud/vm/NicVO.java 8e2edda server/src/com/cloud/vm/UserVmManagerImpl.java df97609 server/src/com/cloud/vm/dao/NicDao.java 762048b server/src/com/cloud/vm/dao/NicDaoImpl.java 5cf152f server/src/com/cloud/vm/dao/NicSecondaryIpDao.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpDaoImpl.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpVO.java PRE-CREATION server/test/com/cloud/network/MockNetworkManagerImpl.java 4a24f9a server/test/com/cloud/network/MockRulesManagerImpl.java ba3dd41 server/test/com/cloud/vpc/MockNetworkManagerImpl.java bcaaa26 setup/db/db/schema-410to420.sql 65add75 Diff: https://reviews.apache.org/r/9396/diff/ Testing --- Tested add adding, deleting and listing addresses on the nic using APIs Thanks, Jayapal Reddy
Re: Review Request: multiple ip address per vm nic changes for isolated and vpc networks changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/#review16419 --- Agree with David. There are several new class files, many of which have logic in them, so I would expect to see unit tests that cover those classes (especially those methods with logic). - Chip Childers On Feb. 10, 2013, 9:12 a.m., Jayapal Reddy wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/ --- (Updated Feb. 10, 2013, 9:12 a.m.) Review request for cloudstack. Description --- Using this feature we can reserve secondary ip addresses for guest vm nic. Administrator will configure the these ip addresses on the nic manually. This addresses bug CLOUDSTACK-24. Diffs - api/src/com/cloud/network/IpAddress.java 47df4d6 api/src/com/cloud/network/NetworkService.java ace1bb6 api/src/com/cloud/network/rules/RulesService.java 921a86e api/src/com/cloud/vm/Nic.java 9d21130 api/src/com/cloud/vm/NicSecondaryIp.java PRE-CREATION api/src/org/apache/cloudstack/api/ApiConstants.java d29408e api/src/org/apache/cloudstack/api/ResponseGenerator.java 0dc85de api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java 39ab812 api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java e0f9bcd api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java ce6ea16 api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListNicsCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListSecondaryIPToNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveNicFromVMCmd.java b1a870e api/src/org/apache/cloudstack/api/response/AddIpToVmNicResponse.java PRE-CREATION api/src/org/apache/cloudstack/api/response/IPAddressResponse.java 251b2dd api/src/org/apache/cloudstack/api/response/ListNicSecondaryIpResponse.java PRE-CREATION api/src/org/apache/cloudstack/api/response/NicResponse.java a7d1a0d client/tomcatconf/commands.properties.in d70649b server/src/com/cloud/api/ApiDBUtils.java 83132c6 server/src/com/cloud/api/ApiResponseHelper.java 8c97615 server/src/com/cloud/network/NetworkManager.java 2904183 server/src/com/cloud/network/NetworkManagerImpl.java 7a6ac27 server/src/com/cloud/network/NetworkModelImpl.java ce48e84 server/src/com/cloud/network/NetworkServiceImpl.java 050a1fe server/src/com/cloud/network/addr/PublicIp.java 7336c9c server/src/com/cloud/network/dao/IPAddressDao.java 9cdb975 server/src/com/cloud/network/dao/IPAddressDaoImpl.java e7067d9 server/src/com/cloud/network/dao/IPAddressVO.java 00da5eb server/src/com/cloud/network/dao/LoadBalancerVMMapVO.java 8856993 server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java 85e850c server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 1abca51 server/src/com/cloud/network/rules/RulesManagerImpl.java 0a00d22 server/src/com/cloud/network/rules/dao/PortForwardingRulesDao.java 91f08e7 server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java 5406ab6 server/src/com/cloud/vm/NicVO.java 8e2edda server/src/com/cloud/vm/UserVmManagerImpl.java 33a53d9 server/src/com/cloud/vm/dao/NicDao.java 762048b server/src/com/cloud/vm/dao/NicDaoImpl.java 5cf152f server/src/com/cloud/vm/dao/NicSecondaryIpDao.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpDaoImpl.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpVO.java PRE-CREATION server/test/com/cloud/network/MockNetworkManagerImpl.java 4a24f9a server/test/com/cloud/network/MockRulesManagerImpl.java ba3dd41 server/test/com/cloud/vpc/MockNetworkManagerImpl.java bcaaa26 setup/db/create-schema.sql f89c885 setup/db/db/schema-40to410.sql d771a15 Diff: https://reviews.apache.org/r/9396/diff/ Testing --- Tested add adding, deleting and listing addresses on the nic using APIs Thanks, Jayapal Reddy
Re: Review Request: multiple ip address per vm nic changes for isolated and vpc networks changes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/#review16397 --- I don't see any tests, and there are a number of new classes introduced, and several others modified. Do you plan on adding tests with this submission? setup/db/db/schema-40to410.sql https://reviews.apache.org/r/9396/#comment34881 I think this content should go in 4.1 - 4.2 upgrade script. 4.1 has already entered feature freeze. - David Nalley On Feb. 10, 2013, 9:12 a.m., Jayapal Reddy wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/ --- (Updated Feb. 10, 2013, 9:12 a.m.) Review request for cloudstack. Description --- Using this feature we can reserve secondary ip addresses for guest vm nic. Administrator will configure the these ip addresses on the nic manually. This addresses bug CLOUDSTACK-24. Diffs - api/src/com/cloud/network/IpAddress.java 47df4d6 api/src/com/cloud/network/NetworkService.java ace1bb6 api/src/com/cloud/network/rules/RulesService.java 921a86e api/src/com/cloud/vm/Nic.java 9d21130 api/src/com/cloud/vm/NicSecondaryIp.java PRE-CREATION api/src/org/apache/cloudstack/api/ApiConstants.java d29408e api/src/org/apache/cloudstack/api/ResponseGenerator.java 0dc85de api/src/org/apache/cloudstack/api/command/user/firewall/CreatePortForwardingRuleCmd.java 39ab812 api/src/org/apache/cloudstack/api/command/user/loadbalancer/AssignToLoadBalancerRuleCmd.java e0f9bcd api/src/org/apache/cloudstack/api/command/user/nat/EnableStaticNatCmd.java ce6ea16 api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListNicsCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/ListSecondaryIPToNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveIpFromVmNicCmd.java PRE-CREATION api/src/org/apache/cloudstack/api/command/user/vm/RemoveNicFromVMCmd.java b1a870e api/src/org/apache/cloudstack/api/response/AddIpToVmNicResponse.java PRE-CREATION api/src/org/apache/cloudstack/api/response/IPAddressResponse.java 251b2dd api/src/org/apache/cloudstack/api/response/ListNicSecondaryIpResponse.java PRE-CREATION api/src/org/apache/cloudstack/api/response/NicResponse.java a7d1a0d client/tomcatconf/commands.properties.in d70649b server/src/com/cloud/api/ApiDBUtils.java 83132c6 server/src/com/cloud/api/ApiResponseHelper.java 8c97615 server/src/com/cloud/network/NetworkManager.java 2904183 server/src/com/cloud/network/NetworkManagerImpl.java 7a6ac27 server/src/com/cloud/network/NetworkModelImpl.java ce48e84 server/src/com/cloud/network/NetworkServiceImpl.java 050a1fe server/src/com/cloud/network/addr/PublicIp.java 7336c9c server/src/com/cloud/network/dao/IPAddressDao.java 9cdb975 server/src/com/cloud/network/dao/IPAddressDaoImpl.java e7067d9 server/src/com/cloud/network/dao/IPAddressVO.java 00da5eb server/src/com/cloud/network/dao/LoadBalancerVMMapVO.java 8856993 server/src/com/cloud/network/lb/LoadBalancingRulesManagerImpl.java 85e850c server/src/com/cloud/network/router/VirtualNetworkApplianceManagerImpl.java 1abca51 server/src/com/cloud/network/rules/RulesManagerImpl.java 0a00d22 server/src/com/cloud/network/rules/dao/PortForwardingRulesDao.java 91f08e7 server/src/com/cloud/network/rules/dao/PortForwardingRulesDaoImpl.java 5406ab6 server/src/com/cloud/vm/NicVO.java 8e2edda server/src/com/cloud/vm/UserVmManagerImpl.java 33a53d9 server/src/com/cloud/vm/dao/NicDao.java 762048b server/src/com/cloud/vm/dao/NicDaoImpl.java 5cf152f server/src/com/cloud/vm/dao/NicSecondaryIpDao.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpDaoImpl.java PRE-CREATION server/src/com/cloud/vm/dao/NicSecondaryIpVO.java PRE-CREATION server/test/com/cloud/network/MockNetworkManagerImpl.java 4a24f9a server/test/com/cloud/network/MockRulesManagerImpl.java ba3dd41 server/test/com/cloud/vpc/MockNetworkManagerImpl.java bcaaa26 setup/db/create-schema.sql f89c885 setup/db/db/schema-40to410.sql d771a15 Diff: https://reviews.apache.org/r/9396/diff/ Testing --- Tested add adding, deleting and listing addresses on the nic using APIs Thanks, Jayapal Reddy