Lior Vernia has uploaded a new change for review. Change subject: engine: Consider removed bonds in CollectVdsNetworkData ......................................................................
engine: Consider removed bonds in CollectVdsNetworkData Changes to removed bonds should also be considered in the CollectVdsNetworkData command, because they won't necessarily be torn down by VDSM, in which case their label changes should still be persisted. There existed code to clear labels from removed bonds in SetupNetworksCommand, but it was pretty much dead code, as removed bonds by definition don't appear in the passed interfaces collection. Change-Id: I44f1d5c0702e5062fe29bc89e12ff2400b46921b Bug-Url: https://bugzilla.redhat.com/1085819 Signed-off-by: Lior Vernia <lver...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java 3 files changed, 14 insertions(+), 24 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/26/26926/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksCommand.java index e2cf4ce..6247534 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksCommand.java @@ -1,9 +1,9 @@ package org.ovirt.engine.core.bll.network.host; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -13,7 +13,6 @@ import org.ovirt.engine.core.bll.VdsHandler; import org.ovirt.engine.core.bll.network.cluster.NetworkClusterHelper; import org.ovirt.engine.core.common.action.SetupNetworksParameters; -import org.ovirt.engine.core.common.businessentities.Entities; import org.ovirt.engine.core.common.businessentities.VDS; import org.ovirt.engine.core.common.businessentities.VDSStatus; import org.ovirt.engine.core.common.businessentities.network.Network; @@ -107,17 +106,13 @@ return; } - if (!getRemovedBonds().isEmpty()) { - unlabelRemovedBonds(); - } - T bckndCmdParams = getParameters(); final SetupNetworksVdsCommandParameters vdsCmdParams = new SetupNetworksVdsCommandParameters( getVdsId(), getNetworks(), getRemovedNetworks(), getBonds(), - getRemovedBonds(), + getRemovedBonds().keySet(), getInterfaces()); vdsCmdParams.setForce(bckndCmdParams.isForce()); vdsCmdParams.setCheckConnectivity(bckndCmdParams.isCheckConnectivity()); @@ -151,15 +146,6 @@ } } - private void unlabelRemovedBonds() { - Map<String, VdsNetworkInterface> nicsByName = Entities.entitiesByName(getInterfaces()); - for (String bond : getRemovedBonds()) { - if (nicsByName.containsKey(bond)) { - nicsByName.get(bond).setLabels(null); - } - } - } - private void updateModifiedInterfaces() { TransactionSupport.executeInNewTransaction(new TransactionMethod<T>() { @@ -180,7 +166,7 @@ return getParameters().getInterfaces(); } - private Set<String> getRemovedBonds() { + private Map<String, VdsNetworkInterface> getRemovedBonds() { return helper.getRemovedBonds(); } @@ -225,9 +211,11 @@ @Override public Boolean runInTransaction() { // save the new network topology to DB + List<VdsNetworkInterface> ifaces = new ArrayList<VdsNetworkInterface>(getInterfaces()); + ifaces.addAll(getRemovedBonds().values()); Backend.getInstance().getResourceManager() .RunVdsCommand(VDSCommandType.CollectVdsNetworkData, - new CollectHostNetworkDataVdsCommandParameters(getVds(), getInterfaces())); + new CollectHostNetworkDataVdsCommandParameters(getVds(), ifaces)); // Update cluster networks (i.e. check if need to activate each new network) for (Network net : getNetworks()) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java index d644bed..0610436 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java @@ -38,7 +38,7 @@ private List<Network> modifiedNetworks = new ArrayList<Network>(); private List<String> removedNetworks = new ArrayList<String>(); private Map<String, VdsNetworkInterface> modifiedBonds = new HashMap<String, VdsNetworkInterface>(); - private Set<String> removedBonds = new HashSet<String>(); + private Map<String, VdsNetworkInterface> removedBonds = new HashMap<String, VdsNetworkInterface>(); private List<VdsNetworkInterface> modifiedInterfaces = new ArrayList<>(); /** All interface`s names that were processed by the helper. */ @@ -294,7 +294,7 @@ } if (bondNameInOldIface != null && !modifiedBonds.containsKey(bondNameInNewIface) - && !removedBonds.contains(bondNameInOldIface)) { + && !removedBonds.containsKey(bondNameInOldIface)) { modifiedBonds.put(bondNameInOldIface, getExistingIfaces().get(bondNameInOldIface)); } } @@ -593,13 +593,15 @@ /** * Extract the bonds to be removed. If a bond was attached to slaves but it's not attached to anything then it * should be removed. Otherwise, no point in removing it: Either it is still a bond, or it isn't attached to any - * slaves either way so no need to touch it. + * slaves either way so no need to touch it. If a bond is removed, its labels are also cleared. */ private void extractRemovedBonds() { for (VdsNetworkInterface iface : getExistingIfaces().values()) { String bondName = iface.getBondName(); if (StringUtils.isNotBlank(bondName) && !bonds.containsKey(bondName)) { - removedBonds.add(bondName); + VdsNetworkInterface existingBond = getExistingIfaces().get(bondName); + existingBond.setLabels(null); + removedBonds.put(bondName, existingBond); } } } @@ -664,7 +666,7 @@ return new ArrayList<VdsNetworkInterface>(modifiedBonds.values()); } - public Set<String> getRemovedBonds() { + public Map<String, VdsNetworkInterface> getRemovedBonds() { return removedBonds; } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java index b9ca651..3086875 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelperTest.java @@ -1432,7 +1432,7 @@ private void assertBondRemoved(SetupNetworksHelper helper, String expectedBondName) { assertTrue(MessageFormat.format("Expected bond ''{0}'' to be removed but it wasn''t. Removed bonds: {1}", expectedBondName, helper.getRemovedBonds()), - helper.getRemovedBonds().contains(expectedBondName)); + helper.getRemovedBonds().containsKey(expectedBondName)); } private void assertNetworkRemoved(SetupNetworksHelper helper, String expectedNetworkName) { -- To view, visit http://gerrit.ovirt.org/26926 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I44f1d5c0702e5062fe29bc89e12ff2400b46921b Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <lver...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
