Moti Asayag has uploaded a new change for review. Change subject: engine: Prevents duplicates parameters in multiple actions ......................................................................
engine: Prevents duplicates parameters in multiple actions The multiple action runner api serves as a method for invoking multiple actions in a single call to the engine. However, it turns out that due to a mysterious bug, in few flows the UI provides the same paramter more than once which might lead to unexpected behavior (i.e. infinite locked vm). By replacing the parameters collection type to list we prevents that from happen, and each parameter class will implement its own euqals() and hasCode() method to enforce the uniqueness. Change-Id: Ic874d31535d2189f934d629e689aa7a534c165d5 Signed-off-by: Moti Asayag <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromPoolRunner.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainsMultipleActionRunner.java 3 files changed, 16 insertions(+), 11 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/02/36102/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java index 834c82b..cc0e015 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MultipleActionsRunner.java @@ -1,7 +1,9 @@ package org.ovirt.engine.core.bll; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.Callable; import org.ovirt.engine.core.bll.context.CommandContext; @@ -20,7 +22,7 @@ private final static int CONCURRENT_ACTIONS = 10; private VdcActionType actionType = VdcActionType.Unknown; - private final List<VdcActionParametersBase> parameters; + private final Set<VdcActionParametersBase> parameters; private final ArrayList<CommandBase<?>> commands = new ArrayList<CommandBase<?>>(); protected boolean isInternal; private boolean isWaitForResult = false; @@ -34,12 +36,12 @@ public MultipleActionsRunner(VdcActionType actionType, List<VdcActionParametersBase> parameters, CommandContext commandContext, boolean isInternal) { this.actionType = actionType; - this.parameters = parameters; this.isInternal = isInternal; this.commandContext = commandContext; + this.parameters = new LinkedHashSet<>(parameters); } - protected List<VdcActionParametersBase> getParameters() { + protected Set<VdcActionParametersBase> getParameters() { return parameters; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromPoolRunner.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromPoolRunner.java index 1bded9b..282b14f 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromPoolRunner.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromPoolRunner.java @@ -1,5 +1,6 @@ package org.ovirt.engine.core.bll; +import java.util.Iterator; import java.util.List; import org.ovirt.engine.core.bll.context.CommandContext; @@ -62,9 +63,9 @@ } private RemoveVmFromPoolParameters getFirstParam() { - List<VdcActionParametersBase> parameters = getParameters(); - if (parameters != null && parameters.size() != 0) { - VdcActionParametersBase param = parameters.get(0); + Iterator<VdcActionParametersBase> iterator = getParameters() == null ? null : getParameters().iterator(); + if (iterator != null && iterator.hasNext()) { + VdcActionParametersBase param = iterator.next(); if (param instanceof RemoveVmFromPoolParameters) { return ((RemoveVmFromPoolParameters) param); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainsMultipleActionRunner.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainsMultipleActionRunner.java index 46c3f79..e50a20a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainsMultipleActionRunner.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AttachStorageDomainsMultipleActionRunner.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.Iterator; import org.ovirt.engine.core.bll.Backend; import org.ovirt.engine.core.bll.CommandBase; @@ -12,8 +13,8 @@ import org.ovirt.engine.core.common.action.VdcActionParametersBase; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VdcReturnValueBase; -import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; import org.ovirt.engine.core.common.businessentities.StoragePool; +import org.ovirt.engine.core.common.businessentities.StoragePoolStatus; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.utils.threadpool.ThreadPoolUtil; @@ -26,17 +27,18 @@ @Override public ArrayList<VdcReturnValueBase> execute() { - if (getParameters().size() > 0) { + Iterator<VdcActionParametersBase> iterator = getParameters() == null ? null : getParameters().iterator(); + if (iterator != null && iterator.hasNext()) { + VdcActionParametersBase parameter = iterator.next(); StoragePool pool = DbFacade.getInstance().getStoragePoolDao().get( - ((StorageDomainPoolParametersBase) getParameters().get(0)).getStoragePoolId()); + ((StorageDomainPoolParametersBase) parameter).getStoragePoolId()); if (pool.getStatus() == StoragePoolStatus.Uninitialized) { ArrayList<Guid> storageDomainIds = new ArrayList<Guid>(); for (VdcActionParametersBase param : getParameters()) { storageDomainIds.add(((StorageDomainPoolParametersBase) param).getStorageDomainId()); } ArrayList<VdcActionParametersBase> parameters = new ArrayList<VdcActionParametersBase>(); - parameters.add(new StoragePoolWithStoragesParameter(pool, storageDomainIds, getParameters().get(0) - .getSessionId())); + parameters.add(new StoragePoolWithStoragesParameter(pool, storageDomainIds, parameter.getSessionId())); if (isInternal) { return Backend.getInstance().runInternalMultipleActions(VdcActionType.AddStoragePoolWithStorages, parameters); -- To view, visit http://gerrit.ovirt.org/36102 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic874d31535d2189f934d629e689aa7a534c165d5 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Moti Asayag <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
