Liron Aravot has uploaded a new change for review. Change subject: core: WIP : RemoveImageDisk - race when updating snapshots ovf (#828192) ......................................................................
core: WIP : RemoveImageDisk - race when updating snapshots ovf (#828192) https://bugzilla.redhat.com/show_bug.cgi?id=828192 when removing an image disk, it should be removed from all the snapshots it's contained within. the removal from the image snapshots includes an update to the snapshot ovf (saved at ovirt DB). the update is an read-update-write operation, so when two or more disks are removed from the same snapshot a race condition might occur. this patch adds a lock on the snapshot when performing the operations on the snapshot ovf to prevent the race condition. Change-Id: Iccb44f1aa9d204477955343167133849a4146753 Signed-off-by: Liron Aravot <lara...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java 1 file changed, 29 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/82/7482/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java index bc1cb4a..642912a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java @@ -2,9 +2,11 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.Map; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.RemoveImageParameters; @@ -22,6 +24,7 @@ import org.ovirt.engine.core.common.businessentities.async_tasks; import org.ovirt.engine.core.common.businessentities.image_storage_domain_map_id; import org.ovirt.engine.core.common.errors.VdcBLLException; +import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.vdscommands.DeleteImageGroupVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.common.vdscommands.VDSReturnValue; @@ -29,6 +32,7 @@ import org.ovirt.engine.core.compat.NGuid; import org.ovirt.engine.core.compat.TransactionScopeOption; import org.ovirt.engine.core.dao.VmDeviceDAO; +import org.ovirt.engine.core.utils.lock.EngineLock; import org.ovirt.engine.core.utils.ovf.OvfManager; import org.ovirt.engine.core.utils.ovf.OvfReaderException; import org.ovirt.engine.core.utils.transaction.TransactionMethod; @@ -168,18 +172,42 @@ for (DiskImage snapshotDisk : snapshotDisks) { NGuid vmSnapshotId = snapshotDisk.getvm_snapshot_id(); if (vmSnapshotId != null && !Guid.Empty.equals(vmSnapshotId.getValue())) { - Snapshot updated = + lockSnapshotWithWait(vmSnapshotId.getValue()); + final Snapshot updated = prepareSnapshotConfigWithoutImageSingleImage(vmSnapshotId.getValue(), snapshotDisk.getImageId()); if (updated != null) { result.add(updated); + TransactionSupport.executeInScope(TransactionScopeOption.Required, + new TransactionMethod<Object>() { + @Override + public Object runInTransaction() { + getSnapshotDao().update(updated); + return null; + } + }); } + freeSnapshotLock(); } } return result; } + private EngineLock snapshotEngineLock = new EngineLock(); + private Map<String, String> snapshotsExlusiveLockMap = new HashMap<String, String>(); + + private void lockSnapshotWithWait(Guid snapshotId) { + snapshotsExlusiveLockMap.clear(); + snapshotsExlusiveLockMap.put(snapshotId.toString(), LockingGroup.SNAPSHOT.name()); + snapshotEngineLock.setExclusiveLocks(snapshotsExlusiveLockMap); + getLockManager().acquireLockWait(snapshotEngineLock); + } + + private void freeSnapshotLock(){ + getLockManager().releaseLock(snapshotEngineLock); + } + /** * Prepare a single {@link Snapshot} object representing a snapshot of a given VM without the give disk. */ -- To view, visit http://gerrit.ovirt.org/7482 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iccb44f1aa9d204477955343167133849a4146753 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <lara...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
