Mike Kolesnik has uploaded a new change for review. Change subject: engine: Remove port if hot plug new ext. nic fails ......................................................................
engine: Remove port if hot plug new ext. nic fails If a new vNIC is added on an external network, and this vNIC is also hot plugged as part of this flow then a corresponding entity will be created on the external provider. If the aforementioned action fails, the vNIC will be removed from the DB (by compensation mechanism) but not from the external provider, leaving garbage forever on it. In this fix, a new vNIC that fails to hot plug will also be removed from the external provider. At this point, there is no need to remove should the hot plug of an existing vNIC fails, as the entity that was create on the external provider should be removed only when the vNIC is being removed, or the network is not used by it anymore. Change-Id: Iddeac63a47286e9b83ac02fb2e55f7dd56092805 Bug-Url: https://bugzilla.redhat.com/987814 Signed-off-by: Mike Kolesnik <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/ActivateDeactivateVmNicCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/ActivateDeactivateVmNicParameters.java 5 files changed, 57 insertions(+), 21 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/78/25478/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java index 02346fe..3434a43 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java @@ -29,10 +29,20 @@ super(parameters); } - protected boolean activateOrDeactivateNic(VmNic nic, PlugAction plugAction) { + protected boolean activateOrDeactivateNewNic(VmNic nic, PlugAction plugAction) { + return activateOrDeactivateNic(nic, plugAction, true); + } + + protected boolean activateOrDeactivateExistingNic(VmNic nic, PlugAction plugAction) { + return activateOrDeactivateNic(nic, plugAction, false); + } + + private boolean activateOrDeactivateNic(VmNic nic, PlugAction plugAction, boolean newNic) { + ActivateDeactivateVmNicParameters parameters = new ActivateDeactivateVmNicParameters(nic, plugAction, newNic); + parameters.setVmId(getParameters().getVmId()); + VdcReturnValueBase returnValue = - getBackend().runInternalAction(VdcActionType.ActivateDeactivateVmNic, - createActivateDeactivateParameters(nic, plugAction)); + getBackend().runInternalAction(VdcActionType.ActivateDeactivateVmNic, parameters); if (!returnValue.getSucceeded()) { propagateFailure(returnValue); } @@ -40,17 +50,9 @@ return returnValue.getSucceeded(); } - @Override protected void setActionMessageParameters() { addCanDoActionMessage(VdcBllMessages.VAR__TYPE__INTERFACE); - } - - private ActivateDeactivateVmNicParameters createActivateDeactivateParameters(VmNic nic, PlugAction plugAction) { - ActivateDeactivateVmNicParameters parameters = new ActivateDeactivateVmNicParameters(nic, plugAction); - parameters.setVmId(getParameters().getVmId()); - - return parameters; } protected boolean addMacToPool(String macAddress) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/ActivateDeactivateVmNicCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/ActivateDeactivateVmNicCommand.java index 8aa46ef..5dbea6b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/ActivateDeactivateVmNicCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/ActivateDeactivateVmNicCommand.java @@ -24,6 +24,7 @@ import org.ovirt.engine.core.common.businessentities.network.VnicProfile; import org.ovirt.engine.core.common.config.Config; import org.ovirt.engine.core.common.config.ConfigValues; +import org.ovirt.engine.core.common.errors.VdcBLLException; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.common.vdscommands.VmNicDeviceVDSParameters; import org.ovirt.engine.core.compat.Guid; @@ -43,6 +44,8 @@ private VnicProfile vnicProfile; private Network network; + + private NetworkProviderProxy providerProxy; public ActivateDeactivateVmNicCommand(T parameters) { super(parameters); @@ -118,32 +121,53 @@ protected void executeVmCommand() { // HotPlug in the host is called only if the Vm is UP if (hotPlugVmNicRequired(getVm().getStatus())) { - if (getNetwork() != null && getNetwork().isExternal()) { - handleExternalNetworks(); + boolean externalNetworkIsPlugged = getParameters().getAction() == PlugAction.PLUG + && getNetwork() != null + && getNetwork().isExternal(); + if (externalNetworkIsPlugged) { + plugToExternalNetwork(); } + try { runVdsCommand(getParameters().getAction().getCommandType(), new VmNicDeviceVDSParameters(getVdsId(), getVm(), getParameters().getNic(), vmDevice)); + } catch (VdcBLLException e) { + if (externalNetworkIsPlugged && getParameters().isNewNic()) { + unplugFromExternalNetwork(); + } + + throw e; + } } // In any case, the device is updated TransactionSupport.executeInNewTransaction(updateDevice()); setSucceeded(true); } - private void handleExternalNetworks() { - Provider<?> provider = getDbFacade().getProviderDao().get(getNetwork().getProvidedBy().getProviderId()); - NetworkProviderProxy providerProxy = ProviderProxyFactory.getInstance().create(provider); + private void plugToExternalNetwork() { Map<String, String> runtimeProperties = - providerProxy.allocate(getNetwork(), vnicProfile, getParameters().getNic()); + getProviderProxy().allocate(getNetwork(), vnicProfile, getParameters().getNic()); if (runtimeProperties != null) { getVm().getRuntimeDeviceCustomProperties().put(vmDevice, runtimeProperties); } } + private void unplugFromExternalNetwork() { + getProviderProxy().deallocate(getParameters().getNic()); + } + + private NetworkProviderProxy getProviderProxy() { + if (providerProxy == null) { + Provider<?> provider = getDbFacade().getProviderDao().get(getNetwork().getProvidedBy().getProviderId()); + providerProxy = ProviderProxyFactory.getInstance().create(provider); + } + return providerProxy; + } + private TransactionMethod<Void> updateDevice() { return new TransactionMethod<Void>() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java index ed6179a..6c35399 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AddVmInterfaceCommand.java @@ -64,7 +64,7 @@ }); if (getInterface().isPlugged()) { - succeeded = activateOrDeactivateNic(getInterface(), PlugAction.PLUG); + succeeded = activateOrDeactivateNewNic(getInterface(), PlugAction.PLUG); } else { succeeded = true; } 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 068a727..2edb943 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 @@ -125,10 +125,10 @@ if (getRequiredAction() != null){ switch (getRequiredAction()) { case PLUG: { - return activateOrDeactivateNic(getInterface(), PlugAction.PLUG); + return activateOrDeactivateExistingNic(getInterface(), PlugAction.PLUG); } case UNPLUG: { - return activateOrDeactivateNic(oldIface, PlugAction.UNPLUG); + return activateOrDeactivateExistingNic(oldIface, PlugAction.UNPLUG); } case UPDATE_VM_DEVICE: { runVdsCommand(VDSCommandType.UpdateVmInterface, diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/ActivateDeactivateVmNicParameters.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/ActivateDeactivateVmNicParameters.java index 82fdcaa..b21dbc9 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/ActivateDeactivateVmNicParameters.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/ActivateDeactivateVmNicParameters.java @@ -7,14 +7,16 @@ private static final long serialVersionUID = 8972183222927384462L; private VmNic nic; private PlugAction action; + private boolean newNic; public ActivateDeactivateVmNicParameters() { } - public ActivateDeactivateVmNicParameters(VmNic nic, PlugAction action) { + public ActivateDeactivateVmNicParameters(VmNic nic, PlugAction action, boolean newNic) { super(); this.nic = nic; this.action = action; + this.newNic = newNic; } public VmNic getNic() { @@ -33,4 +35,12 @@ this.action = action; } + public boolean isNewNic() { + return newNic; + } + + public void setNewNic(boolean newNic) { + this.newNic = newNic; + } + } -- To view, visit http://gerrit.ovirt.org/25478 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iddeac63a47286e9b83ac02fb2e55f7dd56092805 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Mike Kolesnik <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
