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

Reply via email to