Arik Hadas has uploaded a new change for review. Change subject: core: filter duplicate requests to run the same vm ......................................................................
core: filter duplicate requests to run the same vm If engine gets multiple requests to run the same VM at the same time (with MultipleActionRunner), we might get into a situation where the VM does not run and kept locked. Say we got 2 run requests for the same VM: - The first command acquires the lock for the VM - The second command fails to acquire the lock - The can-do-action phase of the first command succeed - We sort the commands before invoking them. As part of the sort, we map Id of VM to its run command. Since we iterate the commands in the same order they were received, we end up with mapping the second command - Since the second command didn't manage to acquire the lock, it won't run Since the first command does not run, it won't release its lock. The solution is to filter the given parameters from duplicate requests to run the same VM. We now override the hashCode and equals methods in RunVmParams class so if we will get duplicate instances, they will be filtered while we insert them to a Set in MultipleActionRunner. Change-Id: I91320cfd50fc7a12b01afae2885a783c0516a6df Bug-Url: https://bugzilla.redhat.com/1174815 Signed-off-by: Arik Hadas <aha...@redhat.com> --- M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RunVmOnceParams.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RunVmParams.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VmOperationParameterBase.java 3 files changed, 107 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/48/36248/1 diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RunVmOnceParams.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RunVmOnceParams.java index a647801..e5274ea 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RunVmOnceParams.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RunVmOnceParams.java @@ -1,6 +1,7 @@ package org.ovirt.engine.core.common.action; import org.ovirt.engine.core.common.config.ConfigValues; +import org.ovirt.engine.core.common.utils.ObjectUtils; import org.ovirt.engine.core.common.validation.annotation.NullOrStringContainedInConfigValueList; import org.ovirt.engine.core.common.validation.group.StartEntity; import org.ovirt.engine.core.common.businessentities.VmInit; @@ -27,6 +28,41 @@ super(vmId); } + @Override + public int hashCode() { + final int prime = 31; + int result = super.hashCode(); + result = prime * result + ((sysPrepDomainName == null) ? 0 : sysPrepDomainName.hashCode()); + result = prime * result + ((sysPrepUserName == null) ? 0 : sysPrepUserName.hashCode()); + result = prime * result + ((sysPrepPassword == null) ? 0 : sysPrepPassword.hashCode()); + result = prime * result + ((vmInit == null) ? 0 : vmInit.hashCode()); + result = prime * result + ((destinationVdsId == null) ? 0 : destinationVdsId.hashCode()); + result = prime * result + ((customEmulatedMachine == null) ? 0 : customEmulatedMachine.hashCode()); + result = prime * result + ((customCpuName == null) ? 0 : customCpuName.hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + + if (!(obj instanceof RunVmOnceParams)) { + return false; + } + + RunVmOnceParams other = (RunVmOnceParams) obj; + return super.equals(obj) + && ObjectUtils.objectsEqual(sysPrepDomainName, other.sysPrepDomainName) + && ObjectUtils.objectsEqual(sysPrepUserName, other.sysPrepUserName) + && ObjectUtils.objectsEqual(sysPrepPassword, other.sysPrepPassword) + && ObjectUtils.objectsEqual(vmInit, other.vmInit) + && ObjectUtils.objectsEqual(destinationVdsId, other.destinationVdsId) + && ObjectUtils.objectsEqual(customEmulatedMachine, other.customEmulatedMachine) + && ObjectUtils.objectsEqual(customCpuName, other.customCpuName); + } + public void setSysPrepDomainName(String sysPrepDomainName) { this.sysPrepDomainName = sysPrepDomainName; } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RunVmParams.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RunVmParams.java index a54334b..a230b7a 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RunVmParams.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RunVmParams.java @@ -4,6 +4,7 @@ import org.ovirt.engine.core.common.businessentities.InitializationType; import org.ovirt.engine.core.common.businessentities.VmPayload; import org.ovirt.engine.core.common.businessentities.aaa.DbUser; +import org.ovirt.engine.core.common.utils.ObjectUtils; import org.ovirt.engine.core.compat.Guid; public class RunVmParams extends VmOperationParameterBase { @@ -41,6 +42,71 @@ acpiEnable = true; } + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + + if (!(obj instanceof RunVmParams)) { + return false; + } + + RunVmParams other = (RunVmParams) obj; + return bootSequence == other.bootSequence + && ObjectUtils.objectsEqual(getVmId(), other.getVmId()) + && ObjectUtils.objectsEqual(diskPath, other.diskPath) + && kvmEnable == other.kvmEnable + && ObjectUtils.objectsEqual(runAndPause, other.runAndPause) + && ObjectUtils.objectsEqual(useVnc, other.useVnc) + && acpiEnable == other.acpiEnable + && ObjectUtils.objectsEqual(win2kHackEnable, other.win2kHackEnable) + && ObjectUtils.objectsEqual(customProperties, other.customProperties) + && ObjectUtils.objectsEqual(floppyPath, other.floppyPath) + && ObjectUtils.objectsEqual(clientIp, other.clientIp) + && ObjectUtils.objectsEqual(requestingUser, other.requestingUser) + && initializationType == other.initializationType + && ObjectUtils.objectsEqual(runAsStateless, other.runAsStateless) + && ObjectUtils.objectsEqual(initrdUrl, other.initrdUrl) + && ObjectUtils.objectsEqual(kernelUrl, other.kernelUrl) + && ObjectUtils.objectsEqual(kernelParams, other.kernelParams) + && ObjectUtils.objectsEqual(payload, other.payload) + && balloonEnabled == other.balloonEnabled + && cpuShares == other.cpuShares + && ObjectUtils.objectsEqual(bootMenuEnabled, other.bootMenuEnabled) + && ObjectUtils.objectsEqual(spiceFileTransferEnabled, other.spiceFileTransferEnabled) + && ObjectUtils.objectsEqual(spiceCopyPasteEnabled, other.spiceCopyPasteEnabled); + } + + @Override + public int hashCode() { + final int prime = 31; + int result = super.hashCode(); + result = prime * result + ((bootSequence == null) ? 0 : bootSequence.hashCode()); + result = prime * result + ((diskPath == null) ? 0 : diskPath.hashCode()); + result = prime * result + (kvmEnable ? 1231 : 1237); + result = prime * result + ((runAndPause == null) ? 0 : runAndPause.hashCode()); + result = prime * result + ((useVnc == null) ? 0 : useVnc.hashCode()); + result = prime * result + (acpiEnable ? 1231 : 1237); + result = prime * result + ((win2kHackEnable == null) ? 0 : win2kHackEnable.hashCode()); + result = prime * result + ((customProperties == null) ? 0 : customProperties.hashCode()); + result = prime * result + ((floppyPath == null) ? 0 : floppyPath.hashCode()); + result = prime * result + ((clientIp == null) ? 0 : clientIp.hashCode()); + result = prime * result + ((requestingUser == null) ? 0 : requestingUser.hashCode()); + result = prime * result + ((initializationType == null) ? 0 : initializationType.hashCode()); + result = prime * result + ((runAsStateless == null) ? 0 : runAsStateless.hashCode()); + result = prime * result + ((initrdUrl == null) ? 0 : initrdUrl.hashCode()); + result = prime * result + ((kernelUrl == null) ? 0 : kernelUrl.hashCode()); + result = prime * result + ((kernelParams == null) ? 0 : kernelParams.hashCode()); + result = prime * result + ((payload == null) ? 0 : payload.hashCode()); + result = prime * result + (balloonEnabled ? 1231 : 1237); + result = prime * result + cpuShares; + result = prime * result + ((bootMenuEnabled == null) ? 0 : bootMenuEnabled.hashCode()); + result = prime * result + ((spiceFileTransferEnabled == null) ? 0 : spiceFileTransferEnabled.hashCode()); + result = prime * result + ((spiceCopyPasteEnabled == null) ? 0 : spiceCopyPasteEnabled.hashCode()); + return result; + } + public BootSequence getBootSequence() { return bootSequence; } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VmOperationParameterBase.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VmOperationParameterBase.java index 4dded34..8916b26 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VmOperationParameterBase.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VmOperationParameterBase.java @@ -32,4 +32,9 @@ public void setQuotaId(Guid value) { quotaId = value; } + + @Override + public int hashCode() { + return vmId == null ? 0 : vmId.hashCode(); + } } -- To view, visit http://gerrit.ovirt.org/36248 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I91320cfd50fc7a12b01afae2885a783c0516a6df Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5 Gerrit-Owner: Arik Hadas <aha...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches