cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jayapal Reddy" <jayapalreddy.ur...@citrix.com>
Subject Re: Review Request: multiple ip address per vm nic changes for isolated and vpc networks changes.
Date Tue, 26 Feb 2013 16:48:01 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message