----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9396/#review17023 -----------------------------------------------------------
api/src/com/cloud/network/NetworkService.java <https://reviews.apache.org/r/9396/#comment36071> can you add a comment what each of the service interface does api/src/com/cloud/network/rules/RulesService.java <https://reviews.apache.org/r/9396/#comment36070> can you rename the new parameter to guest IP or vm guest Ip api/src/org/apache/cloudstack/api/ApiConstants.java <https://reviews.apache.org/r/9396/#comment36072> rename to vm guest ip api/src/org/apache/cloudstack/api/command/user/vm/AddIpToVmNicCmd.java <https://reviews.apache.org/r/9396/#comment36067> Return NicSecondaryIpResponse instead of AddIpToVmNicResponse. I dont see reason why we need AddIpToVmNicResponse, unless you have reason dont add this response api/src/org/apache/cloudstack/api/command/user/vm/ListSecondaryIPToNicCmd.java <https://reviews.apache.org/r/9396/#comment36068> Description is wrong Should return NicSecondaryIpResponse, not AddIpToVmNicResponse api/src/org/apache/cloudstack/api/response/ListNicSecondaryIpResponse.java <https://reviews.apache.org/r/9396/#comment36066> This is not List. Keep this as just NicSecondaryIpResponse api/src/org/apache/cloudstack/api/response/NicResponse.java <https://reviews.apache.org/r/9396/#comment36073> 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 client/tomcatconf/commands.properties.in <https://reviews.apache.org/r/9396/#comment36069> listSecondaryIpToNic API is missing server/src/com/cloud/network/NetworkManagerImpl.java <https://reviews.apache.org/r/9396/#comment36074> need validation on nic and VM id's server/src/com/cloud/network/NetworkServiceImpl.java <https://reviews.apache.org/r/9396/#comment36079> there need to be check if caller can access the Nic, Network etc server/src/com/cloud/network/NetworkServiceImpl.java <https://reviews.apache.org/r/9396/#comment36075> validate Nic ID as well server/src/com/cloud/vm/dao/NicDaoImpl.java <https://reviews.apache.org/r/9396/#comment36076> I do not see any use of this search by secondary IP set, when you are passing the Nic id - 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 > >
