I will do that. -Soheil
________________________________________
From: Chiradeep Vittal [chiradeep.vit...@citrix.com]
Sent: Friday, September 06, 2013 3:07 PM
To: dev@cloudstack.apache.org
Subject: Re: NetworkManager Fix Master branch
Thanks Soheil, can I trouble you to post a review request
On 9/6/13 2:54 PM, "Soheil Eizadi" <seiz...@infoblox.com> wrote:
>I ran the master branch with my NetworkElement integrated which provides
>DHCP service. In my use case I was able to create a VM but when I deleted
>it there was an exception in the NetworkManager when it called:
>
>isDhcpAccrossMultipleSubnetsSupported() >> getDhcpServiceProvider()
>
>
>My DHCP Provider is a NetworkElement but does not implement the
>DhcpServiceProvider interface.
>
>
> public DhcpServiceProvider getDhcpServiceProvider(Network network) {
>
> String DhcpProvider =
>_ntwkSrvcDao.getProviderForServiceInNetwork(network.getId(),
>Service.Dhcp);
>
>
> if (DhcpProvider == null) {
>
> s_logger.debug("Network " + network + " doesn't support
>service " + Service.Dhcp.getName());
>
> return null;
>
> }
>
>
> return
>(DhcpServiceProvider)_networkModel.getElementImplementingProvider(DhcpProv
>ider);
>
>
> }
>
>
>There is a check in the NetworkManager in the Prepare stage but a similar
>check is missing in Release stage. Below is a patch to the problem for
>you to review. I tested it in my environment:
>
>
>diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java
>b/server/src/com/cloud/network/NetworkManagerImpl.java
>
>index ae27554..073cb95 100755
>
>--- a/server/src/com/cloud/network/NetworkManagerImpl.java
>
>+++ b/server/src/com/cloud/network/NetworkManagerImpl.java
>
>@@ -1568,7 +1568,9 @@ public class NetworkManagerImpl extends ManagerBase
>implements NetworkManager, L
>
> }
>
>
> // remove the dhcpservice ip if this is the last nic in subnet.
>
>- if (vm.getType() == Type.User &&
>isDhcpAccrossMultipleSubnetsSupported(network) && isLastNicInSubnet(nic)
>&&
>
>+ DhcpServiceProvider dhcpServiceProvider =
>getDhcpServiceProvider(network);
>
>+ if (dhcpServiceProvider != null &&
>
>+ vm.getType() == Type.User &&
>isDhcpAccrossMultipleSubnetsSupported(dhcpServiceProvider) &&
>isLastNicInSubnet(nic) &&
>
> network.getTrafficType() == TrafficType.Guest &&
>network.getGuestType() == GuestType.Shared) {
>
> removeDhcpServiceInSubnet(nic);
>
> }
>
>@@ -1582,8 +1584,7 @@ public class NetworkManagerImpl extends ManagerBase
>implements NetworkManager, L
>
> }
>
> }
>
>
>- public boolean isDhcpAccrossMultipleSubnetsSupported(Network
>network) {
>
>- DhcpServiceProvider dhcpServiceProvider =
>getDhcpServiceProvider(network);
>
>+ public boolean
>isDhcpAccrossMultipleSubnetsSupported(DhcpServiceProvider
>dhcpServiceProvider) {
>
> Map <Network.Capability, String> capabilities =
>dhcpServiceProvider.getCapabilities().get(Network.Service.Dhcp);
>
> String supportsMultipleSubnets =
>capabilities.get(Network.Capability.DhcpAccrossMultipleSubnets);
>
> if (supportsMultipleSubnets != null &&
>Boolean.valueOf(supportsMultipleSubnets)) {
>
>@@ -2448,7 +2449,12 @@ public class NetworkManagerImpl extends
>ManagerBase implements NetworkManager, L
>
> return null;
>
> }
>
>
>- return
>(DhcpServiceProvider)_networkModel.getElementImplementingProvider(DhcpProv
>ider);
>
>+ NetworkElement element =
>_networkModel.getElementImplementingProvider(DhcpProvider);
>
>+ if ( element instanceof DhcpServiceProvider ) {
>
>+ return
>(DhcpServiceProvider)_networkModel.getElementImplementingProvider(DhcpProv
>ider);
>
>+ } else {
>
>+ return null;
>
>+ }
>
>
> }
>
>
>
>-Soheil
>
>
