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/03/19 10:06:37 UTC

Review Request: CLOUDSTACK-1647: IP Reservation should not happen if the guest-vm cidr and network cidr is not same but their start ip and end ip are same.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10005/
-----------------------------------------------------------

Review request for cloudstack, Murali Reddy and Sateesh Chodapuneedi.


Description
-------

In cases where the start ip and end ip of guest vm cidr and network cidr are same, even when the cidrs appear to be different,the reservation procedure should not go through and user should get a message mentioning that.
Added extra check for the same with proper alert message.


This addresses bug CLOUDSTACK-1647.


Diffs
-----

  server/src/com/cloud/network/NetworkServiceImpl.java 52e81e5 

Diff: https://reviews.apache.org/r/10005/diff/


Testing
-------

CIDR : 10.0.144.0/20, Network CIDR : null, guestVmCidr : 10.0.151.0/20 => Reservation is not applied.
CIDR : 10.0.144.0/21, Network CIDR : 10.0.144.0/20, guestVmCidr : 10.0.151.0/20 => Existing Reservation is not affected.


Thanks,

Saksham Srivastava


Re: Review Request: CLOUDSTACK-1647: IP Reservation should not happen if the guest-vm cidr and network cidr is not same but their start ip and end ip are same.

Posted by Chip Childers <ch...@sungard.com>.
On Fri, Apr 19, 2013 at 09:51:56AM +0000, Saksham Srivastava wrote:
> 
> 
> > On April 18, 2013, 5:21 p.m., Sateesh Chodapuneedi wrote:
> > > server/src/com/cloud/network/NetworkServiceImpl.java, line 1888
> > > <https://reviews.apache.org/r/10005/diff/1/?file=271870#file271870line1888>
> > >
> > >     This code seems dueplicated because the logic to calculate reserved ip range can be used to check if there exists some ip range to be reserved or not. If the calculated range is empty then do not attempt to reserve. Code to prepare response object already had this logic. May be we have to move this to NetUtils and call that from both places.
> 
> Thanks for the comment.
> 
> I have added the logic to NetUtils.java to maker it more generic and usable.
> However the response method does not need to call this boolean function, so making changes only in API checks.

Please see me comment in the review board record.  These new methods are
easily unit tested, so I'd like to see some unit tests to confirm
functionality (positive and negative tests please).

Re: Review Request: CLOUDSTACK-1647: IP Reservation should not happen if the guest-vm cidr and network cidr is not same but their start ip and end ip are same.

Posted by Saksham Srivastava <sa...@citrix.com>.

> On April 18, 2013, 5:21 p.m., Sateesh Chodapuneedi wrote:
> > server/src/com/cloud/network/NetworkServiceImpl.java, line 1888
> > <https://reviews.apache.org/r/10005/diff/1/?file=271870#file271870line1888>
> >
> >     This code seems dueplicated because the logic to calculate reserved ip range can be used to check if there exists some ip range to be reserved or not. If the calculated range is empty then do not attempt to reserve. Code to prepare response object already had this logic. May be we have to move this to NetUtils and call that from both places.

Thanks for the comment.

I have added the logic to NetUtils.java to maker it more generic and usable.
However the response method does not need to call this boolean function, so making changes only in API checks.


- Saksham


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10005/#review19396
-----------------------------------------------------------


On April 19, 2013, 9:45 a.m., Saksham Srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10005/
> -----------------------------------------------------------
> 
> (Updated April 19, 2013, 9:45 a.m.)
> 
> 
> Review request for cloudstack, Murali Reddy and Sateesh Chodapuneedi.
> 
> 
> Description
> -------
> 
> In cases where the start ip and end ip of guest vm cidr and network cidr are same, even when the cidrs appear to be different,the reservation procedure should not go through and user should get a message mentioning that.
> Added extra check for the same with proper alert message.
> 
> 
> This addresses bug CLOUDSTACK-1647.
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/network/NetworkServiceImpl.java 878d2a8 
>   utils/src/com/cloud/utils/net/NetUtils.java 5988dd5 
> 
> Diff: https://reviews.apache.org/r/10005/diff/
> 
> 
> Testing
> -------
> 
> CIDR : 10.0.144.0/20, Network CIDR : null, guestVmCidr : 10.0.151.0/20 => Reservation is not applied.
> CIDR : 10.0.144.0/21, Network CIDR : 10.0.144.0/20, guestVmCidr : 10.0.151.0/20 => Existing Reservation is not affected.
> 
> 
> Thanks,
> 
> Saksham Srivastava
> 
>


Re: Review Request: CLOUDSTACK-1647: IP Reservation should not happen if the guest-vm cidr and network cidr is not same but their start ip and end ip are same.

Posted by Sateesh Chodapuneedi <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10005/#review19396
-----------------------------------------------------------



server/src/com/cloud/network/NetworkServiceImpl.java
<https://reviews.apache.org/r/10005/#comment40106>

    This code seems dueplicated because the logic to calculate reserved ip range can be used to check if there exists some ip range to be reserved or not. If the calculated range is empty then do not attempt to reserve. Code to prepare response object already had this logic. May be we have to move this to NetUtils and call that from both places.


- Sateesh Chodapuneedi


On March 19, 2013, 9:07 a.m., Saksham Srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10005/
> -----------------------------------------------------------
> 
> (Updated March 19, 2013, 9:07 a.m.)
> 
> 
> Review request for cloudstack, Murali Reddy and Sateesh Chodapuneedi.
> 
> 
> Description
> -------
> 
> In cases where the start ip and end ip of guest vm cidr and network cidr are same, even when the cidrs appear to be different,the reservation procedure should not go through and user should get a message mentioning that.
> Added extra check for the same with proper alert message.
> 
> 
> This addresses bug CLOUDSTACK-1647.
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/network/NetworkServiceImpl.java 52e81e5 
> 
> Diff: https://reviews.apache.org/r/10005/diff/
> 
> 
> Testing
> -------
> 
> CIDR : 10.0.144.0/20, Network CIDR : null, guestVmCidr : 10.0.151.0/20 => Reservation is not applied.
> CIDR : 10.0.144.0/21, Network CIDR : 10.0.144.0/20, guestVmCidr : 10.0.151.0/20 => Existing Reservation is not affected.
> 
> 
> Thanks,
> 
> Saksham Srivastava
> 
>


Re: Review Request: CLOUDSTACK-1647: IP Reservation should not happen if the guest-vm cidr and network cidr is not same but their start ip and end ip are same.

Posted by Chip Childers <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10005/#review19453
-----------------------------------------------------------


These new methods are *prime* candidates for having unit tests.  Please provide some test that cover the stated manual test conditions in the patch before it's committed.

- Chip Childers


On April 19, 2013, 9:45 a.m., Saksham Srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10005/
> -----------------------------------------------------------
> 
> (Updated April 19, 2013, 9:45 a.m.)
> 
> 
> Review request for cloudstack, Murali Reddy and Sateesh Chodapuneedi.
> 
> 
> Description
> -------
> 
> In cases where the start ip and end ip of guest vm cidr and network cidr are same, even when the cidrs appear to be different,the reservation procedure should not go through and user should get a message mentioning that.
> Added extra check for the same with proper alert message.
> 
> 
> This addresses bug CLOUDSTACK-1647.
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/network/NetworkServiceImpl.java 878d2a8 
>   utils/src/com/cloud/utils/net/NetUtils.java 5988dd5 
> 
> Diff: https://reviews.apache.org/r/10005/diff/
> 
> 
> Testing
> -------
> 
> CIDR : 10.0.144.0/20, Network CIDR : null, guestVmCidr : 10.0.151.0/20 => Reservation is not applied.
> CIDR : 10.0.144.0/21, Network CIDR : 10.0.144.0/20, guestVmCidr : 10.0.151.0/20 => Existing Reservation is not affected.
> 
> 
> Thanks,
> 
> Saksham Srivastava
> 
>


Re: Review Request: CLOUDSTACK-1647: IP Reservation should not happen if the guest-vm cidr and network cidr is not same but their start ip and end ip are same.

Posted by Sateesh Chodapuneedi <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10005/#review21569
-----------------------------------------------------------

Ship it!


Ship It!

- Sateesh Chodapuneedi


On June 5, 2013, 11:39 a.m., Saksham Srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10005/
> -----------------------------------------------------------
> 
> (Updated June 5, 2013, 11:39 a.m.)
> 
> 
> Review request for cloudstack, Murali Reddy and Sateesh Chodapuneedi.
> 
> 
> Description
> -------
> 
> In cases where the start ip and end ip of guest vm cidr and network cidr are same, even when the cidrs appear to be different,the reservation procedure should not go through and user should get a message mentioning that.
> Added extra check for the same with proper alert message.
> 
> 
> This addresses bug CLOUDSTACK-1647.
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/network/NetworkServiceImpl.java 2bf9f40 
>   utils/src/com/cloud/utils/net/NetUtils.java 8c094c8 
>   utils/test/com/cloud/utils/net/NetUtilsTest.java 16d3402 
> 
> Diff: https://reviews.apache.org/r/10005/diff/
> 
> 
> Testing
> -------
> 
> CIDR : 10.0.144.0/20, Network CIDR : null, guestVmCidr : 10.0.151.0/20 => Reservation is not applied.
> CIDR : 10.0.144.0/21, Network CIDR : 10.0.144.0/20, guestVmCidr : 10.0.151.0/20 => Existing Reservation is not affected.
> Added UnitTest testIsSameIpRange()
> 
> 
> Thanks,
> 
> Saksham Srivastava
> 
>


Re: Review Request: CLOUDSTACK-1647: IP Reservation should not happen if the guest-vm cidr and network cidr is not same but their start ip and end ip are same.

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10005/#review21572
-----------------------------------------------------------


Commit 5dc7387d3b6b1abd841abc92e3c76a3894213d82 in branch refs/heads/master from Saksham Srivastava
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=5dc7387 ]

CLOUDSTACK-1647: IP Reservation should not happen if the guest-vm cidr and network cidr is not same but their start ip and end ip are same.

Signed-off-by: Sateesh Chodapuneedi <sa...@apache.org>


- ASF Subversion and Git Services


On June 5, 2013, 11:39 a.m., Saksham Srivastava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10005/
> -----------------------------------------------------------
> 
> (Updated June 5, 2013, 11:39 a.m.)
> 
> 
> Review request for cloudstack, Murali Reddy and Sateesh Chodapuneedi.
> 
> 
> Description
> -------
> 
> In cases where the start ip and end ip of guest vm cidr and network cidr are same, even when the cidrs appear to be different,the reservation procedure should not go through and user should get a message mentioning that.
> Added extra check for the same with proper alert message.
> 
> 
> This addresses bug CLOUDSTACK-1647.
> 
> 
> Diffs
> -----
> 
>   server/src/com/cloud/network/NetworkServiceImpl.java 2bf9f40 
>   utils/src/com/cloud/utils/net/NetUtils.java 8c094c8 
>   utils/test/com/cloud/utils/net/NetUtilsTest.java 16d3402 
> 
> Diff: https://reviews.apache.org/r/10005/diff/
> 
> 
> Testing
> -------
> 
> CIDR : 10.0.144.0/20, Network CIDR : null, guestVmCidr : 10.0.151.0/20 => Reservation is not applied.
> CIDR : 10.0.144.0/21, Network CIDR : 10.0.144.0/20, guestVmCidr : 10.0.151.0/20 => Existing Reservation is not affected.
> Added UnitTest testIsSameIpRange()
> 
> 
> Thanks,
> 
> Saksham Srivastava
> 
>


Re: Review Request: CLOUDSTACK-1647: IP Reservation should not happen if the guest-vm cidr and network cidr is not same but their start ip and end ip are same.

Posted by Saksham Srivastava <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10005/
-----------------------------------------------------------

(Updated June 5, 2013, 11:39 a.m.)


Review request for cloudstack, Murali Reddy and Sateesh Chodapuneedi.


Changes
-------

Resolving merge conflicts with latest master.


Description
-------

In cases where the start ip and end ip of guest vm cidr and network cidr are same, even when the cidrs appear to be different,the reservation procedure should not go through and user should get a message mentioning that.
Added extra check for the same with proper alert message.


This addresses bug CLOUDSTACK-1647.


Diffs (updated)
-----

  server/src/com/cloud/network/NetworkServiceImpl.java 2bf9f40 
  utils/src/com/cloud/utils/net/NetUtils.java 8c094c8 
  utils/test/com/cloud/utils/net/NetUtilsTest.java 16d3402 

Diff: https://reviews.apache.org/r/10005/diff/


Testing (updated)
-------

CIDR : 10.0.144.0/20, Network CIDR : null, guestVmCidr : 10.0.151.0/20 => Reservation is not applied.
CIDR : 10.0.144.0/21, Network CIDR : 10.0.144.0/20, guestVmCidr : 10.0.151.0/20 => Existing Reservation is not affected.
Added UnitTest testIsSameIpRange()


Thanks,

Saksham Srivastava


Re: Review Request: CLOUDSTACK-1647: IP Reservation should not happen if the guest-vm cidr and network cidr is not same but their start ip and end ip are same.

Posted by Saksham Srivastava <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10005/
-----------------------------------------------------------

(Updated April 23, 2013, 11:17 a.m.)


Review request for cloudstack, Murali Reddy and Sateesh Chodapuneedi.


Description
-------

In cases where the start ip and end ip of guest vm cidr and network cidr are same, even when the cidrs appear to be different,the reservation procedure should not go through and user should get a message mentioning that.
Added extra check for the same with proper alert message.


This addresses bug CLOUDSTACK-1647.


Diffs (updated)
-----

  server/src/com/cloud/network/NetworkServiceImpl.java ac2ac45 
  utils/src/com/cloud/utils/net/NetUtils.java 5988dd5 
  utils/test/com/cloud/utils/net/NetUtilsTest.java 28bd71f 

Diff: https://reviews.apache.org/r/10005/diff/


Testing
-------

CIDR : 10.0.144.0/20, Network CIDR : null, guestVmCidr : 10.0.151.0/20 => Reservation is not applied.
CIDR : 10.0.144.0/21, Network CIDR : 10.0.144.0/20, guestVmCidr : 10.0.151.0/20 => Existing Reservation is not affected.


Thanks,

Saksham Srivastava


Re: Review Request: CLOUDSTACK-1647: IP Reservation should not happen if the guest-vm cidr and network cidr is not same but their start ip and end ip are same.

Posted by Saksham Srivastava <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10005/
-----------------------------------------------------------

(Updated April 23, 2013, 10:31 a.m.)


Review request for cloudstack, Murali Reddy and Sateesh Chodapuneedi.


Changes
-------

Unittest added for new utility added in NetUtils.
Rebased to latest master.


Description
-------

In cases where the start ip and end ip of guest vm cidr and network cidr are same, even when the cidrs appear to be different,the reservation procedure should not go through and user should get a message mentioning that.
Added extra check for the same with proper alert message.


This addresses bug CLOUDSTACK-1647.


Diffs (updated)
-----

  server/src/com/cloud/network/NetworkServiceImpl.java ac2ac45 
  utils/src/com/cloud/utils/net/NetUtils.java 5988dd5 
  utils/test/com/cloud/utils/net/NetUtilsTest.java 28bd71f 

Diff: https://reviews.apache.org/r/10005/diff/


Testing
-------

CIDR : 10.0.144.0/20, Network CIDR : null, guestVmCidr : 10.0.151.0/20 => Reservation is not applied.
CIDR : 10.0.144.0/21, Network CIDR : 10.0.144.0/20, guestVmCidr : 10.0.151.0/20 => Existing Reservation is not affected.


Thanks,

Saksham Srivastava


Re: Review Request: CLOUDSTACK-1647: IP Reservation should not happen if the guest-vm cidr and network cidr is not same but their start ip and end ip are same.

Posted by Saksham Srivastava <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10005/
-----------------------------------------------------------

(Updated April 19, 2013, 9:45 a.m.)


Review request for cloudstack, Murali Reddy and Sateesh Chodapuneedi.


Changes
-------

Added new Utility in NetUtils to check if 2 CIDRs have same IP ranges.


Description
-------

In cases where the start ip and end ip of guest vm cidr and network cidr are same, even when the cidrs appear to be different,the reservation procedure should not go through and user should get a message mentioning that.
Added extra check for the same with proper alert message.


This addresses bug CLOUDSTACK-1647.


Diffs (updated)
-----

  server/src/com/cloud/network/NetworkServiceImpl.java 878d2a8 
  utils/src/com/cloud/utils/net/NetUtils.java 5988dd5 

Diff: https://reviews.apache.org/r/10005/diff/


Testing
-------

CIDR : 10.0.144.0/20, Network CIDR : null, guestVmCidr : 10.0.151.0/20 => Reservation is not applied.
CIDR : 10.0.144.0/21, Network CIDR : 10.0.144.0/20, guestVmCidr : 10.0.151.0/20 => Existing Reservation is not affected.


Thanks,

Saksham Srivastava


Re: Review Request: CLOUDSTACK-1647: IP Reservation should not happen if the guest-vm cidr and network cidr is not same but their start ip and end ip are same.

Posted by Saksham Srivastava <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10005/
-----------------------------------------------------------

(Updated March 19, 2013, 9:07 a.m.)


Review request for cloudstack, Murali Reddy and Sateesh Chodapuneedi.


Description
-------

In cases where the start ip and end ip of guest vm cidr and network cidr are same, even when the cidrs appear to be different,the reservation procedure should not go through and user should get a message mentioning that.
Added extra check for the same with proper alert message.


This addresses bug CLOUDSTACK-1647.


Diffs
-----

  server/src/com/cloud/network/NetworkServiceImpl.java 52e81e5 

Diff: https://reviews.apache.org/r/10005/diff/


Testing
-------

CIDR : 10.0.144.0/20, Network CIDR : null, guestVmCidr : 10.0.151.0/20 => Reservation is not applied.
CIDR : 10.0.144.0/21, Network CIDR : 10.0.144.0/20, guestVmCidr : 10.0.151.0/20 => Existing Reservation is not affected.


Thanks,

Saksham Srivastava