Hello Moti Asayag,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/36247
to review the following change.
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 parameter more than once which
might lead to an unexpected behavior (i.e. infinite locked vm).
By replacing the parameters collection type to set we prevent
that from happen, and each parameter class will implement its
own euqals() and hasCode() methods to enforce the uniqueness.
Change-Id: Ic874d31535d2189f934d629e689aa7a534c165d5
Bug-Url: https://bugzilla.redhat.com/1174815
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, 20 insertions(+), 12 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/47/36247/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 ac516e2..0e1bbda 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..b0d6b97 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<?> iterator = getParameters() == null ? null :
getParameters().iterator();
+ if (iterator != null && iterator.hasNext()) {
+ Object 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..8ae4f52 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,21 @@
@Override
public ArrayList<VdcReturnValueBase> execute() {
- if (getParameters().size() > 0) {
- StoragePool pool = DbFacade.getInstance().getStoragePoolDao().get(
- ((StorageDomainPoolParametersBase)
getParameters().get(0)).getStoragePoolId());
+ Iterator<?> iterator = getParameters() == null ? null :
getParameters().iterator();
+ Object parameter = iterator != null && iterator.hasNext() ?
iterator.next() : null;
+
+ if (parameter instanceof StorageDomainPoolParametersBase) {
+ StorageDomainPoolParametersBase storagePoolParameter =
(StorageDomainPoolParametersBase) parameter;
+ StoragePool pool =
DbFacade.getInstance().getStoragePoolDao().get(storagePoolParameter.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,
+ storagePoolParameter.getSessionId()));
if (isInternal) {
return
Backend.getInstance().runInternalMultipleActions(VdcActionType.AddStoragePoolWithStorages,
parameters);
--
To view, visit http://gerrit.ovirt.org/36247
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic874d31535d2189f934d629e689aa7a534c165d5
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.5
Gerrit-Owner: Arik Hadas <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
