----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23805/#review48842 -----------------------------------------------------------
Looks good, let me get my office lab to work and then I can test. Meanwhile if others may want to test it? Pardon my ignorance but does this change involve any db table/column change since I see state filter added in publicIpAddressDao? api/src/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java <https://reviews.apache.org/r/23805/#comment85580> I guess moving it above was a cosmetic change. Please fix that you don't remove/change anything in the move refactor. api/src/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java <https://reviews.apache.org/r/23805/#comment85581> I guess the move refactor was a cosmetic change, but it removed getAllocatedOnly() method. Please fix it, or explain why we need to remove it? api/src/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java <https://reviews.apache.org/r/23805/#comment85579> This was removed. - Rohit Yadav On July 28, 2014, 10:12 a.m., Ilia Shakitko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23805/ > ----------------------------------------------------------- > > (Updated July 28, 2014, 10:12 a.m.) > > > Review request for cloudstack, Alena Prokharchyk, Alex Huang, Harikrishna > Patnala, Kishan Kavala, Prachi Damle, Rohit Yadav, Ilia Shakitko, Wei Zhou, > and Wido den Hollander. > > > Bugs: CLOUDSTACK-7159 > https://issues.apache.org/jira/browse/CLOUDSTACK-7159 > > > Repository: cloudstack-git > > > Description > ------- > > This improvement is introducing a new parameter for the > "listPublicIpAddresses" API call - "state". > > Few times we've faced an impedemence of having a list of "Free" > publicIpAddresses. You have to go thru all the IPs with (allocatedonly = > false) and filter out the "Free" once. It's not a big deal, but it's an extra > time and traffic between CloudStack and an API consumer. > > I also moved few methods out of the 'API Implementation' and put them above > as a minor refactoring. > Method "getForLoadBalancing" has been removed because it's not being used in > code anywhere else. > > This patch is done for "master" branch. > > > Diffs > ----- > > > api/src/org/apache/cloudstack/api/command/user/address/ListPublicIpAddressesCmd.java > 07ccfe9 > server/src/com/cloud/server/ManagementServerImpl.java 99b12732 > > Diff: https://reviews.apache.org/r/23805/diff/ > > > Testing > ------- > > 1) Build successfull > 2) No tests broken > 3) Tested few different calls with cloudmonkey: > > list publicipaddresses forvirtualnetwork=false listall=true page=1 pagesize=0 > count = 10 > > list publicipaddresses forvirtualnetwork=false listall=true > allocatedonly=false page=1 pagesize=0 > count = 100 > > list publicipaddresses forvirtualnetwork=false listall=true state=free page=1 > pagesize=0 > count = 90 > > > Thanks, > > Ilia Shakitko > >
