Hello Liron Aravot,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/31272

to review the following change.

Change subject: core: drop ConnectSingleAsyncOperation
......................................................................

core: drop ConnectSingleAsyncOperation

This patch drops the ConnectSingleAsyncOperation and it's factory with
the simple use of callables.
The ConnectSingleAsyncOperation is more complicated, harder to maintain
and limited than the simple use of callables (for example, results can't
be returned from it at the moment).

There's currently no need for that infrastructure, we can execute the
needed operations from a helper or a helper method.

Change-Id: I825d80da10ef3d22de7cd68a68d854374e6c136c
Related-To-Bug-Url: https://bugzilla.redhat.com/1115845
Signed-off-by: Liron Aravot <lara...@redhat.com>
---
D 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectSingleAsyncOperation.java
D 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectSingleAsyncOperationFactory.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java
4 files changed, 28 insertions(+), 45 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/72/31272/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectSingleAsyncOperation.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectSingleAsyncOperation.java
deleted file mode 100644
index 2b168a6..0000000
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectSingleAsyncOperation.java
+++ /dev/null
@@ -1,30 +0,0 @@
-package org.ovirt.engine.core.bll.storage;
-
-import java.util.ArrayList;
-import org.ovirt.engine.core.common.businessentities.VDS;
-import org.ovirt.engine.core.common.businessentities.StorageDomain;
-import org.ovirt.engine.core.common.businessentities.StoragePool;
-import org.ovirt.engine.core.utils.log.Log;
-import org.ovirt.engine.core.utils.log.LogFactory;
-
-public class ConnectSingleAsyncOperation extends 
ActivateDeactivateSingleAsyncOperation {
-
-    public ConnectSingleAsyncOperation(ArrayList<VDS> vdss, StorageDomain 
domain, StoragePool storagePool) {
-        super(vdss, domain, storagePool);
-    }
-
-    @Override
-    public void execute(int iterationId) {
-        VDS vds = getVdss().get(iterationId);
-
-        try {
-            
StorageHelperDirector.getInstance().getItem(getStorageDomain().getStorageType())
-                    .connectStorageToDomainByVdsId(getStorageDomain(), 
vds.getId());
-        } catch (RuntimeException e) {
-            log.errorFormat("Failed to connect host {0} to storage domain 
(name: {1}, id: {2}). Exception: {3}",
-                    vds.getName(), getStorageDomain().getName(), 
getStorageDomain().getId(), e);
-        }
-    }
-
-    private static final Log log = 
LogFactory.getLog(ConnectSingleAsyncOperation.class);
-}
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectSingleAsyncOperationFactory.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectSingleAsyncOperationFactory.java
deleted file mode 100644
index 6979836..0000000
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ConnectSingleAsyncOperationFactory.java
+++ /dev/null
@@ -1,10 +0,0 @@
-package org.ovirt.engine.core.bll.storage;
-
-import org.ovirt.engine.core.utils.ISingleAsyncOperation;
-
-public class ConnectSingleAsyncOperationFactory extends 
ActivateDeactivateSingleAsyncOperationFactory {
-    @Override
-    public ISingleAsyncOperation createSingleAsyncOperation() {
-        return new ConnectSingleAsyncOperation(getVdss(), getStorageDomain(), 
getStoragePool());
-    }
-}
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
index bef751a..8e77b79 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
@@ -5,6 +5,7 @@
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashSet;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.Callable;
@@ -27,6 +28,7 @@
 import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMapId;
 import org.ovirt.engine.core.common.businessentities.StorageServerConnections;
 import org.ovirt.engine.core.common.businessentities.StorageType;
+import org.ovirt.engine.core.common.businessentities.VDS;
 import org.ovirt.engine.core.common.config.Config;
 import org.ovirt.engine.core.common.config.ConfigValues;
 import org.ovirt.engine.core.common.errors.VdcBllMessages;
@@ -34,6 +36,7 @@
 import org.ovirt.engine.core.common.eventqueue.EventQueue;
 import org.ovirt.engine.core.common.eventqueue.EventResult;
 import org.ovirt.engine.core.common.eventqueue.EventType;
+import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.dal.dbbroker.DbFacade;
 import org.ovirt.engine.core.dao.BaseDiskDao;
@@ -50,6 +53,7 @@
 import org.ovirt.engine.core.utils.ejb.EjbUtils;
 import org.ovirt.engine.core.utils.linq.LinqUtils;
 import org.ovirt.engine.core.utils.linq.Predicate;
+import org.ovirt.engine.core.utils.threadpool.ThreadPoolUtil;
 import org.ovirt.engine.core.utils.transaction.TransactionMethod;
 import org.ovirt.engine.core.utils.transaction.TransactionSupport;
 
@@ -325,8 +329,28 @@
         }
     }
 
-    protected void connectAllHostsToPool() {
-        runSynchronizeOperation(new ConnectSingleAsyncOperationFactory());
+    protected List<Pair<Guid, Boolean>> connectAllHostsToPool() {
+        List<VDS> hostsInStatusUp = getAllRunningVdssInPool();
+        List<Callable<Pair<Guid, Boolean>>> callables = new LinkedList<>();
+        for (final VDS vds : hostsInStatusUp) {
+            callables.add(new Callable<Pair<Guid, Boolean>>() {
+                @Override
+                public Pair<Guid, Boolean> call() throws Exception {
+                    Pair<Guid, Boolean> toReturn = new Pair<>(vds.getId(), 
Boolean.FALSE);
+                    try {
+                        boolean connectResult = 
StorageHelperDirector.getInstance().getItem(getStorageDomain().getStorageType())
+                                
.connectStorageToDomainByVdsId(getStorageDomain(), vds.getId());
+                        toReturn.setSecond(connectResult);
+                    } catch (RuntimeException e) {
+                        log.errorFormat("Failed to connect host {0} to storage 
domain (name: {1}, id: {2}). Exception: {3}",
+                                vds.getName(), getStorageDomain().getName(), 
getStorageDomain().getId(), e);
+                    }
+                    return toReturn;
+                }
+            });
+        }
+
+        return ThreadPoolUtil.invokeAll(callables);
     }
 
     protected void disconnectAllHostsInPool() {
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java
index 84b4d4e..c6e0f82 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainToPoolCommandTest.java
@@ -12,6 +12,7 @@
 import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 import org.junit.ClassRule;
@@ -38,7 +39,6 @@
 import org.ovirt.engine.core.common.businessentities.StoragePoolIsoMapId;
 import org.ovirt.engine.core.common.businessentities.StoragePoolStatus;
 import org.ovirt.engine.core.common.businessentities.VDS;
-import org.ovirt.engine.core.common.businessentities.VDSStatus;
 import org.ovirt.engine.core.common.interfaces.VDSBrokerFrontend;
 import org.ovirt.engine.core.common.utils.Pair;
 import 
org.ovirt.engine.core.common.vdscommands.AttachStorageDomainVDSCommandParameters;
@@ -88,7 +88,7 @@
 
         doReturn(dbFacade).when(cmd).getDbFacade();
         doNothing().when(cmd).attemptToActivateDomain();
-
+        doReturn(Collections.emptyList()).when(cmd).connectAllHostsToPool();
         when(dbFacade.getStoragePoolIsoMapDao()).thenReturn(isoMapDAO);
         when(dbFacade.getStoragePoolDao()).thenReturn(storagePoolDAO);
         when(dbFacade.getVdsDao()).thenReturn(vdsDAO);
@@ -102,7 +102,6 @@
         when(storageDomainStaticDAO.get(any(Guid.class))).thenReturn(new 
StorageDomainStatic());
 
         doReturn(backendInternal).when(cmd).getBackend();
-        when(vdsDAO.getAllForStoragePoolAndStatus(any(Guid.class), 
any(VDSStatus.class))).thenReturn(new ArrayList<VDS>());
         
when(backendInternal.getResourceManager()).thenReturn(vdsBrokerFrontend);
         VdcReturnValueBase vdcReturnValue = new VdcReturnValueBase();
         vdcReturnValue.setSucceeded(true);


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I825d80da10ef3d22de7cd68a68d854374e6c136c
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.5
Gerrit-Owner: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to