Arik Hadas has uploaded a new change for review. Change subject: core: keep locks for whole disk migration process ......................................................................
core: keep locks for whole disk migration process In order to solve the race between VM migration and storage commands, the storage commands should keep the VM lock for their entine execution. The VM lock is now being hold for the whole images moving/copying stage of live migration, thus preventing the race with VM migration in that time. note that it's not enough - there's still a race as the VM lock is not being hold for the entire live migration process but that would be handled in a different patch. Change-Id: I89acf9f66bce647a7ecb2dd407e51a1ba03f4e17 Bug-Url: https://bugzilla.redhat.com/ Signed-off-by: Arik Hadas <aha...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateDiskCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 7 files changed, 26 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/30/18530/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java index 573e00c..ee2eb3b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java @@ -39,12 +39,13 @@ @DisableInPrepareMode @NonTransactiveCommandAttribute -@LockIdNameAttribute +@LockIdNameAttribute(isReleaseAtEndOfExecute = false) public class MoveOrCopyDiskCommand<T extends MoveOrCopyImageGroupParameters> extends CopyImageGroupCommand<T> implements QuotaStorageDependent { private List<PermissionSubject> cachedPermsList; private List<VM> cachedListVms; + private String cachedDiskIsBeingMigratedMessage; public MoveOrCopyDiskCommand(T parameters) { super(parameters); @@ -368,7 +369,7 @@ if (getParameters().getOperation() == ImageOperation.Copy) { if (!Guid.Empty.equals(getVmTemplateId())) { return Collections.singletonMap(getVmTemplateId().toString(), - LockMessagesMatchUtil.makeLockingPair(LockingGroup.TEMPLATE, VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED)); + LockMessagesMatchUtil.makeLockingPair(LockingGroup.TEMPLATE, getDiskIsBeingMigratedMessage())); } } else { List<VM> vmsForDisk = getVmsForDiskId(); @@ -376,7 +377,7 @@ Map<String, Pair<String, String>> lockMap = new HashMap<>(); for (VM currVm : vmsForDisk) { lockMap.put(currVm.getId().toString(), - LockMessagesMatchUtil.makeLockingPair(LockingGroup.VM, VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED)); + LockMessagesMatchUtil.makeLockingPair(LockingGroup.VM, getDiskIsBeingMigratedMessage())); } return lockMap; } @@ -388,7 +389,20 @@ protected Map<String, Pair<String, String>> getExclusiveLocks() { return Collections.singletonMap( (getImage() != null ? getImage().getId() : Guid.Empty).toString(), - LockMessagesMatchUtil.makeLockingPair(LockingGroup.DISK, VdcBllMessages.ACTION_TYPE_FAILED_OBJECT_LOCKED)); + LockMessagesMatchUtil.makeLockingPair(LockingGroup.DISK, getDiskIsBeingMigratedMessage())); + } + + private String getDiskIsBeingMigratedMessage() { + if (cachedDiskIsBeingMigratedMessage == null) { + StringBuilder builder = new StringBuilder(VdcBllMessages.ACTION_TYPE_FAILED_DISK_IS_BEING_MIGRATED.name()); + if (getImage() != null) { + builder.append(String.format("$DiskName %1$s", getDiskAlias())); + } + builder.append(String.format("$OperationType %1$s", + ImageOperation.Move == getParameters().getOperation() ? "moved" : "copied")); + cachedDiskIsBeingMigratedMessage = builder.toString(); + } + return cachedDiskIsBeingMigratedMessage; } public String getDiskAlias() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateDiskCommand.java index 49ca9bc..ffc1d42 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateDiskCommand.java @@ -22,7 +22,7 @@ import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; @NonTransactiveCommandAttribute -@LockIdNameAttribute +@LockIdNameAttribute(isReleaseAtEndOfExecute = false) public class LiveMigrateDiskCommand<T extends LiveMigrateDiskParameters> extends MoveOrCopyDiskCommand<T> implements TaskHandlerCommand<LiveMigrateDiskParameters> { public LiveMigrateDiskCommand(T parameters) { diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index 52ca36c..f3f5c58 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -547,6 +547,7 @@ ACTION_TYPE_FAILED_SNAPSHOT_IS_BEING_TAKEN_FOR_VM(ErrorType.CONFLICT), ACTION_TYPE_FAILED_DISK_IS_USED_FOR_CREATE_VM(ErrorType.CONFLICT), ACTION_TYPE_FAILED_DISK_IS_BEING_REMOVED(ErrorType.CONFLICT), + ACTION_TYPE_FAILED_DISK_IS_BEING_MIGRATED(ErrorType.CONFLICT), ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED(ErrorType.CONFLICT), ACTION_TYPE_FAILED_VM_IS_BEING_IMPORTED(ErrorType.CONFLICT), ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_REMOVED(ErrorType.CONFLICT), diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index a1bd1f6..d2bf511 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -564,6 +564,7 @@ ACTION_TYPE_FAILED_SNAPSHOT_IS_BEING_TAKEN_FOR_VM=Cannot ${action} ${type}. Snapshot is currently being created for VM ${VmName}. ACTION_TYPE_FAILED_DISK_IS_USED_FOR_CREATE_VM=Cannot ${action} ${type}. This disk is currently in use to create VM ${VmName}. ACTION_TYPE_FAILED_DISK_IS_BEING_REMOVED=Cannot ${action} ${type}. Disk ${DiskName} is being removed. +ACTION_TYPE_FAILED_DISK_IS_BEING_MIGRATED=Cannot ${action} ${type}. Disk ${DiskName} is being ${OperationType}. ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED=Cannot ${action} ${type}. Template ${TemplateName} is being exported. ACTION_TYPE_FAILED_VM_IS_BEING_IMPORTED=Cannot ${action} ${type}. VM ${VmName} is being imported. ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_REMOVED=Cannot ${action} ${type}. Template ${TemplateName} is being removed. diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index c5548f5..c6b59d9 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -1534,6 +1534,9 @@ @DefaultStringValue("Cannot ${action} ${type}. Disk ${DiskName} is being removed.") String ACTION_TYPE_FAILED_DISK_IS_BEING_REMOVED(); + @DefaultStringValue("Cannot ${action} ${type}. Disk ${DiskName} is being ${OperationType}.") + String ACTION_TYPE_FAILED_DISK_IS_BEING_MIGRATED(); + @DefaultStringValue("Cannot ${action} ${type}. Template ${TemplateName} is being exported.") String ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED(); diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 41feda9..d580c48 100644 --- a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -550,6 +550,7 @@ ACTION_TYPE_FAILED_SNAPSHOT_IS_BEING_TAKEN_FOR_VM=Cannot ${action} ${type}. Snapshot is currently being created for VM ${VmName}. ACTION_TYPE_FAILED_DISK_IS_USED_FOR_CREATE_VM=Cannot ${action} ${type}. This disk is currently in use to create VM ${VmName}. ACTION_TYPE_FAILED_DISK_IS_BEING_REMOVED=Cannot ${action} ${type}. Disk ${DiskName} is being removed. +ACTION_TYPE_FAILED_DISK_IS_BEING_MIGRATED=Cannot ${action} ${type}. Disk ${DiskName} is being ${OperationType}. ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED=Cannot ${action} ${type}. Template ${TemplateName} is being exported. ACTION_TYPE_FAILED_VM_IS_BEING_IMPORTED=Cannot ${action} ${type}. VM ${VmName} is being imported. ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_REMOVED=Cannot ${action} ${type}. Template ${TemplateName} is being removed. diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 4ba4b73..2282c55 100644 --- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -568,6 +568,7 @@ ACTION_TYPE_FAILED_SNAPSHOT_IS_BEING_TAKEN_FOR_VM=Cannot ${action} ${type}. Snapshot is currently being created for VM ${VmName}. ACTION_TYPE_FAILED_DISK_IS_USED_FOR_CREATE_VM=Cannot ${action} ${type}. This disk is currently in use to create VM ${VmName}. ACTION_TYPE_FAILED_DISK_IS_BEING_REMOVED=Cannot ${action} ${type}. Disk ${DiskName} is being removed. +ACTION_TYPE_FAILED_DISK_IS_BEING_MIGRATED=Cannot ${action} ${type}. Disk ${DiskName} is being ${OperationType}. ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED=Cannot ${action} ${type}. Template ${TemplateName} is being exported. ACTION_TYPE_FAILED_VM_IS_BEING_IMPORTED=Cannot ${action} ${type}. VM ${VmName} is being imported. ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_REMOVED=Cannot ${action} ${type}. Template ${TemplateName} is being removed. -- To view, visit http://gerrit.ovirt.org/18530 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I89acf9f66bce647a7ecb2dd407e51a1ba03f4e17 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
