Mike Kolesnik has uploaded a new change for review. Change subject: engine: Remove vNIC from external network when necessary ......................................................................
engine: Remove vNIC from external network when necessary Removing the vNIC from the external network only when the vNIC is removed or the VM gets somehow removed. This will allow the user of the vNIC to manipulate it on the external network and not lose his changes when the VM stops. Change-Id: I027992f52c35add0afd1ffe207b1adbdb2c5c957 Bug-Url: https://bugzilla.redhat.com/1016457 Signed-off-by: Mike Kolesnik <mkole...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/VmInterfaceManager.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/RemoveVmInterfaceCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/network/openstack/OpenstackNetworkProviderProxy.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStoragePoolCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/VmInterfaceManagerTest.java 8 files changed, 99 insertions(+), 52 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/66/19966/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java index 35ae0d2..50f35ba 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java @@ -11,6 +11,7 @@ import org.ovirt.engine.core.bll.job.ExecutionContext; import org.ovirt.engine.core.bll.job.ExecutionHandler; import org.ovirt.engine.core.bll.memory.MemoryImageRemoverOnDataDomain; +import org.ovirt.engine.core.bll.network.ExternalNetworkManager; import org.ovirt.engine.core.bll.quota.QuotaConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaStorageConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaStorageDependent; @@ -35,8 +36,9 @@ import org.ovirt.engine.core.common.businessentities.LunDisk; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; -import org.ovirt.engine.core.common.locks.LockingGroup; +import org.ovirt.engine.core.common.businessentities.network.VmNic; import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.NotImplementedException; @@ -88,6 +90,11 @@ true, false); + for (VmNic nic : getInterfaces()) { + ExternalNetworkManager externalNetworkManager = new ExternalNetworkManager(nic); + externalNetworkManager.deallocateIfExternal(); + } + TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { @Override public Void runInTransaction() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolHandler.java index 559b463..e5d3543 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolHandler.java @@ -1,25 +1,17 @@ package org.ovirt.engine.core.bll; import java.util.List; -import java.util.Map; import org.ovirt.engine.core.bll.context.CommandContext; -import org.ovirt.engine.core.bll.network.cluster.NetworkHelper; -import org.ovirt.engine.core.bll.provider.ProviderProxyFactory; -import org.ovirt.engine.core.bll.provider.network.NetworkProviderProxy; import org.ovirt.engine.core.bll.quota.QuotaManager; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VmOperationParameterBase; import org.ovirt.engine.core.common.action.VmPoolSimpleUserParameters; import org.ovirt.engine.core.common.businessentities.DbUser; -import org.ovirt.engine.core.common.businessentities.Entities; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType; import org.ovirt.engine.core.common.businessentities.VmPool; import org.ovirt.engine.core.common.businessentities.VmPoolMap; import org.ovirt.engine.core.common.businessentities.VmPoolType; -import org.ovirt.engine.core.common.businessentities.VmStatic; -import org.ovirt.engine.core.common.businessentities.network.Network; -import org.ovirt.engine.core.common.businessentities.network.VmNic; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.utils.log.Log; @@ -58,23 +50,6 @@ QuotaManager.getInstance().rollbackQuotaByVmId(vmId); VmHandler.removeStatelessVmUnmanagedDevices(vmId); - handleProviderNetworks(vmId); - } - - private static void handleProviderNetworks(Guid vmId) { - List<VmNic> interfaces = DbFacade.getInstance().getVmNicDao().getAllForVm(vmId); - VmStatic vm = DbFacade.getInstance().getVmStaticDao().get(vmId); - Map<String, Network> clusterNetworks = - Entities.entitiesByName(DbFacade.getInstance().getNetworkDao().getAllForCluster(vm.getVdsGroupId())); - - for (VmNic iface : interfaces) { - Network network = NetworkHelper.getNetworkByVnicProfileId(iface.getVnicProfileId()); - if (network != null && network.isExternal() && clusterNetworks.containsKey(network.getName())) { - NetworkProviderProxy providerProxy = ProviderProxyFactory.getInstance().create( - DbFacade.getInstance().getProviderDao().get(network.getProvidedBy().getProviderId())); - providerProxy.deallocate(iface); - } - } } public static void removeVmStatelessImages(Guid vmId, CommandContext context) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/VmInterfaceManager.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/VmInterfaceManager.java index 38783ec..a9acf30 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/VmInterfaceManager.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/VmInterfaceManager.java @@ -3,6 +3,8 @@ import java.util.ArrayList; import java.util.List; +import javax.transaction.Transaction; + import org.ovirt.engine.core.bll.context.CompensationContext; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.FeatureSupported; @@ -101,6 +103,8 @@ public void removeAll(Guid vmId) { List<VmNic> interfaces = getVmNicDao().getAllForVm(vmId); if (interfaces != null) { + removeFromExternalNetworks(interfaces); + for (VmNic iface : interfaces) { getMacPoolManager().freeMac(iface.getMacAddress()); getVmNicDao().remove(iface.getId()); @@ -109,6 +113,14 @@ } } + protected void removeFromExternalNetworks(List<VmNic> interfaces) { + Transaction transaction = TransactionSupport.suspend(); + for (VmNic iface : interfaces) { + new ExternalNetworkManager(iface).deallocateIfExternal(); + } + TransactionSupport.resume(transaction); + } + /** * Finds active VMs which uses a network from a given networks list * diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/RemoveVmInterfaceCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/RemoveVmInterfaceCommand.java index a4ad472..16c0b77 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/RemoveVmInterfaceCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/RemoveVmInterfaceCommand.java @@ -1,6 +1,8 @@ package org.ovirt.engine.core.bll.network.vm; +import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute; import org.ovirt.engine.core.bll.VmCommand; +import org.ovirt.engine.core.bll.network.ExternalNetworkManager; import org.ovirt.engine.core.bll.network.MacPoolManager; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.RemoveVmInterfaceParameters; @@ -10,7 +12,10 @@ import org.ovirt.engine.core.common.businessentities.network.VmInterfaceType; import org.ovirt.engine.core.common.businessentities.network.VmNic; import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.utils.transaction.TransactionMethod; +import org.ovirt.engine.core.utils.transaction.TransactionSupport; +@NonTransactiveCommandAttribute public class RemoveVmInterfaceCommand<T extends RemoveVmInterfaceParameters> extends VmCommand<T> { private String interfaceName = ""; @@ -26,11 +31,13 @@ @Override protected void executeVmCommand() { this.setVmName(getVmStaticDAO().get(getParameters().getVmId()).getName()); - - // return mac to pool VmNic iface = getVmNicDao().get(getParameters().getInterfaceId()); if (iface != null) { + ExternalNetworkManager externalNetworkManager = new ExternalNetworkManager(iface); + externalNetworkManager.deallocateIfExternal(); + + // return mac to pool MacPoolManager.getInstance().freeMac(iface.getMacAddress()); interfaceName = iface.getName(); @@ -42,11 +49,17 @@ } // remove from db - getVmNicDao().remove(getParameters().getInterfaceId()); - getDbFacade().getVmNetworkStatisticsDao().remove(getParameters().getInterfaceId()); - getDbFacade().getVmDeviceDao().remove(new VmDeviceId(getParameters().getInterfaceId(), - getParameters().getVmId())); - setSucceeded(true); + TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { + @Override + public Void runInTransaction() { + getVmNicDao().remove(getParameters().getInterfaceId()); + getDbFacade().getVmNetworkStatisticsDao().remove(getParameters().getInterfaceId()); + getDbFacade().getVmDeviceDao().remove(new VmDeviceId(getParameters().getInterfaceId(), + getParameters().getVmId())); + setSucceeded(true); + return null; + } + }); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java index b8a54ab..d8a5105 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java @@ -7,6 +7,7 @@ import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute; import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.bll.network.ExternalNetworkManager; import org.ovirt.engine.core.bll.network.MacPoolManager; import org.ovirt.engine.core.bll.network.cluster.NetworkHelper; import org.ovirt.engine.core.bll.utils.PermissionSubject; @@ -75,6 +76,15 @@ boolean succeeded = false; boolean macAddedToPool = false; try { + if (isVnicProfileChanged(oldIface, getInterface())) { + Network newNetwork = NetworkHelper.getNetworkByVnicProfileId(getInterface().getVnicProfileId()); + Network oldNetwork = NetworkHelper.getNetworkByVnicProfileId(oldIface.getVnicProfileId()); + if (ObjectUtils.notEqual(oldNetwork, newNetwork)) { + ExternalNetworkManager externalNetworkManager = new ExternalNetworkManager(oldIface); + externalNetworkManager.deallocateIfExternal(); + } + } + if (macShouldBeChanged) { macAddedToPool = addMacToPool(getMacAddress()); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/network/openstack/OpenstackNetworkProviderProxy.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/network/openstack/OpenstackNetworkProviderProxy.java index 0a5feab..8926363 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/network/openstack/OpenstackNetworkProviderProxy.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/network/openstack/OpenstackNetworkProviderProxy.java @@ -150,23 +150,26 @@ @Override public Map<String, String> allocate(Network network, VmNic nic) { - deallocate(nic); try { - com.woorea.openstack.quantum.model.Network externalNetwork = - getClient().networks().show(network.getProvidedBy().getExternalId()).execute(); - PortForCreate port = new PortForCreate(); - port.setAdminStateUp(true); - port.setName(nic.getName()); - port.setTenantId(externalNetwork.getTenantId()); - port.setMacAddress(nic.getMacAddress()); - port.setNetworkId(externalNetwork.getId()); - port.setDeviceOwner(DEVICE_OWNER); - port.setDeviceId(nic.getId().toString()); + Port port = locatePort(nic); - Port createdPort = getClient().ports().create(port).execute(); + if (port == null) { + com.woorea.openstack.quantum.model.Network externalNetwork = + getClient().networks().show(network.getProvidedBy().getExternalId()).execute(); + PortForCreate portForCreate = new PortForCreate(); + portForCreate.setAdminStateUp(true); + portForCreate.setName(nic.getName()); + portForCreate.setTenantId(externalNetwork.getTenantId()); + portForCreate.setMacAddress(nic.getMacAddress()); + portForCreate.setNetworkId(externalNetwork.getId()); + portForCreate.setDeviceOwner(DEVICE_OWNER); + portForCreate.setDeviceId(nic.getId().toString()); + port = getClient().ports().create(portForCreate).execute(); + } + Map<String, String> runtimeProperties = new HashMap<>(); - runtimeProperties.put("vnic_id", createdPort.getId()); + runtimeProperties.put("vnic_id", port.getId()); runtimeProperties.put("provider_type", provider.getType().name()); runtimeProperties.put("plugin_type", provider.getAdditionalProperties().getPluginType()); @@ -179,17 +182,26 @@ @Override public void deallocate(VmNic nic) { try { - List<Port> ports = getClient().ports().list().execute().getList(); - for (Port port : ports) { - if (DEVICE_OWNER.equals(port.getDeviceOwner()) && nic.getId().toString().equals(port.getDeviceId())) { - getClient().ports().delete(port.getId()).execute(); - } + Port port = locatePort(nic); + + if (port != null) { + getClient().ports().delete(port.getId()).execute(); } } catch (RuntimeException e) { throw new VdcBLLException(VdcBllErrors.PROVIDER_FAILURE, e); } } + private Port locatePort(VmNic nic) { + List<Port> ports = getClient().ports().list().execute().getList(); + for (Port port : ports) { + if (DEVICE_OWNER.equals(port.getDeviceOwner()) && nic.getId().toString().equals(port.getDeviceId())) { + return port; + } + } + return null; + } + @JsonIgnoreProperties(ignoreUnknown = true) private static class ApiRootResponse { // No implementation since we don't care what's inside the response, just that it succeeded. diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStoragePoolCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStoragePoolCommand.java index 1eea76c..7651667 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStoragePoolCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveStoragePoolCommand.java @@ -11,6 +11,7 @@ import org.ovirt.engine.core.bll.LockMessagesMatchUtil; import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute; import org.ovirt.engine.core.bll.context.CommandContext; +import org.ovirt.engine.core.bll.network.ExternalNetworkManager; import org.ovirt.engine.core.bll.network.MacPoolManager; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.DetachStorageDomainFromPoolParameters; @@ -27,6 +28,7 @@ import org.ovirt.engine.core.common.businessentities.VDSStatus; import org.ovirt.engine.core.common.businessentities.VmStatic; import org.ovirt.engine.core.common.businessentities.network.Network; +import org.ovirt.engine.core.common.businessentities.network.VmNic; import org.ovirt.engine.core.common.businessentities.network.VnicProfile; import org.ovirt.engine.core.common.errors.VdcBLLException; import org.ovirt.engine.core.common.errors.VdcBllMessages; @@ -90,11 +92,21 @@ } private void removeNetworks() { + final List<Network> networks = getNetworkDAO().getAllForDataCenter(getStoragePoolId()); + for (Network network : networks) { + if (network.isExternal()) { + ExternalNetworkManager externalNetworkManager = new ExternalNetworkManager(network); + for (VmNic nic : getVmNicDao().getAllForNetwork(network.getId())) { + externalNetworkManager.setNic(nic); + externalNetworkManager.deallocateIfExternal(); + } + } + } + TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { @Override public Void runInTransaction() { - final List<Network> networks = getNetworkDAO().getAllForDataCenter(getStoragePoolId()); for (final Network net : networks) { List<VnicProfile> profiles = getDbFacade().getVnicProfileDao().getAllForNetwork(net.getId()); for (VnicProfile vnicProfile : profiles) { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/VmInterfaceManagerTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/VmInterfaceManagerTest.java index 3c56024..8e6ba30 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/VmInterfaceManagerTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/VmInterfaceManagerTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyList; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.times; @@ -63,6 +64,9 @@ @Mock private VmDAO vmDAO; + @Mock + private ExternalNetworkManager externalNetworkManager; + @Spy private VmInterfaceManager vmInterfaceManager = new VmInterfaceManager(); @@ -70,6 +74,7 @@ private Version version; @Before + @SuppressWarnings("unchecked") public void setupMocks() { MockitoAnnotations.initMocks(this); @@ -79,6 +84,7 @@ doReturn(vmNicDao).when(vmInterfaceManager).getVmNicDao(); doReturn(vmDAO).when(vmInterfaceManager).getVmDAO(); doNothing().when(vmInterfaceManager).auditLogMacInUseUnplug(any(VmNic.class)); + doNothing().when(vmInterfaceManager).removeFromExternalNetworks(anyList()); doNothing().when(vmInterfaceManager).log(any(AuditLogableBase.class), any(AuditLogType.class)); } -- To view, visit http://gerrit.ovirt.org/19966 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I027992f52c35add0afd1ffe207b1adbdb2c5c957 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Mike Kolesnik <mkole...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
