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