You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Alena Prokharchyk <Al...@citrix.com> on 2014/02/06 22:42:01 UTC
commit 6523c068695d0431070060667c222eb40d54b14d breaks network
removal
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) && network.getTrafficType() == TrafficType.Guest
With
+ DhcpServiceProvider dhcpServiceProvider = getDhcpServiceProvider(network);
When you try to call getDhcpServiceProvider(network), it throws an exception because DHCP service is not enabled in Public/Control networks of system vms nics. So system vm always fails to expunge.
Could you please fix it by checking if DHCP service is enabled on the network, before getting the DHCP service provider?
Thanks,
Alena.
RE: commit 6523c068695d0431070060667c222eb40d54b14d breaks network
removal
Posted by Animesh Chaturvedi <an...@citrix.com>.
> -----Original Message-----
> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
> Sent: Thursday, February 06, 2014 9:56 PM
> To: Alena Prokharchyk
> Cc: dev@cloudstack.apache.org; eizadi@infoblox.com
> Subject: Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks
> network removal
>
> 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.
>
[Animesh] Daan was this change introduced because of being flagged during findbug run?
> On Fri, Feb 7, 2014 at 6:49 AM, Daan Hoogland <da...@gmail.com>
> wrote:
> > will do Alena,
> >
> > thanks for the headsup
> >
> > On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
> > <Al...@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 causes the failure:
> >>
> >> - if (vm.getType() == Type.User &&
> >> isDhcpAccrossMultipleSubnetsSupported(network) &&
> >> isLastNicInSubnet(nic) &&
> >> network.getTrafficType() == TrafficType.Guest
> >>
> >> With
> >>
> >> + DhcpServiceProvider dhcpServiceProvider =
> >> getDhcpServiceProvider(network);
> >>
> >>
> >> When you try to call getDhcpServiceProvider(network), it throws an
> >> exception because DHCP service is not enabled in Public/Control
> >> networks of system vms nics. So system vm always fails to expunge.
> >>
> >> Could you please fix it by checking if DHCP service is enabled on the
> >> network, before getting the DHCP service provider?
> >>
> >> Thanks,
> >> Alena.
> >>
> >>
> >>
> >
> >
> >
> > --
> > Daan
>
>
>
> --
> Daan
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal
Posted by Daan Hoogland <da...@gmail.com>.
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 <da...@gmail.com> wrote:
> thanks Murali, will do
>
> On Fri, Feb 7, 2014 at 9:58 AM, murali reddy <mu...@gmail.com> wrote:
>> On Fri, Feb 7, 2014 at 11:34 AM, Daan Hoogland <da...@gmail.com>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) {
>>> // remove the dhcpservice ip if this is the last nic in subnet.
>>> DhcpServiceProvider dhcpServiceProvider =
>>> getDhcpServiceProvider(network);
>>> if (dhcpServiceProvider != null
>>> &&
>>> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>> && isLastNicInSubnet(nic)) {
>>> removeDhcpServiceInSubnet(nic);
>>> }
>>> }
>>>
>>>
>> Daan, it would still break if network does not have DHCP service enabled,
>> best would be to check
>> '_networkModel.areServicesSupportedInNetwork(network.getId(),
>> Service.Dhcp)' then get the provider.
>>
>>
>>> On Fri, Feb 7, 2014 at 6:55 AM, Daan Hoogland <da...@gmail.com>
>>> wrote:
>>> > 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 <da...@gmail.com>
>>> wrote:
>>> >> will do Alena,
>>> >>
>>> >> thanks for the headsup
>>> >>
>>> >> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
>>> >> <Al...@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
>>> >>> causes the failure:
>>> >>>
>>> >>> - if (vm.getType() == Type.User &&
>>> >>> isDhcpAccrossMultipleSubnetsSupported(network) &&
>>> isLastNicInSubnet(nic) &&
>>> >>> network.getTrafficType() == TrafficType.Guest
>>> >>>
>>> >>> With
>>> >>>
>>> >>> + DhcpServiceProvider dhcpServiceProvider =
>>> >>> getDhcpServiceProvider(network);
>>> >>>
>>> >>>
>>> >>> When you try to call getDhcpServiceProvider(network), it throws an
>>> exception
>>> >>> because DHCP service is not enabled in Public/Control networks of
>>> system vms
>>> >>> nics. So system vm always fails to expunge.
>>> >>>
>>> >>> Could you please fix it by checking if DHCP service is enabled on the
>>> >>> network, before getting the DHCP service provider?
>>> >>>
>>> >>> Thanks,
>>> >>> Alena.
>>> >>>
>>> >>>
>>> >>>
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >> Daan
>>> >
>>> >
>>> >
>>> > --
>>> > Daan
>>>
>>>
>>>
>>> --
>>> Daan
>>>
>
>
>
> --
> Daan
--
Daan
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal
Posted by Daan Hoogland <da...@gmail.com>.
thanks Murali, will do
On Fri, Feb 7, 2014 at 9:58 AM, murali reddy <mu...@gmail.com> wrote:
> On Fri, Feb 7, 2014 at 11:34 AM, Daan Hoogland <da...@gmail.com>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) {
>> // remove the dhcpservice ip if this is the last nic in subnet.
>> DhcpServiceProvider dhcpServiceProvider =
>> getDhcpServiceProvider(network);
>> if (dhcpServiceProvider != null
>> &&
>> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>> && isLastNicInSubnet(nic)) {
>> removeDhcpServiceInSubnet(nic);
>> }
>> }
>>
>>
> Daan, it would still break if network does not have DHCP service enabled,
> best would be to check
> '_networkModel.areServicesSupportedInNetwork(network.getId(),
> Service.Dhcp)' then get the provider.
>
>
>> On Fri, Feb 7, 2014 at 6:55 AM, Daan Hoogland <da...@gmail.com>
>> wrote:
>> > 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 <da...@gmail.com>
>> wrote:
>> >> will do Alena,
>> >>
>> >> thanks for the headsup
>> >>
>> >> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
>> >> <Al...@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
>> >>> causes the failure:
>> >>>
>> >>> - if (vm.getType() == Type.User &&
>> >>> isDhcpAccrossMultipleSubnetsSupported(network) &&
>> isLastNicInSubnet(nic) &&
>> >>> network.getTrafficType() == TrafficType.Guest
>> >>>
>> >>> With
>> >>>
>> >>> + DhcpServiceProvider dhcpServiceProvider =
>> >>> getDhcpServiceProvider(network);
>> >>>
>> >>>
>> >>> When you try to call getDhcpServiceProvider(network), it throws an
>> exception
>> >>> because DHCP service is not enabled in Public/Control networks of
>> system vms
>> >>> nics. So system vm always fails to expunge.
>> >>>
>> >>> Could you please fix it by checking if DHCP service is enabled on the
>> >>> network, before getting the DHCP service provider?
>> >>>
>> >>> Thanks,
>> >>> Alena.
>> >>>
>> >>>
>> >>>
>> >>
>> >>
>> >>
>> >> --
>> >> Daan
>> >
>> >
>> >
>> > --
>> > Daan
>>
>>
>>
>> --
>> Daan
>>
--
Daan
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal
Posted by murali reddy <mu...@gmail.com>.
On Fri, Feb 7, 2014 at 11:34 AM, Daan Hoogland <da...@gmail.com>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) {
> // remove the dhcpservice ip if this is the last nic in subnet.
> DhcpServiceProvider dhcpServiceProvider =
> getDhcpServiceProvider(network);
> if (dhcpServiceProvider != null
> &&
> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
> && isLastNicInSubnet(nic)) {
> removeDhcpServiceInSubnet(nic);
> }
> }
>
>
Daan, it would still break if network does not have DHCP service enabled,
best would be to check
'_networkModel.areServicesSupportedInNetwork(network.getId(),
Service.Dhcp)' then get the provider.
> On Fri, Feb 7, 2014 at 6:55 AM, Daan Hoogland <da...@gmail.com>
> wrote:
> > 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 <da...@gmail.com>
> wrote:
> >> will do Alena,
> >>
> >> thanks for the headsup
> >>
> >> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
> >> <Al...@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
> >>> causes the failure:
> >>>
> >>> - if (vm.getType() == Type.User &&
> >>> isDhcpAccrossMultipleSubnetsSupported(network) &&
> isLastNicInSubnet(nic) &&
> >>> network.getTrafficType() == TrafficType.Guest
> >>>
> >>> With
> >>>
> >>> + DhcpServiceProvider dhcpServiceProvider =
> >>> getDhcpServiceProvider(network);
> >>>
> >>>
> >>> When you try to call getDhcpServiceProvider(network), it throws an
> exception
> >>> because DHCP service is not enabled in Public/Control networks of
> system vms
> >>> nics. So system vm always fails to expunge.
> >>>
> >>> Could you please fix it by checking if DHCP service is enabled on the
> >>> network, before getting the DHCP service provider?
> >>>
> >>> Thanks,
> >>> Alena.
> >>>
> >>>
> >>>
> >>
> >>
> >>
> >> --
> >> Daan
> >
> >
> >
> > --
> > Daan
>
>
>
> --
> Daan
>
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal
Posted by Daan Hoogland <da...@gmail.com>.
ok,
I was wondering about side effects of the checks.
thanks,
On Mon, Feb 10, 2014 at 6:46 PM, Alena Prokharchyk
<Al...@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
> 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" <da...@gmail.com> wrote:
>
>>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. So what
>>is the importance of using your version? (just curious)
>>
>><mine>
>> if (vm.getType() == Type.User
>> &&
>>_networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp))
>>{
>> // remove the dhcpservice ip if this is the last nic in
>>subnet.
>> DhcpServiceProvider dhcpServiceProvider =
>>getDhcpServiceProvider(network);
>> if (dhcpServiceProvider != null
>> &&
>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>> && isLastNicInSubnet(nic)
>> && network.getTrafficType() == TrafficType.Guest
>> && network.getGuestType() == GuestType.Shared) {
>> removeDhcpServiceInSubnet(nic);
>> }
>> }
>></mine>
>>
>><yours>
>>if (vm.getType() == Type.User
>> && network.getTrafficType() == TrafficType.Guest
>> && isLastNicInSubnet(nic)
>> && network.getGuestType() == GuestType.Shared
>> &&
>>_networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp))
>>{
>> //2) Now get the DHCP provider, and do the rest of the checks
>> DhcpServiceProvider dhcpServiceProvider =
>>getDhcpServiceProvider(network);
>> if (dhcpServiceProvider != null
>> && isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)) {
>> removeDhcpServiceInSubnet(nic);
>> }
>>}
>></yours>
>>
>>On Fri, Feb 7, 2014 at 8:30 PM, Daan Hoogland <da...@gmail.com>
>>wrote:
>>> 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
>>> <Al...@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 is enabled on the network
>>>> if (vm.getType() == Type.User && network.getTrafficType() ==
>>>> TrafficType.Guest && isLastNicInSubnet(nic) && network.getGuestType()
>>>>==
>>>> GuestType.Shared &&
>>>>
>>>>_networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp
>>>>))
>>>> {
>>>>
>>>> //2) Now get the DHCP provider, and do the rest of the checks
>>>> DhcpServiceProvider dhcpServiceProvider =
>>>> getDhcpServiceProvider(network);
>>>> if (dhcpServiceProvider != null &&
>>>> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)) {
>>>> removeDhcpServiceInSubnet(nic);
>>>> }
>>>> }
>>>>
>>>>
>>>>
>>>> Could you please test it for 2 Shared networks - one with DHCP service,
>>>> and one w/o?
>>>>
>>>> Thank you!
>>>> Alena.
>>>>
>>>>
>>>>
>>>> On 2/7/14, 10:04 AM, "Daan Hoogland" <da...@gmail.com> wrote:
>>>>
>>>>>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() == Type.User
>>>>> &&
>>>>>_networkModel.areServicesSupportedInNetwork(network.getId(),
>>>>>Service.Dhcp)
>>>>> && network.getTrafficType() == TrafficType.Guest
>>>>> && network.getGuestType() == GuestType.Shared) {
>>>>> // remove the dhcpservice ip if this is the last nic in
>>>>>subnet.
>>>>> DhcpServiceProvider dhcpServiceProvider =
>>>>>getDhcpServiceProvider(network);
>>>>> if (dhcpServiceProvider != null
>>>>> &&
>>>>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>>>> && isLastNicInSubnet(nic)) {
>>>>> removeDhcpServiceInSubnet(nic);
>>>>> }
>>>>> }
>>>>>
>>>>>What do you sugest?
>>>>>
>>>>> if (vm.getType() == Type.User
>>>>> &&
>>>>>_networkModel.areServicesSupportedInNetwork(network.getId(),
>>>>>Service.Dhcp)) {
>>>>> // remove the dhcpservice ip if this is the last nic in
>>>>>subnet.
>>>>> DhcpServiceProvider dhcpServiceProvider =
>>>>>getDhcpServiceProvider(network);
>>>>> if (dhcpServiceProvider != null
>>>>> &&
>>>>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>>>> && isLastNicInSubnet(nic)
>>>>> && network.getTrafficType() == TrafficType.Guest
>>>>> && network.getGuestType() == GuestType.Shared) {
>>>>> removeDhcpServiceInSubnet(nic);
>>>>> }
>>>>> }
>>>>>
>>>>>???
>>>>>
>>>>>On Fri, Feb 7, 2014 at 6:56 PM, Alena Prokharchyk
>>>>><Al...@citrix.com> wrote:
>>>>>> 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.
>>>>>>
>>>>>> On 2/6/14, 10:04 PM, "Daan Hoogland" <da...@gmail.com> 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) {
>>>>>>> // remove the dhcpservice ip if this is the last nic in
>>>>>>>subnet.
>>>>>>> DhcpServiceProvider dhcpServiceProvider =
>>>>>>>getDhcpServiceProvider(network);
>>>>>>> if (dhcpServiceProvider != null
>>>>>>> &&
>>>>>>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>>>>>> && isLastNicInSubnet(nic)) {
>>>>>>> removeDhcpServiceInSubnet(nic);
>>>>>>> }
>>>>>>> }
>>>>>>>
>>>>>>>On Fri, Feb 7, 2014 at 6:55 AM, Daan Hoogland
>>>>>>><da...@gmail.com>
>>>>>>>wrote:
>>>>>>>> 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
>>>>>>>><da...@gmail.com>
>>>>>>>>wrote:
>>>>>>>>> will do Alena,
>>>>>>>>>
>>>>>>>>> thanks for the headsup
>>>>>>>>>
>>>>>>>>> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
>>>>>>>>> <Al...@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
>>>>>>>>>> causes the failure:
>>>>>>>>>>
>>>>>>>>>> - if (vm.getType() == Type.User &&
>>>>>>>>>> isDhcpAccrossMultipleSubnetsSupported(network) &&
>>>>>>>>>>isLastNicInSubnet(nic) &&
>>>>>>>>>> network.getTrafficType() == TrafficType.Guest
>>>>>>>>>>
>>>>>>>>>> With
>>>>>>>>>>
>>>>>>>>>> + DhcpServiceProvider dhcpServiceProvider =
>>>>>>>>>> getDhcpServiceProvider(network);
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> When you try to call getDhcpServiceProvider(network), it throws
>>>>>>>>>>an
>>>>>>>>>>exception
>>>>>>>>>> because DHCP service is not enabled in Public/Control networks of
>>>>>>>>>>system vms
>>>>>>>>>> nics. So system vm always fails to expunge.
>>>>>>>>>>
>>>>>>>>>> Could you please fix it by checking if DHCP service is enabled on
>>>>>>>>>>the
>>>>>>>>>> network, before getting the DHCP service provider?
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Alena.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> --
>>>>>>>>> Daan
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Daan
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>--
>>>>>>>Daan
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>>--
>>>>>Daan
>>>>
>>>
>>>
>>>
>>> --
>>> Daan
>>
>>
>>
>>--
>>Daan
>
--
Daan
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network
removal
Posted by Alena Prokharchyk <Al...@citrix.com>.
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" <da...@gmail.com> wrote:
>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. So what
>is the importance of using your version? (just curious)
>
><mine>
> if (vm.getType() == Type.User
> &&
>_networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp))
>{
> // remove the dhcpservice ip if this is the last nic in
>subnet.
> DhcpServiceProvider dhcpServiceProvider =
>getDhcpServiceProvider(network);
> if (dhcpServiceProvider != null
> &&
>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
> && isLastNicInSubnet(nic)
> && network.getTrafficType() == TrafficType.Guest
> && network.getGuestType() == GuestType.Shared) {
> removeDhcpServiceInSubnet(nic);
> }
> }
></mine>
>
><yours>
>if (vm.getType() == Type.User
> && network.getTrafficType() == TrafficType.Guest
> && isLastNicInSubnet(nic)
> && network.getGuestType() == GuestType.Shared
> &&
>_networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp))
>{
> //2) Now get the DHCP provider, and do the rest of the checks
> DhcpServiceProvider dhcpServiceProvider =
>getDhcpServiceProvider(network);
> if (dhcpServiceProvider != null
> && isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)) {
> removeDhcpServiceInSubnet(nic);
> }
>}
></yours>
>
>On Fri, Feb 7, 2014 at 8:30 PM, Daan Hoogland <da...@gmail.com>
>wrote:
>> 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
>> <Al...@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 is enabled on the network
>>> if (vm.getType() == Type.User && network.getTrafficType() ==
>>> TrafficType.Guest && isLastNicInSubnet(nic) && network.getGuestType()
>>>==
>>> GuestType.Shared &&
>>>
>>>_networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp
>>>))
>>> {
>>>
>>> //2) Now get the DHCP provider, and do the rest of the checks
>>> DhcpServiceProvider dhcpServiceProvider =
>>> getDhcpServiceProvider(network);
>>> if (dhcpServiceProvider != null &&
>>> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)) {
>>> removeDhcpServiceInSubnet(nic);
>>> }
>>> }
>>>
>>>
>>>
>>> Could you please test it for 2 Shared networks - one with DHCP service,
>>> and one w/o?
>>>
>>> Thank you!
>>> Alena.
>>>
>>>
>>>
>>> On 2/7/14, 10:04 AM, "Daan Hoogland" <da...@gmail.com> wrote:
>>>
>>>>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() == Type.User
>>>> &&
>>>>_networkModel.areServicesSupportedInNetwork(network.getId(),
>>>>Service.Dhcp)
>>>> && network.getTrafficType() == TrafficType.Guest
>>>> && network.getGuestType() == GuestType.Shared) {
>>>> // remove the dhcpservice ip if this is the last nic in
>>>>subnet.
>>>> DhcpServiceProvider dhcpServiceProvider =
>>>>getDhcpServiceProvider(network);
>>>> if (dhcpServiceProvider != null
>>>> &&
>>>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>>> && isLastNicInSubnet(nic)) {
>>>> removeDhcpServiceInSubnet(nic);
>>>> }
>>>> }
>>>>
>>>>What do you sugest?
>>>>
>>>> if (vm.getType() == Type.User
>>>> &&
>>>>_networkModel.areServicesSupportedInNetwork(network.getId(),
>>>>Service.Dhcp)) {
>>>> // remove the dhcpservice ip if this is the last nic in
>>>>subnet.
>>>> DhcpServiceProvider dhcpServiceProvider =
>>>>getDhcpServiceProvider(network);
>>>> if (dhcpServiceProvider != null
>>>> &&
>>>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>>> && isLastNicInSubnet(nic)
>>>> && network.getTrafficType() == TrafficType.Guest
>>>> && network.getGuestType() == GuestType.Shared) {
>>>> removeDhcpServiceInSubnet(nic);
>>>> }
>>>> }
>>>>
>>>>???
>>>>
>>>>On Fri, Feb 7, 2014 at 6:56 PM, Alena Prokharchyk
>>>><Al...@citrix.com> wrote:
>>>>> 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.
>>>>>
>>>>> On 2/6/14, 10:04 PM, "Daan Hoogland" <da...@gmail.com> 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) {
>>>>>> // remove the dhcpservice ip if this is the last nic in
>>>>>>subnet.
>>>>>> DhcpServiceProvider dhcpServiceProvider =
>>>>>>getDhcpServiceProvider(network);
>>>>>> if (dhcpServiceProvider != null
>>>>>> &&
>>>>>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>>>>> && isLastNicInSubnet(nic)) {
>>>>>> removeDhcpServiceInSubnet(nic);
>>>>>> }
>>>>>> }
>>>>>>
>>>>>>On Fri, Feb 7, 2014 at 6:55 AM, Daan Hoogland
>>>>>><da...@gmail.com>
>>>>>>wrote:
>>>>>>> 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
>>>>>>><da...@gmail.com>
>>>>>>>wrote:
>>>>>>>> will do Alena,
>>>>>>>>
>>>>>>>> thanks for the headsup
>>>>>>>>
>>>>>>>> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
>>>>>>>> <Al...@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
>>>>>>>>> causes the failure:
>>>>>>>>>
>>>>>>>>> - if (vm.getType() == Type.User &&
>>>>>>>>> isDhcpAccrossMultipleSubnetsSupported(network) &&
>>>>>>>>>isLastNicInSubnet(nic) &&
>>>>>>>>> network.getTrafficType() == TrafficType.Guest
>>>>>>>>>
>>>>>>>>> With
>>>>>>>>>
>>>>>>>>> + DhcpServiceProvider dhcpServiceProvider =
>>>>>>>>> getDhcpServiceProvider(network);
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> When you try to call getDhcpServiceProvider(network), it throws
>>>>>>>>>an
>>>>>>>>>exception
>>>>>>>>> because DHCP service is not enabled in Public/Control networks of
>>>>>>>>>system vms
>>>>>>>>> nics. So system vm always fails to expunge.
>>>>>>>>>
>>>>>>>>> Could you please fix it by checking if DHCP service is enabled on
>>>>>>>>>the
>>>>>>>>> network, before getting the DHCP service provider?
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Alena.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Daan
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Daan
>>>>>>
>>>>>>
>>>>>>
>>>>>>--
>>>>>>Daan
>>>>>
>>>>
>>>>
>>>>
>>>>--
>>>>Daan
>>>
>>
>>
>>
>> --
>> Daan
>
>
>
>--
>Daan
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal
Posted by Daan Hoogland <da...@gmail.com>.
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. So what
is the importance of using your version? (just curious)
<mine>
if (vm.getType() == Type.User
&&
_networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp))
{
// remove the dhcpservice ip if this is the last nic in subnet.
DhcpServiceProvider dhcpServiceProvider =
getDhcpServiceProvider(network);
if (dhcpServiceProvider != null
&&
isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
&& isLastNicInSubnet(nic)
&& network.getTrafficType() == TrafficType.Guest
&& network.getGuestType() == GuestType.Shared) {
removeDhcpServiceInSubnet(nic);
}
}
</mine>
<yours>
if (vm.getType() == Type.User
&& network.getTrafficType() == TrafficType.Guest
&& isLastNicInSubnet(nic)
&& network.getGuestType() == GuestType.Shared
&& _networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp))
{
//2) Now get the DHCP provider, and do the rest of the checks
DhcpServiceProvider dhcpServiceProvider = getDhcpServiceProvider(network);
if (dhcpServiceProvider != null
&& isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)) {
removeDhcpServiceInSubnet(nic);
}
}
</yours>
On Fri, Feb 7, 2014 at 8:30 PM, Daan Hoogland <da...@gmail.com> wrote:
> 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
> <Al...@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 is enabled on the network
>> if (vm.getType() == Type.User && network.getTrafficType() ==
>> TrafficType.Guest && isLastNicInSubnet(nic) && network.getGuestType() ==
>> GuestType.Shared &&
>> _networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp))
>> {
>>
>> //2) Now get the DHCP provider, and do the rest of the checks
>> DhcpServiceProvider dhcpServiceProvider =
>> getDhcpServiceProvider(network);
>> if (dhcpServiceProvider != null &&
>> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)) {
>> removeDhcpServiceInSubnet(nic);
>> }
>> }
>>
>>
>>
>> Could you please test it for 2 Shared networks - one with DHCP service,
>> and one w/o?
>>
>> Thank you!
>> Alena.
>>
>>
>>
>> On 2/7/14, 10:04 AM, "Daan Hoogland" <da...@gmail.com> wrote:
>>
>>>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() == Type.User
>>> &&
>>>_networkModel.areServicesSupportedInNetwork(network.getId(),
>>>Service.Dhcp)
>>> && network.getTrafficType() == TrafficType.Guest
>>> && network.getGuestType() == GuestType.Shared) {
>>> // remove the dhcpservice ip if this is the last nic in
>>>subnet.
>>> DhcpServiceProvider dhcpServiceProvider =
>>>getDhcpServiceProvider(network);
>>> if (dhcpServiceProvider != null
>>> &&
>>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>> && isLastNicInSubnet(nic)) {
>>> removeDhcpServiceInSubnet(nic);
>>> }
>>> }
>>>
>>>What do you sugest?
>>>
>>> if (vm.getType() == Type.User
>>> &&
>>>_networkModel.areServicesSupportedInNetwork(network.getId(),
>>>Service.Dhcp)) {
>>> // remove the dhcpservice ip if this is the last nic in
>>>subnet.
>>> DhcpServiceProvider dhcpServiceProvider =
>>>getDhcpServiceProvider(network);
>>> if (dhcpServiceProvider != null
>>> &&
>>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>> && isLastNicInSubnet(nic)
>>> && network.getTrafficType() == TrafficType.Guest
>>> && network.getGuestType() == GuestType.Shared) {
>>> removeDhcpServiceInSubnet(nic);
>>> }
>>> }
>>>
>>>???
>>>
>>>On Fri, Feb 7, 2014 at 6:56 PM, Alena Prokharchyk
>>><Al...@citrix.com> wrote:
>>>> 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.
>>>>
>>>> On 2/6/14, 10:04 PM, "Daan Hoogland" <da...@gmail.com> 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) {
>>>>> // remove the dhcpservice ip if this is the last nic in
>>>>>subnet.
>>>>> DhcpServiceProvider dhcpServiceProvider =
>>>>>getDhcpServiceProvider(network);
>>>>> if (dhcpServiceProvider != null
>>>>> &&
>>>>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>>>> && isLastNicInSubnet(nic)) {
>>>>> removeDhcpServiceInSubnet(nic);
>>>>> }
>>>>> }
>>>>>
>>>>>On Fri, Feb 7, 2014 at 6:55 AM, Daan Hoogland <da...@gmail.com>
>>>>>wrote:
>>>>>> 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
>>>>>><da...@gmail.com>
>>>>>>wrote:
>>>>>>> will do Alena,
>>>>>>>
>>>>>>> thanks for the headsup
>>>>>>>
>>>>>>> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
>>>>>>> <Al...@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
>>>>>>>> causes the failure:
>>>>>>>>
>>>>>>>> - if (vm.getType() == Type.User &&
>>>>>>>> isDhcpAccrossMultipleSubnetsSupported(network) &&
>>>>>>>>isLastNicInSubnet(nic) &&
>>>>>>>> network.getTrafficType() == TrafficType.Guest
>>>>>>>>
>>>>>>>> With
>>>>>>>>
>>>>>>>> + DhcpServiceProvider dhcpServiceProvider =
>>>>>>>> getDhcpServiceProvider(network);
>>>>>>>>
>>>>>>>>
>>>>>>>> When you try to call getDhcpServiceProvider(network), it throws an
>>>>>>>>exception
>>>>>>>> because DHCP service is not enabled in Public/Control networks of
>>>>>>>>system vms
>>>>>>>> nics. So system vm always fails to expunge.
>>>>>>>>
>>>>>>>> Could you please fix it by checking if DHCP service is enabled on
>>>>>>>>the
>>>>>>>> network, before getting the DHCP service provider?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Alena.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Daan
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Daan
>>>>>
>>>>>
>>>>>
>>>>>--
>>>>>Daan
>>>>
>>>
>>>
>>>
>>>--
>>>Daan
>>
>
>
>
> --
> Daan
--
Daan
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal
Posted by Daan Hoogland <da...@gmail.com>.
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. So what is the importance of using your version? (just curious)
<mine>
if (vm.getType() == Type.User
&& _networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp)) {
// remove the dhcpservice ip if this is the last nic in subnet.
DhcpServiceProvider dhcpServiceProvider = getDhcpServiceProvider(network);
if (dhcpServiceProvider != null
&& isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
&& isLastNicInSubnet(nic)
&& network.getTrafficType() == TrafficType.Guest
&& network.getGuestType() == GuestType.Shared) {
removeDhcpServiceInSubnet(nic);
}
}
</mine>
<yours>
if (vm.getType() == Type.User
&& network.getTrafficType() == TrafficType.Guest
&& isLastNicInSubnet(nic)
&& network.getGuestType() == GuestType.Shared
&& _networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp))
{
//2) Now get the DHCP provider, and do the rest of the checks
DhcpServiceProvider dhcpServiceProvider = getDhcpServiceProvider(network);
if (dhcpServiceProvider != null
&& isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)) {
removeDhcpServiceInSubnet(nic);
}
}
</yours>
On Fri, Feb 7, 2014 at 8:30 PM, Daan Hoogland <da...@gmail.com> wrote:
> 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
> <Al...@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 is enabled on the network
>> if (vm.getType() == Type.User && network.getTrafficType() ==
>> TrafficType.Guest && isLastNicInSubnet(nic) && network.getGuestType() ==
>> GuestType.Shared &&
>> _networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp))
>> {
>>
>> //2) Now get the DHCP provider, and do the rest of the checks
>> DhcpServiceProvider dhcpServiceProvider =
>> getDhcpServiceProvider(network);
>> if (dhcpServiceProvider != null &&
>> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)) {
>> removeDhcpServiceInSubnet(nic);
>> }
>> }
>>
>>
>>
>> Could you please test it for 2 Shared networks - one with DHCP service,
>> and one w/o?
>>
>> Thank you!
>> Alena.
>>
>>
>>
>> On 2/7/14, 10:04 AM, "Daan Hoogland" <da...@gmail.com> wrote:
>>
>>> 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() == Type.User
>>> &&
>>> _networkModel.areServicesSupportedInNetwork(network.getId(),
>>> Service.Dhcp)
>>> && network.getTrafficType() == TrafficType.Guest
>>> && network.getGuestType() == GuestType.Shared) {
>>> // remove the dhcpservice ip if this is the last nic in
>>> subnet.
>>> DhcpServiceProvider dhcpServiceProvider =
>>> getDhcpServiceProvider(network);
>>> if (dhcpServiceProvider != null
>>> &&
>>> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>> && isLastNicInSubnet(nic)) {
>>> removeDhcpServiceInSubnet(nic);
>>> }
>>> }
>>>
>>> What do you sugest?
>>>
>>> if (vm.getType() == Type.User
>>> &&
>>> _networkModel.areServicesSupportedInNetwork(network.getId(),
>>> Service.Dhcp)) {
>>> // remove the dhcpservice ip if this is the last nic in
>>> subnet.
>>> DhcpServiceProvider dhcpServiceProvider =
>>> getDhcpServiceProvider(network);
>>> if (dhcpServiceProvider != null
>>> &&
>>> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>> && isLastNicInSubnet(nic)
>>> && network.getTrafficType() == TrafficType.Guest
>>> && network.getGuestType() == GuestType.Shared) {
>>> removeDhcpServiceInSubnet(nic);
>>> }
>>> }
>>>
>>> ???
>>>
>>> On Fri, Feb 7, 2014 at 6:56 PM, Alena Prokharchyk
>>> <Al...@citrix.com> wrote:
>>>> 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.
>>>>
>>>> On 2/6/14, 10:04 PM, "Daan Hoogland" <da...@gmail.com> 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) {
>>>>> // remove the dhcpservice ip if this is the last nic in
>>>>> subnet.
>>>>> DhcpServiceProvider dhcpServiceProvider =
>>>>> getDhcpServiceProvider(network);
>>>>> if (dhcpServiceProvider != null
>>>>> &&
>>>>> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>>>> && isLastNicInSubnet(nic)) {
>>>>> removeDhcpServiceInSubnet(nic);
>>>>> }
>>>>> }
>>>>>
>>>>> On Fri, Feb 7, 2014 at 6:55 AM, Daan Hoogland <da...@gmail.com>
>>>>> wrote:
>>>>>> 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
>>>>>> <da...@gmail.com>
>>>>>> wrote:
>>>>>>> will do Alena,
>>>>>>>
>>>>>>> thanks for the headsup
>>>>>>>
>>>>>>> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
>>>>>>> <Al...@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
>>>>>>>> causes the failure:
>>>>>>>>
>>>>>>>> - if (vm.getType() == Type.User &&
>>>>>>>> isDhcpAccrossMultipleSubnetsSupported(network) &&
>>>>>>>> isLastNicInSubnet(nic) &&
>>>>>>>> network.getTrafficType() == TrafficType.Guest
>>>>>>>>
>>>>>>>> With
>>>>>>>>
>>>>>>>> + DhcpServiceProvider dhcpServiceProvider =
>>>>>>>> getDhcpServiceProvider(network);
>>>>>>>>
>>>>>>>>
>>>>>>>> When you try to call getDhcpServiceProvider(network), it throws an
>>>>>>>> exception
>>>>>>>> because DHCP service is not enabled in Public/Control networks of
>>>>>>>> system vms
>>>>>>>> nics. So system vm always fails to expunge.
>>>>>>>>
>>>>>>>> Could you please fix it by checking if DHCP service is enabled on
>>>>>>>> the
>>>>>>>> network, before getting the DHCP service provider?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Alena.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Daan
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Daan
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Daan
>>>>
>>>
>>>
>>>
>>> --
>>> Daan
>>
>
>
>
> --
> Daan
--
Daan
On Fri, Feb 7, 2014 at 8:30 PM, Daan Hoogland <da...@gmail.com> wrote:
> 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
> <Al...@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 is enabled on the network
>> if (vm.getType() == Type.User && network.getTrafficType() ==
>> TrafficType.Guest && isLastNicInSubnet(nic) && network.getGuestType() ==
>> GuestType.Shared &&
>> _networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp))
>> {
>>
>> //2) Now get the DHCP provider, and do the rest of the checks
>> DhcpServiceProvider dhcpServiceProvider =
>> getDhcpServiceProvider(network);
>> if (dhcpServiceProvider != null &&
>> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)) {
>> removeDhcpServiceInSubnet(nic);
>> }
>> }
>>
>>
>>
>> Could you please test it for 2 Shared networks - one with DHCP service,
>> and one w/o?
>>
>> Thank you!
>> Alena.
>>
>>
>>
>> On 2/7/14, 10:04 AM, "Daan Hoogland" <da...@gmail.com> wrote:
>>
>>> 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() == Type.User
>>> &&
>>> _networkModel.areServicesSupportedInNetwork(network.getId(),
>>> Service.Dhcp)
>>> && network.getTrafficType() == TrafficType.Guest
>>> && network.getGuestType() == GuestType.Shared) {
>>> // remove the dhcpservice ip if this is the last nic in
>>> subnet.
>>> DhcpServiceProvider dhcpServiceProvider =
>>> getDhcpServiceProvider(network);
>>> if (dhcpServiceProvider != null
>>> &&
>>> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>> && isLastNicInSubnet(nic)) {
>>> removeDhcpServiceInSubnet(nic);
>>> }
>>> }
>>>
>>> What do you sugest?
>>>
>>> if (vm.getType() == Type.User
>>> &&
>>> _networkModel.areServicesSupportedInNetwork(network.getId(),
>>> Service.Dhcp)) {
>>> // remove the dhcpservice ip if this is the last nic in
>>> subnet.
>>> DhcpServiceProvider dhcpServiceProvider =
>>> getDhcpServiceProvider(network);
>>> if (dhcpServiceProvider != null
>>> &&
>>> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>> && isLastNicInSubnet(nic)
>>> && network.getTrafficType() == TrafficType.Guest
>>> && network.getGuestType() == GuestType.Shared) {
>>> removeDhcpServiceInSubnet(nic);
>>> }
>>> }
>>>
>>> ???
>>>
>>> On Fri, Feb 7, 2014 at 6:56 PM, Alena Prokharchyk
>>> <Al...@citrix.com> wrote:
>>>> 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.
>>>>
>>>> On 2/6/14, 10:04 PM, "Daan Hoogland" <da...@gmail.com> 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) {
>>>>> // remove the dhcpservice ip if this is the last nic in
>>>>> subnet.
>>>>> DhcpServiceProvider dhcpServiceProvider =
>>>>> getDhcpServiceProvider(network);
>>>>> if (dhcpServiceProvider != null
>>>>> &&
>>>>> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>>>> && isLastNicInSubnet(nic)) {
>>>>> removeDhcpServiceInSubnet(nic);
>>>>> }
>>>>> }
>>>>>
>>>>> On Fri, Feb 7, 2014 at 6:55 AM, Daan Hoogland <da...@gmail.com>
>>>>> wrote:
>>>>>> 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
>>>>>> <da...@gmail.com>
>>>>>> wrote:
>>>>>>> will do Alena,
>>>>>>>
>>>>>>> thanks for the headsup
>>>>>>>
>>>>>>> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
>>>>>>> <Al...@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
>>>>>>>> causes the failure:
>>>>>>>>
>>>>>>>> - if (vm.getType() == Type.User &&
>>>>>>>> isDhcpAccrossMultipleSubnetsSupported(network) &&
>>>>>>>> isLastNicInSubnet(nic) &&
>>>>>>>> network.getTrafficType() == TrafficType.Guest
>>>>>>>>
>>>>>>>> With
>>>>>>>>
>>>>>>>> + DhcpServiceProvider dhcpServiceProvider =
>>>>>>>> getDhcpServiceProvider(network);
>>>>>>>>
>>>>>>>>
>>>>>>>> When you try to call getDhcpServiceProvider(network), it throws an
>>>>>>>> exception
>>>>>>>> because DHCP service is not enabled in Public/Control networks of
>>>>>>>> system vms
>>>>>>>> nics. So system vm always fails to expunge.
>>>>>>>>
>>>>>>>> Could you please fix it by checking if DHCP service is enabled on
>>>>>>>> the
>>>>>>>> network, before getting the DHCP service provider?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Alena.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Daan
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Daan
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Daan
>>>>
>>>
>>>
>>>
>>> --
>>> Daan
>>
>
>
>
> --
> Daan
--
Daan
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal
Posted by Daan Hoogland <da...@gmail.com>.
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
<Al...@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 is enabled on the network
> if (vm.getType() == Type.User && network.getTrafficType() ==
> TrafficType.Guest && isLastNicInSubnet(nic) && network.getGuestType() ==
> GuestType.Shared &&
> _networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp))
> {
>
> //2) Now get the DHCP provider, and do the rest of the checks
> DhcpServiceProvider dhcpServiceProvider =
> getDhcpServiceProvider(network);
> if (dhcpServiceProvider != null &&
> isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)) {
> removeDhcpServiceInSubnet(nic);
> }
> }
>
>
>
> Could you please test it for 2 Shared networks - one with DHCP service,
> and one w/o?
>
> Thank you!
> Alena.
>
>
>
> On 2/7/14, 10:04 AM, "Daan Hoogland" <da...@gmail.com> wrote:
>
>>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() == Type.User
>> &&
>>_networkModel.areServicesSupportedInNetwork(network.getId(),
>>Service.Dhcp)
>> && network.getTrafficType() == TrafficType.Guest
>> && network.getGuestType() == GuestType.Shared) {
>> // remove the dhcpservice ip if this is the last nic in
>>subnet.
>> DhcpServiceProvider dhcpServiceProvider =
>>getDhcpServiceProvider(network);
>> if (dhcpServiceProvider != null
>> &&
>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>> && isLastNicInSubnet(nic)) {
>> removeDhcpServiceInSubnet(nic);
>> }
>> }
>>
>>What do you sugest?
>>
>> if (vm.getType() == Type.User
>> &&
>>_networkModel.areServicesSupportedInNetwork(network.getId(),
>>Service.Dhcp)) {
>> // remove the dhcpservice ip if this is the last nic in
>>subnet.
>> DhcpServiceProvider dhcpServiceProvider =
>>getDhcpServiceProvider(network);
>> if (dhcpServiceProvider != null
>> &&
>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>> && isLastNicInSubnet(nic)
>> && network.getTrafficType() == TrafficType.Guest
>> && network.getGuestType() == GuestType.Shared) {
>> removeDhcpServiceInSubnet(nic);
>> }
>> }
>>
>>???
>>
>>On Fri, Feb 7, 2014 at 6:56 PM, Alena Prokharchyk
>><Al...@citrix.com> wrote:
>>> 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.
>>>
>>> On 2/6/14, 10:04 PM, "Daan Hoogland" <da...@gmail.com> 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) {
>>>> // remove the dhcpservice ip if this is the last nic in
>>>>subnet.
>>>> DhcpServiceProvider dhcpServiceProvider =
>>>>getDhcpServiceProvider(network);
>>>> if (dhcpServiceProvider != null
>>>> &&
>>>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>>> && isLastNicInSubnet(nic)) {
>>>> removeDhcpServiceInSubnet(nic);
>>>> }
>>>> }
>>>>
>>>>On Fri, Feb 7, 2014 at 6:55 AM, Daan Hoogland <da...@gmail.com>
>>>>wrote:
>>>>> 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
>>>>><da...@gmail.com>
>>>>>wrote:
>>>>>> will do Alena,
>>>>>>
>>>>>> thanks for the headsup
>>>>>>
>>>>>> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
>>>>>> <Al...@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
>>>>>>> causes the failure:
>>>>>>>
>>>>>>> - if (vm.getType() == Type.User &&
>>>>>>> isDhcpAccrossMultipleSubnetsSupported(network) &&
>>>>>>>isLastNicInSubnet(nic) &&
>>>>>>> network.getTrafficType() == TrafficType.Guest
>>>>>>>
>>>>>>> With
>>>>>>>
>>>>>>> + DhcpServiceProvider dhcpServiceProvider =
>>>>>>> getDhcpServiceProvider(network);
>>>>>>>
>>>>>>>
>>>>>>> When you try to call getDhcpServiceProvider(network), it throws an
>>>>>>>exception
>>>>>>> because DHCP service is not enabled in Public/Control networks of
>>>>>>>system vms
>>>>>>> nics. So system vm always fails to expunge.
>>>>>>>
>>>>>>> Could you please fix it by checking if DHCP service is enabled on
>>>>>>>the
>>>>>>> network, before getting the DHCP service provider?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Alena.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Daan
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Daan
>>>>
>>>>
>>>>
>>>>--
>>>>Daan
>>>
>>
>>
>>
>>--
>>Daan
>
--
Daan
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network
removal
Posted by Alena Prokharchyk <Al...@citrix.com>.
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 &&
_networkModel.areServicesSupportedInNetwork(network.getId(),Service.Dhcp))
{
//2) Now get the DHCP provider, and do the rest of the checks
DhcpServiceProvider dhcpServiceProvider =
getDhcpServiceProvider(network);
if (dhcpServiceProvider != null &&
isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)) {
removeDhcpServiceInSubnet(nic);
}
}
Could you please test it for 2 Shared networks - one with DHCP service,
and one w/o?
Thank you!
Alena.
On 2/7/14, 10:04 AM, "Daan Hoogland" <da...@gmail.com> wrote:
>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() == Type.User
> &&
>_networkModel.areServicesSupportedInNetwork(network.getId(),
>Service.Dhcp)
> && network.getTrafficType() == TrafficType.Guest
> && network.getGuestType() == GuestType.Shared) {
> // remove the dhcpservice ip if this is the last nic in
>subnet.
> DhcpServiceProvider dhcpServiceProvider =
>getDhcpServiceProvider(network);
> if (dhcpServiceProvider != null
> &&
>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
> && isLastNicInSubnet(nic)) {
> removeDhcpServiceInSubnet(nic);
> }
> }
>
>What do you sugest?
>
> if (vm.getType() == Type.User
> &&
>_networkModel.areServicesSupportedInNetwork(network.getId(),
>Service.Dhcp)) {
> // remove the dhcpservice ip if this is the last nic in
>subnet.
> DhcpServiceProvider dhcpServiceProvider =
>getDhcpServiceProvider(network);
> if (dhcpServiceProvider != null
> &&
>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
> && isLastNicInSubnet(nic)
> && network.getTrafficType() == TrafficType.Guest
> && network.getGuestType() == GuestType.Shared) {
> removeDhcpServiceInSubnet(nic);
> }
> }
>
>???
>
>On Fri, Feb 7, 2014 at 6:56 PM, Alena Prokharchyk
><Al...@citrix.com> wrote:
>> 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.
>>
>> On 2/6/14, 10:04 PM, "Daan Hoogland" <da...@gmail.com> 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) {
>>> // remove the dhcpservice ip if this is the last nic in
>>>subnet.
>>> DhcpServiceProvider dhcpServiceProvider =
>>>getDhcpServiceProvider(network);
>>> if (dhcpServiceProvider != null
>>> &&
>>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>>> && isLastNicInSubnet(nic)) {
>>> removeDhcpServiceInSubnet(nic);
>>> }
>>> }
>>>
>>>On Fri, Feb 7, 2014 at 6:55 AM, Daan Hoogland <da...@gmail.com>
>>>wrote:
>>>> 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
>>>><da...@gmail.com>
>>>>wrote:
>>>>> will do Alena,
>>>>>
>>>>> thanks for the headsup
>>>>>
>>>>> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
>>>>> <Al...@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
>>>>>> causes the failure:
>>>>>>
>>>>>> - if (vm.getType() == Type.User &&
>>>>>> isDhcpAccrossMultipleSubnetsSupported(network) &&
>>>>>>isLastNicInSubnet(nic) &&
>>>>>> network.getTrafficType() == TrafficType.Guest
>>>>>>
>>>>>> With
>>>>>>
>>>>>> + DhcpServiceProvider dhcpServiceProvider =
>>>>>> getDhcpServiceProvider(network);
>>>>>>
>>>>>>
>>>>>> When you try to call getDhcpServiceProvider(network), it throws an
>>>>>>exception
>>>>>> because DHCP service is not enabled in Public/Control networks of
>>>>>>system vms
>>>>>> nics. So system vm always fails to expunge.
>>>>>>
>>>>>> Could you please fix it by checking if DHCP service is enabled on
>>>>>>the
>>>>>> network, before getting the DHCP service provider?
>>>>>>
>>>>>> Thanks,
>>>>>> Alena.
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Daan
>>>>
>>>>
>>>>
>>>> --
>>>> Daan
>>>
>>>
>>>
>>>--
>>>Daan
>>
>
>
>
>--
>Daan
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal
Posted by Daan Hoogland <da...@gmail.com>.
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() == Type.User
&&
_networkModel.areServicesSupportedInNetwork(network.getId(),
Service.Dhcp)
&& network.getTrafficType() == TrafficType.Guest
&& network.getGuestType() == GuestType.Shared) {
// remove the dhcpservice ip if this is the last nic in subnet.
DhcpServiceProvider dhcpServiceProvider =
getDhcpServiceProvider(network);
if (dhcpServiceProvider != null
&&
isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
&& isLastNicInSubnet(nic)) {
removeDhcpServiceInSubnet(nic);
}
}
What do you sugest?
if (vm.getType() == Type.User
&&
_networkModel.areServicesSupportedInNetwork(network.getId(),
Service.Dhcp)) {
// remove the dhcpservice ip if this is the last nic in subnet.
DhcpServiceProvider dhcpServiceProvider =
getDhcpServiceProvider(network);
if (dhcpServiceProvider != null
&&
isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
&& isLastNicInSubnet(nic)
&& network.getTrafficType() == TrafficType.Guest
&& network.getGuestType() == GuestType.Shared) {
removeDhcpServiceInSubnet(nic);
}
}
???
On Fri, Feb 7, 2014 at 6:56 PM, Alena Prokharchyk
<Al...@citrix.com> wrote:
> 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.
>
> On 2/6/14, 10:04 PM, "Daan Hoogland" <da...@gmail.com> 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) {
>> // remove the dhcpservice ip if this is the last nic in
>>subnet.
>> DhcpServiceProvider dhcpServiceProvider =
>>getDhcpServiceProvider(network);
>> if (dhcpServiceProvider != null
>> &&
>>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
>> && isLastNicInSubnet(nic)) {
>> removeDhcpServiceInSubnet(nic);
>> }
>> }
>>
>>On Fri, Feb 7, 2014 at 6:55 AM, Daan Hoogland <da...@gmail.com>
>>wrote:
>>> 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 <da...@gmail.com>
>>>wrote:
>>>> will do Alena,
>>>>
>>>> thanks for the headsup
>>>>
>>>> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
>>>> <Al...@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
>>>>> causes the failure:
>>>>>
>>>>> - if (vm.getType() == Type.User &&
>>>>> isDhcpAccrossMultipleSubnetsSupported(network) &&
>>>>>isLastNicInSubnet(nic) &&
>>>>> network.getTrafficType() == TrafficType.Guest
>>>>>
>>>>> With
>>>>>
>>>>> + DhcpServiceProvider dhcpServiceProvider =
>>>>> getDhcpServiceProvider(network);
>>>>>
>>>>>
>>>>> When you try to call getDhcpServiceProvider(network), it throws an
>>>>>exception
>>>>> because DHCP service is not enabled in Public/Control networks of
>>>>>system vms
>>>>> nics. So system vm always fails to expunge.
>>>>>
>>>>> Could you please fix it by checking if DHCP service is enabled on the
>>>>> network, before getting the DHCP service provider?
>>>>>
>>>>> Thanks,
>>>>> Alena.
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Daan
>>>
>>>
>>>
>>> --
>>> Daan
>>
>>
>>
>>--
>>Daan
>
--
Daan
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network
removal
Posted by Alena Prokharchyk <Al...@citrix.com>.
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.
On 2/6/14, 10:04 PM, "Daan Hoogland" <da...@gmail.com> 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) {
> // remove the dhcpservice ip if this is the last nic in
>subnet.
> DhcpServiceProvider dhcpServiceProvider =
>getDhcpServiceProvider(network);
> if (dhcpServiceProvider != null
> &&
>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
> && isLastNicInSubnet(nic)) {
> removeDhcpServiceInSubnet(nic);
> }
> }
>
>On Fri, Feb 7, 2014 at 6:55 AM, Daan Hoogland <da...@gmail.com>
>wrote:
>> 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 <da...@gmail.com>
>>wrote:
>>> will do Alena,
>>>
>>> thanks for the headsup
>>>
>>> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
>>> <Al...@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
>>>> causes the failure:
>>>>
>>>> - if (vm.getType() == Type.User &&
>>>> isDhcpAccrossMultipleSubnetsSupported(network) &&
>>>>isLastNicInSubnet(nic) &&
>>>> network.getTrafficType() == TrafficType.Guest
>>>>
>>>> With
>>>>
>>>> + DhcpServiceProvider dhcpServiceProvider =
>>>> getDhcpServiceProvider(network);
>>>>
>>>>
>>>> When you try to call getDhcpServiceProvider(network), it throws an
>>>>exception
>>>> because DHCP service is not enabled in Public/Control networks of
>>>>system vms
>>>> nics. So system vm always fails to expunge.
>>>>
>>>> Could you please fix it by checking if DHCP service is enabled on the
>>>> network, before getting the DHCP service provider?
>>>>
>>>> Thanks,
>>>> Alena.
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> Daan
>>
>>
>>
>> --
>> Daan
>
>
>
>--
>Daan
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal
Posted by Daan Hoogland <da...@gmail.com>.
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 subnet.
DhcpServiceProvider dhcpServiceProvider =
getDhcpServiceProvider(network);
if (dhcpServiceProvider != null
&&
isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider)
&& isLastNicInSubnet(nic)) {
removeDhcpServiceInSubnet(nic);
}
}
On Fri, Feb 7, 2014 at 6:55 AM, Daan Hoogland <da...@gmail.com> wrote:
> 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 <da...@gmail.com> wrote:
>> will do Alena,
>>
>> thanks for the headsup
>>
>> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
>> <Al...@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
>>> causes the failure:
>>>
>>> - if (vm.getType() == Type.User &&
>>> isDhcpAccrossMultipleSubnetsSupported(network) && isLastNicInSubnet(nic) &&
>>> network.getTrafficType() == TrafficType.Guest
>>>
>>> With
>>>
>>> + DhcpServiceProvider dhcpServiceProvider =
>>> getDhcpServiceProvider(network);
>>>
>>>
>>> When you try to call getDhcpServiceProvider(network), it throws an exception
>>> because DHCP service is not enabled in Public/Control networks of system vms
>>> nics. So system vm always fails to expunge.
>>>
>>> Could you please fix it by checking if DHCP service is enabled on the
>>> network, before getting the DHCP service provider?
>>>
>>> Thanks,
>>> Alena.
>>>
>>>
>>>
>>
>>
>>
>> --
>> Daan
>
>
>
> --
> Daan
--
Daan
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal
Posted by Daan Hoogland <da...@gmail.com>.
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 <da...@gmail.com> wrote:
> will do Alena,
>
> thanks for the headsup
>
> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
> <Al...@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
>> causes the failure:
>>
>> - if (vm.getType() == Type.User &&
>> isDhcpAccrossMultipleSubnetsSupported(network) && isLastNicInSubnet(nic) &&
>> network.getTrafficType() == TrafficType.Guest
>>
>> With
>>
>> + DhcpServiceProvider dhcpServiceProvider =
>> getDhcpServiceProvider(network);
>>
>>
>> When you try to call getDhcpServiceProvider(network), it throws an exception
>> because DHCP service is not enabled in Public/Control networks of system vms
>> nics. So system vm always fails to expunge.
>>
>> Could you please fix it by checking if DHCP service is enabled on the
>> network, before getting the DHCP service provider?
>>
>> Thanks,
>> Alena.
>>
>>
>>
>
>
>
> --
> Daan
--
Daan
Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal
Posted by Daan Hoogland <da...@gmail.com>.
will do Alena,
thanks for the headsup
On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
<Al...@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
> causes the failure:
>
> - if (vm.getType() == Type.User &&
> isDhcpAccrossMultipleSubnetsSupported(network) && isLastNicInSubnet(nic) &&
> network.getTrafficType() == TrafficType.Guest
>
> With
>
> + DhcpServiceProvider dhcpServiceProvider =
> getDhcpServiceProvider(network);
>
>
> When you try to call getDhcpServiceProvider(network), it throws an exception
> because DHCP service is not enabled in Public/Control networks of system vms
> nics. So system vm always fails to expunge.
>
> Could you please fix it by checking if DHCP service is enabled on the
> network, before getting the DHCP service provider?
>
> Thanks,
> Alena.
>
>
>
--
Daan