[Engine-patches] Change in ovirt-engine[master]: [WIP] Improving synchronization of MacPoolManager

[yzaslavs]

Yair Zaslavsky has uploaded a new change for review.

Change subject: [WIP] Improving synchronization of MacPoolManager
......................................................................

[WIP] Improving synchronization of MacPoolManager

Improving the synchronization of MacPpoolManager, changing
from synchronzied to RWLock, and changing the synchronization
of freeMacs (the write lock should be locked one for all macs
that are removed)

Change-Id: I62fa797fbf286513183f0a3c3409cd79bdc25341
Bug-Url: https://bugzilla.redhat.com/885441
signed-off-by: Yair Zaslavsky <yzasl...@redhat.com>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MacPoolManager.java
1 file changed, 44 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/93/10193/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MacPoolManager.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MacPoolManager.java
index b1cda61..fcb2305 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MacPoolManager.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MacPoolManager.java
@@ -4,6 +4,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.ovirt.engine.core.common.AuditLogType;
 import org.ovirt.engine.core.common.businessentities.VmNetworkInterface;
@@ -31,11 +32,12 @@
     private final Set<String> mAvailableMacs = new HashSet<String>();
     private final Set<String> mAllocatedMacs = new HashSet<String>();
     private final Set<String> mAllocatedCustomMacs = new HashSet<String>();
-    private final Object mLocObj = new Object();
+    private final ReentrantReadWriteLock mLocObj = new 
ReentrantReadWriteLock();
     private boolean mInitialized = false;
 
     public void initialize() {
-        synchronized (mLocObj) {
+        mLocObj.writeLock().lock();
+        try {
             String ranges = Config.<String> 
GetValue(ConfigValues.MacPoolRanges);
             if (!"".equals(ranges)) {
                 try {
@@ -50,6 +52,8 @@
                 AddMac(iface.getMacAddress());
             }
             mInitialized = true;
+        } finally {
+            mLocObj.writeLock().unlock();
         }
     }
 
@@ -127,7 +131,8 @@
     public String allocateNewMac() {
         String mac = null;
         log.info("MacPoolManager::allocateNewMac entered");
-        synchronized (mLocObj) {
+        mLocObj.writeLock().lock();
+        try {
             if (!mInitialized) {
                 throw new 
VdcBLLException(VdcBllErrors.MAC_POOL_NOT_INITIALIZED);
             }
@@ -137,6 +142,8 @@
             Iterator<String> my = mAvailableMacs.iterator();
             mac = my.next();
             CommitNewMac(mac);
+        } finally {
+            mLocObj.writeLock().unlock();
         }
         log.infoFormat("MacPoolManager::allocateNewMac allocated mac = '{0}", 
mac);
         return mac;
@@ -159,16 +166,23 @@
 
     public void freeMac(String mac) {
         log.infoFormat("MacPoolManager::freeMac(mac = '{0}') - entered", mac);
-        synchronized (mLocObj) {
+        mLocObj.writeLock().lock();
+        try {
             if (!mInitialized) {
                 throw new 
VdcBLLException(VdcBllErrors.MAC_POOL_NOT_INITIALIZED);
             }
-            if (mAllocatedCustomMacs.contains(mac)) {
-                mAllocatedCustomMacs.remove(mac);
-            } else if (mAllocatedMacs.contains(mac)) {
-                mAllocatedMacs.remove(mac);
-                mAvailableMacs.add(mac);
-            }
+            internalFreeLock(mac);
+        } finally {
+            mLocObj.writeLock().unlock();
+        }
+    }
+
+    private void internalFreeLock(String mac) {
+        if (mAllocatedCustomMacs.contains(mac)) {
+            mAllocatedCustomMacs.remove(mac);
+        } else if (mAllocatedMacs.contains(mac)) {
+            mAllocatedMacs.remove(mac);
+            mAvailableMacs.add(mac);
         }
     }
 
@@ -180,7 +194,8 @@
      */
     public boolean AddMac(String mac) {
         boolean retVal = true;
-        synchronized (mLocObj) {
+        mLocObj.writeLock().lock();
+        try {
             if (mAllocatedMacs.contains(mac)) {
                 retVal = false;
             } else {
@@ -192,19 +207,34 @@
                     mAllocatedCustomMacs.add(mac);
                 }
             }
+        } finally {
+            mLocObj.writeLock().unlock();
         }
         return retVal;
     }
 
     public boolean IsMacInUse(String mac) {
-        synchronized (mLocObj) {
+        mLocObj.readLock().lock();
+        try {
             return mAllocatedMacs.contains(mac) || 
mAllocatedCustomMacs.contains(mac);
+        } finally {
+            mLocObj.readLock().unlock();
         }
     }
 
     public void freeMacs(List<String> macs) {
-        for (String mac : macs) {
-            freeMac(mac);
+        log.info("MacPoolManager::freeMacs - entered");
+        mLocObj.writeLock().lock();
+        try {
+            if (!mInitialized) {
+                throw new 
VdcBLLException(VdcBllErrors.MAC_POOL_NOT_INITIALIZED);
+            }
+            for (String mac : macs) {
+                internalFreeLock(mac);
+            }
+
+        } finally {
+            mLocObj.writeLock().unlock();
         }
     }
 


--
To view, visit http://gerrit.ovirt.org/10193
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I62fa797fbf286513183f0a3c3409cd79bdc25341
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches