Muli Salem has uploaded a new change for review. Change subject: engine: Allow Import/Snapshot Duplicate MAC Address ......................................................................
engine: Allow Import/Snapshot Duplicate MAC Address This patch, allows importing/previewing a VM with a network interface with an existing MAC address, even though the AllowDuplicateMacAddresses flag is false. Instead of failing the command, we simply unplug the network interface. When user tries to plug in the interface, we will fail the action unless the above flag is true. Change-Id: I269157b37426942331002c33a22fa354ba0cf39e Bug-Url: https://bugzilla.redhat.com/873338 Signed-off-by: Muli Salem <msa...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.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/snapshots/SnapshotsManager.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/VmInterfaceManagerTest.java M backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties 5 files changed, 20 insertions(+), 30 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/41/12241/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java index 67578b9..c265344 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java @@ -39,6 +39,7 @@ import org.ovirt.engine.core.common.businessentities.Snapshot; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotStatus; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType; +import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainType; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.VM; @@ -48,7 +49,6 @@ import org.ovirt.engine.core.common.businessentities.VmStatistics; import org.ovirt.engine.core.common.businessentities.VmTemplate; import org.ovirt.engine.core.common.businessentities.VmTemplateStatus; -import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.network.Network; import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface; import org.ovirt.engine.core.common.errors.VdcBLLException; @@ -926,7 +926,7 @@ iface.setNetworkName(null); } - vmInterfaceManager.add(iface, getCompensationContext(), getParameters().isImportAsNewEntity()); + vmInterfaceManager.forceAdd(iface, getCompensationContext(), getParameters().isImportAsNewEntity()); macsAdded.add(iface.getMacAddress()); } 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 01a3f78..3073a49 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 @@ -9,8 +9,6 @@ import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.network.Network; import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface; -import org.ovirt.engine.core.common.errors.VdcBLLException; -import org.ovirt.engine.core.common.errors.VdcBllErrors; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; @@ -29,9 +27,11 @@ public class VmInterfaceManager { protected Log log = LogFactory.getLog(getClass()); + /** * Add a {@link VmNetworkInterface} to the VM, trying to acquire a MAC from the {@link MacPoolManager}.<br> - * If the MAC is already in use, a warning will be sent to the user. + * If the MAC is already in use and MAC will still be added to {@link MacPoolManager}, a warning will be sent to the + * user and the Network Interface will be unplugged. * * @param iface * The interface to save. @@ -39,13 +39,17 @@ * Used to snapshot the saved entities. * @return <code>true</code> if the MAC wasn't used, <code>false</code> if it was. */ - public void add(final VmNetworkInterface iface, CompensationContext compensationContext, boolean allocateMac) { + public void forceAdd(final VmNetworkInterface iface, CompensationContext compensationContext, boolean allocateMac) { if (allocateMac) { iface.setMacAddress(getMacPoolManager().allocateNewMac()); - } else if (!getMacPoolManager().addMac(iface.getMacAddress())) { - auditLogMacInUse(iface); - log.errorFormat("VmInterfaceManager::Mac {0} is in use.", iface.getMacAddress()); - throw new VdcBLLException(VdcBllErrors.MAC_ADDRESS_IS_IN_USE); + } else { + if (!getMacPoolManager().addMac(iface.getMacAddress())) { + iface.setPlugged(false); + getMacPoolManager().forceAddMac(iface.getMacAddress()); + auditLogMacInUse(iface); + log.infoFormat("VmInterfaceManager::Network Interface {0}'s MAC address is in use, " + + "therefore it is being unplugged from VM.", iface.getName()); + } } getVmNetworkInterfaceDao().save(iface); @@ -62,6 +66,7 @@ AuditLogableBase logable = new AuditLogableBase(); logable.addCustomValue("MACAddr", iface.getMacAddress()); logable.addCustomValue("VmName", iface.getVmName()); + logable.addCustomValue("IfaceName", iface.getName()); log(logable, AuditLogType.MAC_ADDRESS_IS_IN_USE); return null; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java index cc55547..8b2a038 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/SnapshotsManager.java @@ -340,7 +340,7 @@ } vmInterface.setVmId(vmId); - vmInterfaceManager.add(vmInterface, compensationContext, false); + vmInterfaceManager.forceAdd(vmInterface, compensationContext, false); } } 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 3865d31..b1005e6 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,7 +3,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; @@ -26,8 +25,6 @@ import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.network.Network; import org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface; -import org.ovirt.engine.core.common.errors.VdcBLLException; -import org.ovirt.engine.core.common.errors.VdcBllErrors; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogableBase; import org.ovirt.engine.core.dao.VmDAO; @@ -76,26 +73,14 @@ @Test public void addWithExistingMacAddress() { VmNetworkInterface iface = createNewInterface(); - - try { - runAddAndVerify(iface, false, times(1)); - fail(String.format("Expected to catch %s, but didn't.", VdcBLLException.class.getSimpleName())); - } catch (VdcBLLException e) { - assertEquals(VdcBllErrors.MAC_ADDRESS_IS_IN_USE, e.getErrorCode()); - } catch (Exception e) { - fail(String.format("Expected to catch %s, but caught %s instead.", - VdcBLLException.class.getSimpleName(), - e.getClass().getSimpleName())); - } + runAddAndVerify(iface, true, times(1)); verify(vmInterfaceManager).auditLogMacInUse(iface); } protected void runAddAndVerify(VmNetworkInterface iface, boolean addMacResult, VerificationMode addMacVerification) { - when(macPoolManager.addMac(iface.getMacAddress())).thenReturn(addMacResult); - - vmInterfaceManager.add(iface, NoOpCompensationContext.getInstance(), false); + vmInterfaceManager.forceAdd(iface, NoOpCompensationContext.getInstance(), false); verifyAddDelegatedCorrectly(iface, addMacVerification); } @@ -104,7 +89,7 @@ VmNetworkInterface iface = createNewInterface(); String newMac = RandomUtils.instance().nextString(10); when(macPoolManager.allocateNewMac()).thenReturn(newMac); - vmInterfaceManager.add(iface, NoOpCompensationContext.getInstance(), true); + vmInterfaceManager.forceAdd(iface, NoOpCompensationContext.getInstance(), true); assertEquals(newMac, iface.getMacAddress()); } diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties b/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties index b776f06..f4d461e 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties @@ -321,7 +321,7 @@ USER_UPDATE_VM_POOL_WITH_VMS_FAILED=Failed to update VM Pool ${VmPoolName}(User: ${UserName}). USER_VM_POOL_MAX_SUBSEQUENT_FAILURES_REACHED=Not all VMs where successfully created in VM Pool ${VmPoolName}. MAC_POOL_EMPTY=No MAC addresses left in the MAC Address Pool. -MAC_ADDRESS_IS_IN_USE=VM ${VmName}: Mac Address ${MACAddr} is in use. +MAC_ADDRESS_IS_IN_USE=VM ${VmName}: Mac Address ${MACAddr} is in use, therefore Network Interface ${IfaceName} was unplugged. MAC_ADDRESSES_POOL_NOT_INITIALIZED=Mac Address Pool is not initialized. ${Message} USER_PASSWORD_CHANGED=Password changed successfully for ${UserName} USER_PASSWORD_CHANGE_FAILED=Failed to change password. (User: ${UserName}) -- To view, visit http://gerrit.ovirt.org/12241 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I269157b37426942331002c33a22fa354ba0cf39e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Muli Salem <msa...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
