From 534e0977c3b73fd26230bffd5dcac0da0be7f954 Mon Sep 17 00:00:00 2001 From: Koushik Das Date: Wed, 21 Aug 2013 12:48:51 +0530 Subject: [PATCH] CLOUDSTACK-2131: [Performance][Enhancement] Avoid checking for providers that are not enabled while creating network. For some scenarios like prepare nic, all network service providers are checked which is not efficient and also introduces unnecessary dependencies. The check to use only the required providers is already there for implement, shutdown operation on network. Put the same check for all missing cases. --- .../com/cloud/network/NetworkManagerImpl.java | 88 +++++++++++++------ 1 file changed, 62 insertions(+), 26 deletions(-) diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java index c683809fc9d..c099e3de251 100755 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -1305,12 +1305,18 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L updateNic(nic, network.getId(), 1); } + List providersToImplement = getNetworkProviders(network.getId()); for (NetworkElement element : _networkElements) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Asking " + element.getName() + " to prepare for " + nic); - } - if(!prepareElement(element, network, profile, vmProfile, dest, context)) { - throw new InsufficientAddressCapacityException("unable to configure the dhcp service, due to insufficiant address capacity",Network.class, network.getId()); + if (providersToImplement.contains(element.getProvider())) { + if (!_networkModel.isProviderEnabledInPhysicalNetwork(_networkModel.getPhysicalNetworkId(network), element.getProvider().getName())) { + throw new CloudRuntimeException("Service provider " + element.getProvider().getName() + " either doesn't exist or is not enabled in physical network id: " + network.getPhysicalNetworkId()); + } + if (s_logger.isDebugEnabled()) { + s_logger.debug("Asking " + element.getName() + " to prepare for " + nic); + } + if(!prepareElement(element, network, profile, vmProfile, dest, context)) { + throw new InsufficientAddressCapacityException("unable to configure the dhcp service, due to insufficiant address capacity",Network.class, network.getId()); + } } } @@ -1335,10 +1341,16 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L s_logger.error("NetworkGuru "+guru+" prepareForMigration failed."); // XXX: Transaction error } } + List providersToImplement = getNetworkProviders(network.getId()); for (NetworkElement element : _networkElements) { - if(element instanceof NetworkMigrationResponder){ - if(!((NetworkMigrationResponder) element).prepareMigration(profile, network, vm, dest, context)){ - s_logger.error("NetworkElement "+element+" prepareForMigration failed."); // XXX: Transaction error + if (providersToImplement.contains(element.getProvider())) { + if (!_networkModel.isProviderEnabledInPhysicalNetwork(_networkModel.getPhysicalNetworkId(network), element.getProvider().getName())) { + throw new CloudRuntimeException("Service provider " + element.getProvider().getName() + " either doesn't exist or is not enabled in physical network id: " + network.getPhysicalNetworkId()); + } + if(element instanceof NetworkMigrationResponder){ + if(!((NetworkMigrationResponder) element).prepareMigration(profile, network, vm, dest, context)){ + s_logger.error("NetworkElement "+element+" prepareForMigration failed."); // XXX: Transaction error + } } } } @@ -1370,9 +1382,15 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L if(guru instanceof NetworkMigrationResponder){ ((NetworkMigrationResponder) guru).commitMigration(nicSrc, network, src, src_context, dst_context); } + List providersToImplement = getNetworkProviders(network.getId()); for (NetworkElement element : _networkElements) { - if(element instanceof NetworkMigrationResponder){ - ((NetworkMigrationResponder) element).commitMigration(nicSrc, network, src, src_context, dst_context); + if (providersToImplement.contains(element.getProvider())) { + if (!_networkModel.isProviderEnabledInPhysicalNetwork(_networkModel.getPhysicalNetworkId(network), element.getProvider().getName())) { + throw new CloudRuntimeException("Service provider " + element.getProvider().getName() + " either doesn't exist or is not enabled in physical network id: " + network.getPhysicalNetworkId()); + } + if(element instanceof NetworkMigrationResponder){ + ((NetworkMigrationResponder) element).commitMigration(nicSrc, network, src, src_context, dst_context); + } } } // update the reservation id @@ -1396,9 +1414,15 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L if(guru instanceof NetworkMigrationResponder){ ((NetworkMigrationResponder) guru).rollbackMigration(nicDst, network, dst, src_context, dst_context); } + List providersToImplement = getNetworkProviders(network.getId()); for (NetworkElement element : _networkElements) { - if(element instanceof NetworkMigrationResponder){ - ((NetworkMigrationResponder) element).rollbackMigration(nicDst, network, dst, src_context, dst_context); + if (providersToImplement.contains(element.getProvider())) { + if (!_networkModel.isProviderEnabledInPhysicalNetwork(_networkModel.getPhysicalNetworkId(network), element.getProvider().getName())) { + throw new CloudRuntimeException("Service provider " + element.getProvider().getName() + " either doesn't exist or is not enabled in physical network id: " + network.getPhysicalNetworkId()); + } + if(element instanceof NetworkMigrationResponder){ + ((NetworkMigrationResponder) element).rollbackMigration(nicDst, network, dst, src_context, dst_context); + } } } } @@ -1457,13 +1481,19 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L txn.commit(); // Perform release on network elements + List providersToImplement = getNetworkProviders(network.getId()); for (NetworkElement element : _networkElements) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Asking " + element.getName() + " to release " + nic); + if (providersToImplement.contains(element.getProvider())) { + if (!_networkModel.isProviderEnabledInPhysicalNetwork(_networkModel.getPhysicalNetworkId(network), element.getProvider().getName())) { + throw new CloudRuntimeException("Service provider " + element.getProvider().getName() + " either doesn't exist or is not enabled in physical network id: " + network.getPhysicalNetworkId()); + } + if (s_logger.isDebugEnabled()) { + s_logger.debug("Asking " + element.getName() + " to release " + nic); + } + //NOTE: Context appear to never be used in release method + //implementations. Consider removing it from interface Element + element.release(network, profile, vmProfile, null); } - //NOTE: Context appear to never be used in release method - //implementations. Consider removing it from interface Element - element.release(network, profile, vmProfile, null); } } else { @@ -1505,16 +1535,22 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L * because the nic is now being removed. */ if (nic.getReservationStrategy() == Nic.ReservationStrategy.Create) { + List providersToImplement = getNetworkProviders(network.getId()); for (NetworkElement element : _networkElements) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Asking " + element.getName() + " to release " + nic); - } - try { - element.release(network, profile, vm, null); - } catch (ConcurrentOperationException ex) { - s_logger.warn("release failed during the nic " + nic.toString() + " removeNic due to ", ex); - } catch (ResourceUnavailableException ex) { - s_logger.warn("release failed during the nic " + nic.toString() + " removeNic due to ", ex); + if (providersToImplement.contains(element.getProvider())) { + if (!_networkModel.isProviderEnabledInPhysicalNetwork(_networkModel.getPhysicalNetworkId(network), element.getProvider().getName())) { + throw new CloudRuntimeException("Service provider " + element.getProvider().getName() + " either doesn't exist or is not enabled in physical network id: " + network.getPhysicalNetworkId()); + } + if (s_logger.isDebugEnabled()) { + s_logger.debug("Asking " + element.getName() + " to release " + nic); + } + try { + element.release(network, profile, vm, null); + } catch (ConcurrentOperationException ex) { + s_logger.warn("release failed during the nic " + nic.toString() + " removeNic due to ", ex); + } catch (ResourceUnavailableException ex) { + s_logger.warn("release failed during the nic " + nic.toString() + " removeNic due to ", ex); + } } } }