Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-10 Thread Daan Hoogland
ok, I was wondering about side effects of the checks. thanks, On Mon, Feb 10, 2014 at 6:46 PM, Alena Prokharchyk alena.prokharc...@citrix.com wrote: No big difference. But its better to evaluate if the network is eligible for the check, before retrieving DHCP provider. IN your code snippet you

Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-08 Thread Daan Hoogland
Alena, I added unit tests and made a review request for you, https://reviews.apache.org/r/17872/ I have a question though. I can see some small the semantic differnce between the following two snippits but is only in the evaluation order of the conditions under which to execute, not in the

RE: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-07 Thread Animesh Chaturvedi
-Original Message- From: Daan Hoogland [mailto:daan.hoogl...@gmail.com] Sent: Thursday, February 06, 2014 9:56 PM To: Alena Prokharchyk Cc: dev@cloudstack.apache.org; eiz...@infoblox.com Subject: Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal second

Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-07 Thread murali reddy
On Fri, Feb 7, 2014 at 11:34 AM, Daan Hoogland daan.hoogl...@gmail.comwrote: Alena, The revert didn't apply. Would the folowing do the trick? if (vm.getType() == Type.User network.getTrafficType() == TrafficType.Guest network.getGuestType() ==

Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-07 Thread Daan Hoogland
thanks Murali, will do On Fri, Feb 7, 2014 at 9:58 AM, murali reddy muralimmre...@gmail.com wrote: On Fri, Feb 7, 2014 at 11:34 AM, Daan Hoogland daan.hoogl...@gmail.comwrote: Alena, The revert didn't apply. Would the folowing do the trick? if (vm.getType() == Type.User

Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-07 Thread Daan Hoogland
Hope this is not blocking at the moment so I can take the time to add a unit test. If not I have the code ready to ship. please bug me. On Fri, Feb 7, 2014 at 11:32 AM, Daan Hoogland daan.hoogl...@gmail.com wrote: thanks Murali, will do On Fri, Feb 7, 2014 at 9:58 AM, murali reddy

Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-07 Thread Alena Prokharchyk
Daan, 1) What is the reason you execute this code snippet just for Shared networks? 2) As I suggested in my prev email, before retrieving Dhcpprovider, you should check if dhcp service is enabled on the network. Use method areServicesSupportedInNetwork From NetworkModel to check that. -Alena.

Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-07 Thread Daan Hoogland
H Alena, I am just trying to fix an old contribution that I applied as it seemed not to harm in a basic test. revert didn't work so I am looking for a quick remedy. The original patch does it for shared only. I don't care either way. Lets do the best thing. the code now if (vm.getType()

Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-07 Thread Alena Prokharchyk
Daan, Here is how it should look: //1) Make all the checks that used to exist in original code + if DHCP service is enabled on the network if (vm.getType() == Type.User network.getTrafficType() == TrafficType.Guest isLastNicInSubnet(nic) network.getGuestType() == GuestType.Shared

Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-07 Thread Daan Hoogland
sure, will try to find a spot asap. and write unit tests to simulate those two situations. On Fri, Feb 7, 2014 at 7:20 PM, Alena Prokharchyk alena.prokharc...@citrix.com wrote: Daan, Here is how it should look: //1) Make all the checks that used to exist in original code + if DHCP service

commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-06 Thread Alena Prokharchyk
Soheil/Daan, The commit in the subject breaks network System vms destroy (VR, SSVM, CPVM), resulting in the network removal failures. Following line replacement causes the failure: - if (vm.getType() == Type.User isDhcpAccrossMultipleSubnetsSupported(network) isLastNicInSubnet(nic)

Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-06 Thread Daan Hoogland
will do Alena, thanks for the headsup On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk alena.prokharc...@citrix.com wrote: Soheil/Daan, The commit in the subject breaks network System vms destroy (VR, SSVM, CPVM), resulting in the network removal failures. Following line replacement

Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-06 Thread Daan Hoogland
second thought, Soheils mail bounces and the commit does not refer a ticket from jira. I am going to revert. I should have been more vigilant. sorry. On Fri, Feb 7, 2014 at 6:49 AM, Daan Hoogland daan.hoogl...@gmail.com wrote: will do Alena, thanks for the headsup On Thu, Feb 6, 2014 at

Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-06 Thread Daan Hoogland
Alena, The revert didn't apply. Would the folowing do the trick? if (vm.getType() == Type.User network.getTrafficType() == TrafficType.Guest network.getGuestType() == GuestType.Shared) { // remove the dhcpservice ip if this is the last nic in