cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daan Hoogland <daan.hoogl...@gmail.com>
Subject Re: commit 6523c068695d0431070060667c222eb40d54b14d breaks network removal
Date Fri, 07 Feb 2014 19:30:00 GMT
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.Prokharchyk@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" <daan.hoogland@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
>><Alena.Prokharchyk@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" <daan.hoogland@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 <daan.hoogland@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
>>>>><daan.hoogland@gmail.com>
>>>>>wrote:
>>>>>> will do Alena,
>>>>>>
>>>>>> thanks for the headsup
>>>>>>
>>>>>> On Thu, Feb 6, 2014 at 10:42 PM, Alena Prokharchyk
>>>>>> <Alena.Prokharchyk@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

Mime
View raw message