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