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 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 > might retrieve DHCP provi

Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-10 Thread Alena Prokharchyk
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 might retrieve DHCP provider only to discover later that the check is not needed at all. -Alena. On 2/8/14, 7:07 AM, "Daan Hoogland" wrote: >Alena, >

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 logic.

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 log

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 wrote: > 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 networ

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 && _ne

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, 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
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 wrote: > thanks Murali, will do > > On Fri, Feb 7, 2014 at 9:58 AM, murali reddy wrote: >> On Fri, Feb 7, 2014 at

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 wrote: > On Fri, Feb 7, 2014 at 11:34 AM, Daan Hoogland wrote: > >> Alena, >> >> The revert didn't apply. Would the folowing do the trick? >> >> if (vm.getType() == Type.User >> && network.getTrafficType()

Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal

2014-02-07 Thread murali reddy
On Fri, Feb 7, 2014 at 11:34 AM, Daan Hoogland wrote: > 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)

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

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

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 wrote: > will do Alena, > > thanks for the headsup > > On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokha

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 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 > causes the failure: > > - if

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) &&