You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by saksham srivastava <sa...@citrix.com> on 2013/02/01 07:06:19 UTC
Re: Review Request: IP Address Reservation within a Network
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9180/
-----------------------------------------------------------
(Updated Feb. 1, 2013, 6:06 a.m.)
Review request for cloudstack, Murali Reddy and Chiradeep Vittal.
Changes
-------
Fixing the extra whitespaces.
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 (updated)
-----
api/src/com/cloud/network/Network.java 413b6d9
api/src/com/cloud/network/NetworkProfile.java bb59b04
api/src/com/cloud/network/NetworkService.java 786afb1
api/src/com/cloud/network/vpc/VpcService.java 68e062c
api/src/org/apache/cloudstack/api/ApiConstants.java d242830
api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java 978c71b
api/src/org/apache/cloudstack/api/response/NetworkResponse.java 64cc953
server/src/com/cloud/api/ApiResponseHelper.java 641f25b
server/src/com/cloud/network/NetworkServiceImpl.java 7530e94
server/src/com/cloud/network/NetworkVO.java 14b643b
server/src/com/cloud/network/vpc/VpcManagerImpl.java fa6bd30
server/test/com/cloud/network/MockNetworkManagerImpl.java e628033
server/test/com/cloud/vpc/MockNetworkManagerImpl.java a0f9d00
server/test/com/cloud/vpc/MockVpcManagerImpl.java 25799d1
setup/db/create-schema.sql ead98a5
setup/db/db/schema-40to410.sql ed4946e
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
Re: Review Request: IP Address Reservation within a Network
Posted by Saksham Srivastava <sa...@citrix.com>.
> On Feb. 11, 2013, 7:22 a.m., Murali Reddy wrote:
> > api/src/com/cloud/network/NetworkProfile.java, line 71
> > <https://reviews.apache.org/r/9180/diff/4/?file=257005#file257005line71>
> >
> > put related fields, setters, initialisation code together
> >
> > i see same issue in multiple places
This happened due to merge conflicts with IPv6. I have changed the order now.
> On Feb. 11, 2013, 7:22 a.m., Murali Reddy wrote:
> > api/src/com/cloud/network/Network.java, line 293
> > <https://reviews.apache.org/r/9180/diff/4/?file=257004#file257004line293>
> >
> > 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
Changed getGuestCidr to getNetworkCidr, added comments for both.
> On Feb. 11, 2013, 7:22 a.m., Murali Reddy wrote:
> > api/src/org/apache/cloudstack/api/response/NetworkResponse.java, lines 49-50
> > <https://reviews.apache.org/r/9180/diff/4/?file=257010#file257010line49>
> >
> > keep guestCidr, cidr, reservedIprange to gether
Done.
> On Feb. 11, 2013, 7:22 a.m., Murali Reddy wrote:
> > api/src/org/apache/cloudstack/api/response/NetworkResponse.java, line 59
> > <https://reviews.apache.org/r/9180/diff/4/?file=257010#file257010line59>
> >
> > comment for both guestCidr, cidr are not good enough for one to understand the difference and relation between them
Added exhaustive comments for both.
> On Feb. 11, 2013, 7:22 a.m., Murali Reddy wrote:
> > server/src/com/cloud/network/NetworkServiceImpl.java, line 1666
> > <https://reviews.apache.org/r/9180/diff/4/?file=257012#file257012line1666>
> >
> > add comment explaining logic for both cases updating already configured value and new value bing configured
Done.
> On Feb. 11, 2013, 7:22 a.m., Murali Reddy wrote:
> > server/src/com/cloud/network/NetworkServiceImpl.java, line 1667
> > <https://reviews.apache.org/r/9180/diff/4/?file=257012#file257012line1667>
> >
> > exception message is wrong
> >
> > you can mention it as guestVmcidr, need to be subset of gestCidr
Changed the exception message.
> On Feb. 11, 2013, 7:22 a.m., Murali Reddy wrote:
> > server/src/com/cloud/network/NetworkServiceImpl.java, line 1669
> > <https://reviews.apache.org/r/9180/diff/4/?file=257012#file257012line1669>
> >
> > indentation is not correct
Improved it.
> On Feb. 11, 2013, 7:22 a.m., Murali Reddy wrote:
> > server/src/com/cloud/network/NetworkServiceImpl.java, line 1671
> > <https://reviews.apache.org/r/9180/diff/4/?file=257012#file257012line1671>
> >
> > same here, exception message is not correct
Changed.
> On Feb. 11, 2013, 7:22 a.m., Murali Reddy wrote:
> > server/src/com/cloud/network/NetworkServiceImpl.java, line 1678
> > <https://reviews.apache.org/r/9180/diff/4/?file=257012#file257012line1678>
> >
> > put this block under, previous if (guestVmCidr!= null ) {} block
Done.
> On Feb. 11, 2013, 7:22 a.m., Murali Reddy wrote:
> > setup/db/create-schema.sql, line 249
> > <https://reviews.apache.org/r/9180/diff/4/?file=257018#file257018line249>
> >
> > 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'
Added relevant comments for it.
> On Feb. 11, 2013, 7:22 a.m., Murali Reddy wrote:
> > setup/db/create-schema.sql, line 252
> > <https://reviews.apache.org/r/9180/diff/4/?file=257018#file257018line252>
> >
> > 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
network_cidr added in place of guest_cidr. Added intuitive comments.
> On Feb. 11, 2013, 7:22 a.m., Murali Reddy wrote:
> > setup/db/create-schema.sql, line 252
> > <https://reviews.apache.org/r/9180/diff/4/?file=257018#file257018line252>
> >
> > 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
network_cidr added in place of guest_cidr. Added intuitive comments.
> On Feb. 11, 2013, 7:22 a.m., Murali Reddy wrote:
> > setup/db/db/schema-40to410.sql, line 85
> > <https://reviews.apache.org/r/9180/diff/4/?file=257019#file257019line85>
> >
> > you should be using 4.1 to 4.2 upgrade script
Changed it.
- Saksham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9180/#review16414
-----------------------------------------------------------
On Feb. 19, 2013, 12:57 p.m., Saksham Srivastava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9180/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2013, 12:57 p.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 cd7d700
> 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 a94e935
> server/src/com/cloud/network/NetworkServiceImpl.java 37b4903
> server/src/com/cloud/network/dao/NetworkVO.java d51a065
> server/src/com/cloud/network/vpc/VpcManagerImpl.java fbb5788
> server/src/com/cloud/upgrade/dao/Upgrade410to420.java a43727c
> 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/db/schema-410to420.sql 65add75
>
> 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
>
>
RE: Review Request: IP Address Reservation within a Network
Posted by Animesh Chaturvedi <an...@citrix.com>.
Saksham
Please respond to review comments with updated patch
Animesh
> -----Original Message-----
> From: Murali Reddy [mailto:noreply@reviews.apache.org] On Behalf Of Murali
> Reddy
> Sent: Sunday, February 10, 2013 11:22 PM
> To: Murali Reddy; Chiradeep Vittal
> Cc: Saksham Srivastava; cloudstack
> Subject: Re: Review Request: IP Address Reservation within a Network
>
>
> -----------------------------------------------------------
> 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/UpdateNetworkC
> md.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
> >
> >
Re: Review Request: IP Address Reservation within a Network
Posted by Murali Reddy <mu...@gmail.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request: IP Address Reservation within a Network
Posted by Murali Reddy <mu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9180/#review16930
-----------------------------------------------------------
Ship it!
merged to master with commit
commit ea3db2f073a5076d7822497a3032a0cd2d47a297
Author: Saksham Srivastava <sa...@citrix.com>
Date: Fri Feb 22 15:46:45 2013 +0530
- Murali Reddy
On Feb. 19, 2013, 12:57 p.m., Saksham Srivastava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9180/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2013, 12:57 p.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 cd7d700
> 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 a94e935
> server/src/com/cloud/network/NetworkServiceImpl.java 37b4903
> server/src/com/cloud/network/dao/NetworkVO.java d51a065
> server/src/com/cloud/network/vpc/VpcManagerImpl.java fbb5788
> server/src/com/cloud/upgrade/dao/Upgrade410to420.java a43727c
> 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/db/schema-410to420.sql 65add75
>
> 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
>
>
Re: Review Request: IP Address Reservation within a Network
Posted by Saksham Srivastava <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9180/
-----------------------------------------------------------
(Updated Feb. 19, 2013, 12:57 p.m.)
Review request for cloudstack, Murali Reddy and Chiradeep Vittal.
Changes
-------
Incorporated review comments.
Renamed guest_cidr to network_cidr.
Added intutive comments for the variables.
Patch applies cleanly on latest master.
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 (updated)
-----
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 cd7d700
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 a94e935
server/src/com/cloud/network/NetworkServiceImpl.java 37b4903
server/src/com/cloud/network/dao/NetworkVO.java d51a065
server/src/com/cloud/network/vpc/VpcManagerImpl.java fbb5788
server/src/com/cloud/upgrade/dao/Upgrade410to420.java a43727c
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/db/schema-410to420.sql 65add75
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
Re: Review Request: IP Address Reservation within a Network
Posted by Saksham Srivastava <sa...@citrix.com>.
> On Feb. 19, 2013, 1:24 a.m., Sateesh Chodapuneedi wrote:
> > server/src/com/cloud/network/NetworkServiceImpl.java, line 1680
> > <https://reviews.apache.org/r/9180/diff/4/?file=257012#file257012line1680>
> >
> > Even de-activated nics would be included in the list of nicsPresent. If a system is used for long term this list may be huge with many de-activated nics to iterate in the below loop. Better to get list of nics that are not removed.
list will have only non removed nics.
- Saksham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9180/#review16726
-----------------------------------------------------------
On Feb. 19, 2013, 12:57 p.m., Saksham Srivastava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9180/
> -----------------------------------------------------------
>
> (Updated Feb. 19, 2013, 12:57 p.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 cd7d700
> 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 a94e935
> server/src/com/cloud/network/NetworkServiceImpl.java 37b4903
> server/src/com/cloud/network/dao/NetworkVO.java d51a065
> server/src/com/cloud/network/vpc/VpcManagerImpl.java fbb5788
> server/src/com/cloud/upgrade/dao/Upgrade410to420.java a43727c
> 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/db/schema-410to420.sql 65add75
>
> 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
>
>
Re: Review Request: IP Address Reservation within a Network
Posted by Sateesh Chodapuneedi <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9180/#review16726
-----------------------------------------------------------
server/src/com/cloud/api/ApiResponseHelper.java
<https://reviews.apache.org/r/9180/#comment35469>
Both if statements can be combined together with &&
server/src/com/cloud/network/NetworkServiceImpl.java
<https://reviews.apache.org/r/9180/#comment35468>
Specify that ip reservation cant be supported over isolated guest networks.
server/src/com/cloud/network/NetworkServiceImpl.java
<https://reviews.apache.org/r/9180/#comment35467>
Specify the parameter that is invalid.
server/src/com/cloud/network/NetworkServiceImpl.java
<https://reviews.apache.org/r/9180/#comment35466>
we can avoid nesting of if statement here. Combine both if statement.
server/src/com/cloud/network/NetworkServiceImpl.java
<https://reviews.apache.org/r/9180/#comment35460>
Even de-activated nics would be included in the list of nicsPresent. If a system is used for long term this list may be huge with many de-activated nics to iterate in the below loop. Better to get list of nics that are not removed.
server/src/com/cloud/network/NetworkServiceImpl.java
<https://reviews.apache.org/r/9180/#comment35465>
elaborate guestvmcidr.
server/src/com/cloud/network/NetworkServiceImpl.java
<https://reviews.apache.org/r/9180/#comment35464>
remove variable name from message. This could be elaborated.
server/src/com/cloud/network/NetworkServiceImpl.java
<https://reviews.apache.org/r/9180/#comment35461>
Specify active IP address here.
- Sateesh Chodapuneedi
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
>
>
Re: Review Request: IP Address Reservation within a Network
Posted by Saksham Srivastava <sa...@citrix.com>.
-----------------------------------------------------------
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.
Changes
-------
Patch uploaded after Javelin and Ipv6 merges.
Removed merge conflicts.
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 (updated)
-----
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
Re: Review Request: IP Address Reservation within a Network
Posted by saksham srivastava <sa...@citrix.com>.
> On Feb. 1, 2013, 11:07 a.m., Murali Reddy wrote:
> > Also its not clear to me, how you are ensuring that IP is allocated only from the guest CIDR range when its different from the network CIDR?
Murali, thanks a lot for your reviews. I am answering all your queries here.
For this feature a column guest_cidr is added in networks table. So there are now 2 columns in the table "cidr" and "guest_cidr".
1) For non isolated guest networks there has been no change at all, the "cidr" column continues to be the way it used to be i.e. the network cidr.
and "guest_cidr" always continues to be null, throughout the life cycle.
============================================================================
2) For isolated guest networks :
Whenever a network is created, "cidr" will have network cidr and "guest_cidr" will be null.(The way it has been working).
When there is no reservation, "guest_cidr" is null, and "cidr" continues to be the network cidr.
When first time reservation happens, a valid guestvmcidr is passed as parameter in updateNetwork API:
"guest_cidr" copies the value of network cidr from "cidr" column
and "cidr" gets populated with guestvmcidr
so "cidr" is now the effective IP range for guest vms.
Now for the next time when reservation is reapplied, the "guest_cidr" already has network cidr and
"cidr" gets again re-popluated by the new guestvmcidr passed in the API.
Same continues for every subsequent reservation.
Guest VMs always pick IPs from "cidr" as usual and thus reservation gets applied.
In short "guest_cidr" will now always have the network cidr
"cidr" will always have the guestvmcidr value
When the network is reimplemented like in case of upgrade to ext offerings :
"cidr" is set to network cidr by copying the value from "guest_cidr"
"guest_cidr" is reset to null to avoid an inconsistent state after the reimplementation.
After reimplementation "cidr" will have the new network cidr and "guest_cidr" will be null and reservation can now be reapplied.
The reason of doing this was not to touch Gurus and NetworkManagerImpl at all and also not to alter the functionality of "cidr" and continue
it to serve as the address space for Guest VMs.
=============================================================================
Response:
guestcidr is the network cidr
cidr is the effective cidr for guest vms
reserved_ip_range is list of IPs to be used for non cloudstack purposes.
The reason to add reserved_ip_range is to help reduce the IP calculations by admin.
For instance :
guestcidr = 10.1.1.0/24
cidr 10.1.1.64/26
reservediprange : 10.1.1.1-10.1.1.63 , 10.1.1.128-10.1.1.254
So to ease the calculation for the admin and also list the reservation range which is the prime aim of this feature, reservediprange is added.
I would sincerely agree to document both the request and response to avoid any ambiguity.
- saksham
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9180/#review16004
-----------------------------------------------------------
On Feb. 1, 2013, 10:34 a.m., saksham srivastava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9180/
> -----------------------------------------------------------
>
> (Updated Feb. 1, 2013, 10:34 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 a70bf02
> api/src/com/cloud/network/NetworkProfile.java e3be941
> api/src/com/cloud/network/NetworkService.java 786afb1
> api/src/com/cloud/network/vpc/VpcService.java 68e062c
> api/src/org/apache/cloudstack/api/ApiConstants.java d895191
> api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java 978c71b
> api/src/org/apache/cloudstack/api/response/NetworkResponse.java 40d6850
> server/src/com/cloud/api/ApiResponseHelper.java 2dcd09c
> server/src/com/cloud/network/NetworkServiceImpl.java 5c70caa
> server/src/com/cloud/network/NetworkVO.java 8020e7a
> server/src/com/cloud/network/vpc/VpcManagerImpl.java 26e882e
> server/test/com/cloud/network/MockNetworkManagerImpl.java af0b03f
> server/test/com/cloud/vpc/MockNetworkManagerImpl.java 382068a
> server/test/com/cloud/vpc/MockVpcManagerImpl.java 25799d1
> setup/db/create-schema.sql a847b43
> setup/db/db/schema-40to410.sql 6d5b262
>
> 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
>
>
Re: Review Request: IP Address Reservation within a Network
Posted by Murali Reddy <mu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9180/#review16004
-----------------------------------------------------------
Also its not clear to me, how you are ensuring that IP is allocated only from the guest CIDR range when its different from the network CIDR?
- Murali Reddy
On Feb. 1, 2013, 10:34 a.m., saksham srivastava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9180/
> -----------------------------------------------------------
>
> (Updated Feb. 1, 2013, 10:34 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 a70bf02
> api/src/com/cloud/network/NetworkProfile.java e3be941
> api/src/com/cloud/network/NetworkService.java 786afb1
> api/src/com/cloud/network/vpc/VpcService.java 68e062c
> api/src/org/apache/cloudstack/api/ApiConstants.java d895191
> api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java 978c71b
> api/src/org/apache/cloudstack/api/response/NetworkResponse.java 40d6850
> server/src/com/cloud/api/ApiResponseHelper.java 2dcd09c
> server/src/com/cloud/network/NetworkServiceImpl.java 5c70caa
> server/src/com/cloud/network/NetworkVO.java 8020e7a
> server/src/com/cloud/network/vpc/VpcManagerImpl.java 26e882e
> server/test/com/cloud/network/MockNetworkManagerImpl.java af0b03f
> server/test/com/cloud/vpc/MockNetworkManagerImpl.java 382068a
> server/test/com/cloud/vpc/MockVpcManagerImpl.java 25799d1
> setup/db/create-schema.sql a847b43
> setup/db/db/schema-40to410.sql 6d5b262
>
> 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
>
>
Re: Review Request: IP Address Reservation within a Network
Posted by Murali Reddy <mu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9180/#review16003
-----------------------------------------------------------
api/src/com/cloud/network/Network.java
<https://reviews.apache.org/r/9180/#comment34304>
can you rename getter names to reflect what they are returning
getNeworkCidr getGuestVmCidr
api/src/org/apache/cloudstack/api/response/NetworkResponse.java
<https://reviews.apache.org/r/9180/#comment34305>
keep both request and response has same cidr.
i.e specify only guestVmCidr,
server/src/com/cloud/api/ApiResponseHelper.java
<https://reviews.apache.org/r/9180/#comment34306>
return what is reserved for guest VM, let the caller figure what is reserved for non cloudstack VM's
setup/db/create-schema.sql
<https://reviews.apache.org/r/9180/#comment34303>
can you rename to reflect the purpose of CIDR and give better description message defining the fields
e.g., network_cidr, guest_vm_cidr
setup/db/db/schema-40to410.sql
<https://reviews.apache.org/r/9180/#comment34307>
do you expect the guest_cidr's to be null after upgrade or same as cidr?
- Murali Reddy
On Feb. 1, 2013, 10:34 a.m., saksham srivastava wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9180/
> -----------------------------------------------------------
>
> (Updated Feb. 1, 2013, 10:34 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 a70bf02
> api/src/com/cloud/network/NetworkProfile.java e3be941
> api/src/com/cloud/network/NetworkService.java 786afb1
> api/src/com/cloud/network/vpc/VpcService.java 68e062c
> api/src/org/apache/cloudstack/api/ApiConstants.java d895191
> api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java 978c71b
> api/src/org/apache/cloudstack/api/response/NetworkResponse.java 40d6850
> server/src/com/cloud/api/ApiResponseHelper.java 2dcd09c
> server/src/com/cloud/network/NetworkServiceImpl.java 5c70caa
> server/src/com/cloud/network/NetworkVO.java 8020e7a
> server/src/com/cloud/network/vpc/VpcManagerImpl.java 26e882e
> server/test/com/cloud/network/MockNetworkManagerImpl.java af0b03f
> server/test/com/cloud/vpc/MockNetworkManagerImpl.java 382068a
> server/test/com/cloud/vpc/MockVpcManagerImpl.java 25799d1
> setup/db/create-schema.sql a847b43
> setup/db/db/schema-40to410.sql 6d5b262
>
> 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
>
>
Re: Review Request: IP Address Reservation within a Network
Posted by saksham srivastava <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9180/
-----------------------------------------------------------
(Updated Feb. 1, 2013, 10:34 a.m.)
Review request for cloudstack, Murali Reddy and Chiradeep Vittal.
Changes
-------
Modifying the patch after IPv6 merge.
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 (updated)
-----
api/src/com/cloud/network/Network.java a70bf02
api/src/com/cloud/network/NetworkProfile.java e3be941
api/src/com/cloud/network/NetworkService.java 786afb1
api/src/com/cloud/network/vpc/VpcService.java 68e062c
api/src/org/apache/cloudstack/api/ApiConstants.java d895191
api/src/org/apache/cloudstack/api/command/user/network/UpdateNetworkCmd.java 978c71b
api/src/org/apache/cloudstack/api/response/NetworkResponse.java 40d6850
server/src/com/cloud/api/ApiResponseHelper.java 2dcd09c
server/src/com/cloud/network/NetworkServiceImpl.java 5c70caa
server/src/com/cloud/network/NetworkVO.java 8020e7a
server/src/com/cloud/network/vpc/VpcManagerImpl.java 26e882e
server/test/com/cloud/network/MockNetworkManagerImpl.java af0b03f
server/test/com/cloud/vpc/MockNetworkManagerImpl.java 382068a
server/test/com/cloud/vpc/MockVpcManagerImpl.java 25799d1
setup/db/create-schema.sql a847b43
setup/db/db/schema-40to410.sql 6d5b262
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