----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9180/#review16414 -----------------------------------------------------------
api/src/com/cloud/network/Network.java <https://reviews.apache.org/r/9180/#comment34904> Please add comment that explaining what is getGuestCidr is for. Keep it along with getCidr() and put a comment for getCidr, explaining what it return in case of IP reservation is in place api/src/com/cloud/network/NetworkProfile.java <https://reviews.apache.org/r/9180/#comment34911> put related fields, setters, initialisation code together i see same issue in multiple places api/src/org/apache/cloudstack/api/response/NetworkResponse.java <https://reviews.apache.org/r/9180/#comment34905> keep guestCidr, cidr, reservedIprange to gether api/src/org/apache/cloudstack/api/response/NetworkResponse.java <https://reviews.apache.org/r/9180/#comment34906> comment for both guestCidr, cidr are not good enough for one to understand the difference and relation between them server/src/com/cloud/network/NetworkServiceImpl.java <https://reviews.apache.org/r/9180/#comment34907> add comment explaining logic for both cases updating already configured value and new value bing configured server/src/com/cloud/network/NetworkServiceImpl.java <https://reviews.apache.org/r/9180/#comment34908> exception message is wrong you can mention it as guestVmcidr, need to be subset of gestCidr server/src/com/cloud/network/NetworkServiceImpl.java <https://reviews.apache.org/r/9180/#comment34898> indentation is not correct server/src/com/cloud/network/NetworkServiceImpl.java <https://reviews.apache.org/r/9180/#comment34909> same here, exception message is not correct server/src/com/cloud/network/NetworkServiceImpl.java <https://reviews.apache.org/r/9180/#comment34899> put this block under, previous if (guestVmCidr!= null ) {} block setup/db/create-schema.sql <https://reviews.apache.org/r/9180/#comment34901> this no longer acting as 'network cidr'? this you are using it as cidr from which VM's get the IP right? Add right comment, also mention that this will be subset or equal to 'guest_cidr' setup/db/create-schema.sql <https://reviews.apache.org/r/9180/#comment34902> terminology is confusing, cidr, and guest_cidr which does what. Please add better comments or give intuitive names. Can we use 'guest_vm_cidr' to represent the CIDR from which VM's get the IP allocated. and 'network_cidr' to represent the CIDR of the network, that is inclusive of guest_vm_cidr and IP address that can be allocated out side cloudstack setup/db/create-schema.sql <https://reviews.apache.org/r/9180/#comment34903> terminology is confusing. cidr, and guest_cidr which does what? Please add better comments or give intuitive names. Can we use 'guest_vm_cidr' to represent the CIDR from which VM's get the IP allocated. and 'network_cidr' to represent the CIDR of the network, that is inclusive of guest_vm_cidr and IP address that can be allocated out side cloudstack setup/db/db/schema-40to410.sql <https://reviews.apache.org/r/9180/#comment34900> you should be using 4.1 to 4.2 upgrade script - Murali Reddy On Feb. 8, 2013, 9:39 a.m., Saksham Srivastava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9180/ > ----------------------------------------------------------- > > (Updated Feb. 8, 2013, 9:39 a.m.) > > > Review request for cloudstack, Murali Reddy and Chiradeep Vittal. > > > Description > ------- > > CLOUDSTACK-705 IP Address reservation for Isolated Guest Networks > > > CloudStack uses Guest CIDR for dhcp-range for the Guest VMs. The entire CIDR > is used by CloudStack for assigning IPs to Guest VMs. > IP Address Reservation will allow part of address space to be used for non > CloudStack hosts/physical servers also, by restricting the address space of > CloudStack Guest VMs. > Reservation can be configured using update Network API by specifying > guestvmCidr as an additional parameter. > Reservation will be applicable for Isolated Guest Networks including VPC. > reservediprange in the response will return the IP range that can be used for > non Cloudstack hosts. > > > This addresses bug CLOUDSTACK-705. > > > Diffs > ----- > > api/src/com/cloud/network/Network.java 2bf7b7f > api/src/com/cloud/network/NetworkProfile.java 37d46ac > api/src/com/cloud/network/NetworkService.java ace1bb6 > api/src/com/cloud/network/vpc/VpcService.java cc66b58 > api/src/org/apache/cloudstack/api/ApiConstants.java d29408e > > api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java > 6777407 > api/src/org/apache/cloudstack/api/response/NetworkResponse.java 7b29efb > server/src/com/cloud/api/ApiResponseHelper.java 8c97615 > server/src/com/cloud/network/NetworkServiceImpl.java d38d1f8 > server/src/com/cloud/network/dao/NetworkVO.java d51a065 > server/src/com/cloud/network/vpc/VpcManagerImpl.java 7197c36 > server/test/com/cloud/network/MockNetworkManagerImpl.java 4a24f9a > server/test/com/cloud/vpc/MockNetworkManagerImpl.java bcaaa26 > server/test/com/cloud/vpc/MockVpcManagerImpl.java 0a44a49 > setup/db/create-schema.sql f89c885 > setup/db/db/schema-40to410.sql d771a15 > > Diff: https://reviews.apache.org/r/9180/diff/ > > > Testing > ------- > > Tested manually the following scenarios: > Applying reservation when there are running VMs inside the guest_vm_cidr. > Applying reservation when there are running VMs outside the > guest_vm_cidr.(not allowed) > Applying reservation when external device like Netscaler is configured in the > guest_cidr. > Applying reservation in VPC tiers. > Applying reservation outside the range of guest_cidr.(not allowed) > > > Thanks, > > Saksham Srivastava > >
