Muli Salem has uploaded a new change for review.
Change subject: engine: Support Duplicate Mac Addresses
......................................................................
engine: Support Duplicate Mac Addresses
This patch adds support to duplicate Mac addresses.
It separates the logic in MacPoolManager into two
possible polices, duplicate and non-duplicate.
Upon engine start, the config value is taken
and the MacPoolManager is initialized accordingly.
Change-Id: Ie96eda19de2d3a44e24806095fb690e4eba41165
Signed-off-by: Muli Salem <msa...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java
A
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerPolicy.java
A
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerPolicyDuplicate.java
A
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerPolicyNonDuplicate.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
6 files changed, 197 insertions(+), 50 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/98/10698/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java
index 40afe72..03d711e 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManager.java
@@ -1,6 +1,5 @@
package org.ovirt.engine.core.bll.network;
-import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
@@ -27,9 +26,7 @@
private static Log log = LogFactory.getLog(MacPoolManager.class);
- private final Set<String> availableMacs = new HashSet<String>();
- private final Set<String> allocatedMacs = new HashSet<String>();
- private final Set<String> allocatedCustomMacs = new HashSet<String>();
+ private MacPoolManagerPolicy policy;
private final ReentrantReadWriteLock lockObj = new
ReentrantReadWriteLock();
private boolean initialized;
@@ -42,6 +39,11 @@
}
public void initialize() {
+ if (Config.<Boolean>
GetValue(ConfigValues.AllowDuplicateMacAddresses)) {
+ policy = new MacPoolManagerPolicyDuplicate();
+ } else {
+ policy = new MacPoolManagerPolicyNonDuplicate();
+ }
lockObj.writeLock().lock();
try {
String ranges = Config.<String>
GetValue(ConfigValues.MacPoolRanges);
@@ -79,9 +81,13 @@
}
}
- if (availableMacs.isEmpty()) {
+ if (getAvailableMacs().isEmpty()) {
throw new
VdcBLLException(VdcBllErrors.MAC_POOL_INITIALIZATION_FAILED);
}
+ }
+
+ private Set<String> getAvailableMacs() {
+ return policy.getAvailableMacs();
}
private String parseRangePart(String start) {
@@ -123,10 +129,10 @@
}
value = builder.toString();
value = value.substring(0, value.length() - 1);
- if (!availableMacs.contains(value)) {
- availableMacs.add(value);
+ if (!getAvailableMacs().contains(value)) {
+ getAvailableMacs().add(value);
}
- if (availableMacs.size() > Config.<Integer>
GetValue(ConfigValues.MaxMacsCountInPool)) {
+ if (getAvailableMacs().size() > Config.<Integer>
GetValue(ConfigValues.MaxMacsCountInPool)) {
throw new MacPoolExceededMaxException();
}
}
@@ -142,10 +148,10 @@
logInitializationError("Failed to allocate new Mac address.");
throw new
VdcBLLException(VdcBllErrors.MAC_POOL_NOT_INITIALIZED);
}
- if (availableMacs.isEmpty()) {
+ if (getAvailableMacs().isEmpty()) {
throw new VdcBLLException(VdcBllErrors.MAC_POOL_NO_MACS_LEFT);
}
- Iterator<String> my = availableMacs.iterator();
+ Iterator<String> my = getAvailableMacs().iterator();
mac = my.next();
commitNewMac(mac);
} finally {
@@ -156,9 +162,8 @@
}
private boolean commitNewMac(String mac) {
- availableMacs.remove(mac);
- allocatedMacs.add(mac);
- if (availableMacs.isEmpty()) {
+ policy.commitNewMac(mac);
+ if (getAvailableMacs().isEmpty()) {
AuditLogableBase logable = new AuditLogableBase();
AuditLogDirector.log(logable, AuditLogType.MAC_POOL_EMPTY);
return false;
@@ -167,7 +172,7 @@
}
public int getavailableMacsCount() {
- return availableMacs.size();
+ return getAvailableMacs().size();
}
public void freeMac(String mac) {
@@ -177,7 +182,7 @@
if (!initialized) {
logInitializationError("Failed to free mac address " + mac + "
.");
} else {
- internalFreeMac(mac);
+ policy.freeMac(mac);
}
} finally {
lockObj.writeLock().unlock();
@@ -191,15 +196,6 @@
AuditLogDirector.log(logable,
AuditLogType.MAC_ADDRESSES_POOL_NOT_INITIALIZED);
}
- private void internalFreeMac(String mac) {
- if (allocatedCustomMacs.contains(mac)) {
- allocatedCustomMacs.remove(mac);
- } else if (allocatedMacs.contains(mac)) {
- allocatedMacs.remove(mac);
- availableMacs.add(mac);
- }
- }
-
/**
* Add user define mac address Function return false if the mac is in use
*
@@ -210,17 +206,7 @@
boolean retVal = true;
lockObj.writeLock().lock();
try {
- if (allocatedMacs.contains(mac)) {
- retVal = false;
- } else {
- if (availableMacs.contains(mac)) {
- retVal = commitNewMac(mac);
- } else if (allocatedCustomMacs.contains(mac)) {
- retVal = false;
- } else {
- allocatedCustomMacs.add(mac);
- }
- }
+ retVal = policy.addMac(mac);
} finally {
lockObj.writeLock().unlock();
}
@@ -230,7 +216,7 @@
public boolean isMacInUse(String mac) {
lockObj.readLock().lock();
try {
- return allocatedMacs.contains(mac) ||
allocatedCustomMacs.contains(mac);
+ return policy.isMacInUse(mac);
} finally {
lockObj.readLock().unlock();
}
@@ -244,7 +230,7 @@
logInitializationError("Failed to free MAC addresses.");
}
for (String mac : macs) {
- internalFreeMac(mac);
+ policy.freeMac(mac);
}
} finally {
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerPolicy.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerPolicy.java
new file mode 100644
index 0000000..70c37b7
--- /dev/null
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerPolicy.java
@@ -0,0 +1,39 @@
+package org.ovirt.engine.core.bll.network;
+
+import java.util.Set;
+
+public interface MacPoolManagerPolicy {
+
+ /**
+ * Commits a Mac address and removes it from the available Mac addresses
list
+ *
+ * @param mac
+ */
+ void commitNewMac(String mac);
+
+ /**
+ * Frees the given Mac address
+ */
+ void freeMac(String mac);
+
+ /**
+ * Adds a Mac address.
+ *
+ * @param mac
+ * @return
+ */
+ public boolean addMac(String mac);
+
+ /**
+ * Checks whether the given Mac address is in use.
+ *
+ * @param mac
+ * @return
+ */
+ public boolean isMacInUse(String mac);
+
+ /**
+ * Returns the set of available Mac addresses
+ */
+ public Set<String> getAvailableMacs();
+}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerPolicyDuplicate.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerPolicyDuplicate.java
new file mode 100644
index 0000000..cff4baa
--- /dev/null
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerPolicyDuplicate.java
@@ -0,0 +1,78 @@
+package org.ovirt.engine.core.bll.network;
+
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+public class MacPoolManagerPolicyDuplicate implements MacPoolManagerPolicy {
+
+ private final Set<String> availableMacs = new HashSet<String>();
+
+ // Maps that hold the allocated Mac addresses as keys, and counters as
values
+ private final Map<String, Integer> allocatedMacs = new HashMap<String,
Integer>();
+ private final Map<String, Integer> allocatedCustomMacs = new
HashMap<String, Integer>();
+
+ @Override
+ public void commitNewMac(String mac) {
+ availableMacs.remove(mac);
+ addMacToMap(allocatedMacs, mac);
+ }
+
+ @Override
+ public void freeMac(String mac) {
+ if (allocatedMacs.get(mac) != null) {
+ if (allocatedMacs.get(mac) == 1) {
+ allocatedMacs.remove(mac);
+ availableMacs.add(mac);
+ } else if (allocatedMacs.get(mac) > 1) {
+ decrementMacInMap(allocatedMacs, mac);
+ }
+ } else if (allocatedCustomMacs.get(mac) != null) {
+ if (allocatedCustomMacs.get(mac) == 1) {
+ allocatedCustomMacs.remove(mac);
+ } else if (allocatedCustomMacs.get(mac) > 1) {
+ decrementMacInMap(allocatedCustomMacs, mac);
+ }
+ }
+ }
+
+ @Override
+ public boolean addMac(String mac) {
+ if (availableMacs.contains(mac)) {
+ commitNewMac(mac);
+ } else if (allocatedMacs.containsKey(mac)) {
+ addMacToMap(allocatedMacs, mac);
+ } else {
+ addMacToMap(allocatedCustomMacs, mac);
+ }
+ return true;
+ }
+
+ @Override
+ public boolean isMacInUse(String mac) {
+ return allocatedMacs.containsKey(mac) ||
allocatedCustomMacs.containsKey(mac);
+ }
+
+ @Override
+ public Set<String> getAvailableMacs() {
+ return availableMacs;
+ }
+
+ private void incrementMacInMap(Map<String, Integer> macMap, String mac) {
+ macMap.put(mac, macMap.get(mac) + 1);
+ }
+
+ private void decrementMacInMap(Map<String, Integer> macMap, String mac) {
+ macMap.put(mac, macMap.get(mac) - 1);
+ }
+
+ private void addMacToMap(Map<String, Integer> macMap, String mac) {
+ if (macMap.containsKey(mac)) {
+ incrementMacInMap(macMap, mac);
+ } else {
+ macMap.put(mac, 1); // First time this mac is used
+ }
+ }
+
+}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerPolicyNonDuplicate.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerPolicyNonDuplicate.java
new file mode 100644
index 0000000..0747c68
--- /dev/null
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/MacPoolManagerPolicyNonDuplicate.java
@@ -0,0 +1,54 @@
+package org.ovirt.engine.core.bll.network;
+
+import java.util.HashSet;
+import java.util.Set;
+
+public class MacPoolManagerPolicyNonDuplicate implements MacPoolManagerPolicy {
+
+ private final Set<String> availableMacs = new HashSet<String>();
+ private final Set<String> allocatedMacs = new HashSet<String>();
+ private final Set<String> allocatedCustomMacs = new HashSet<String>();
+
+ @Override
+ public void commitNewMac(String mac) {
+ availableMacs.remove(mac);
+ allocatedMacs.add(mac);
+ }
+
+ @Override
+ public void freeMac(String mac) {
+ if (allocatedCustomMacs.contains(mac)) {
+ allocatedCustomMacs.remove(mac);
+ } else if (allocatedMacs.contains(mac)) {
+ allocatedMacs.remove(mac);
+ availableMacs.add(mac);
+ }
+ }
+
+ @Override
+ public boolean addMac(String mac) {
+ if (allocatedMacs.contains(mac)) {
+ return false;
+ } else {
+ if (availableMacs.contains(mac)) {
+ commitNewMac(mac);
+ return true;
+ } else if (allocatedCustomMacs.contains(mac)) {
+ return false;
+ } else {
+ allocatedCustomMacs.add(mac);
+ return true;
+ }
+ }
+ }
+
+ @Override
+ public boolean isMacInUse(String mac) {
+ return allocatedMacs.contains(mac) ||
allocatedCustomMacs.contains(mac);
+ }
+
+ @Override
+ public Set<String> getAvailableMacs() {
+ return availableMacs;
+ }
+}
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 170ba10..c464fe9 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
@@ -23,8 +23,6 @@
import org.ovirt.engine.core.common.businessentities.network.Network;
import org.ovirt.engine.core.common.businessentities.network.VmInterfaceType;
import
org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface;
-import org.ovirt.engine.core.common.config.Config;
-import org.ovirt.engine.core.common.config.ConfigValues;
import org.ovirt.engine.core.common.utils.ValidationUtils;
import org.ovirt.engine.core.common.validation.group.CreateEntity;
import org.ovirt.engine.core.compat.Guid;
@@ -182,10 +180,8 @@
return false;
}
- Boolean allowDupMacs = Config.<Boolean>
GetValue(ConfigValues.AllowDuplicateMacAddresses);
// this must be the last check because it adds the mac address to
the pool
- if (!MacPoolManager.getInstance().addMac(getMacAddress())
- && !allowDupMacs) {
+ if (!MacPoolManager.getInstance().addMac(getMacAddress())) {
addCanDoActionMessage(VdcBllMessages.NETWORK_MAC_ADDRESS_IN_USE);
return false;
}
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 c6b9f69..d61f2a5 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
@@ -24,8 +24,6 @@
import org.ovirt.engine.core.common.businessentities.network.Network;
import org.ovirt.engine.core.common.businessentities.network.VmInterfaceType;
import
org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface;
-import org.ovirt.engine.core.common.config.Config;
-import org.ovirt.engine.core.common.config.ConfigValues;
import org.ovirt.engine.core.common.utils.ValidationUtils;
import org.ovirt.engine.core.common.validation.group.UpdateVmNic;
import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
@@ -157,9 +155,7 @@
super.rollback();
if (macAddressChanged) {
MacPoolManager.getInstance().addMac(oldIface.getMacAddress());
- if (!Config.<Boolean>
GetValue(ConfigValues.AllowDuplicateMacAddresses)) {
- MacPoolManager.getInstance().freeMac(getMacAddress());
- }
+ MacPoolManager.getInstance().freeMac(getMacAddress());
}
}
@@ -244,10 +240,8 @@
return false;
}
- Boolean allowDupMacs = Config.<Boolean>
GetValue(ConfigValues.AllowDuplicateMacAddresses);
// this must be the last check because it adds the mac address to
the pool
- if (!MacPoolManager.getInstance().addMac(getMacAddress())
- && !allowDupMacs) {
+ if (!MacPoolManager.getInstance().addMac(getMacAddress())) {
addCanDoActionMessage(VdcBllMessages.NETWORK_MAC_ADDRESS_IN_USE);
return false;
}
--
To view, visit http://gerrit.ovirt.org/10698
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie96eda19de2d3a44e24806095fb690e4eba41165
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
