Arik Hadas has uploaded a new change for review. Change subject: core: prevent removing vm from export domain while it is being imported ......................................................................
core: prevent removing vm from export domain while it is being imported While VM is being imported from export domain, there was no kind of lock that prevented other commands from modifing the VM. So while the import operation was on progress, the user could remove the VM which is obviously something we need to prevent. This patch prevents the scenario above from happening by modify the ImportVmCommand to take shared lock on the VM at the export domain for the whole execution of the command, and modify the RemoveVmFromImportExportCommand to take exclusive lock on the VM which is about to be removed. Change-Id: I5c34612f754375a9d15816e9972671e1b13023ad Bug-Url: https://bugzilla.redhat.com/733917 Signed-off-by: Arik Hadas <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromImportExportCommand.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, 44 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/55/16955/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java index 057ca9c..e4df76a 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java @@ -103,6 +103,7 @@ private final List<Guid> imageGuidList = new ArrayList<Guid>(); private final List<String> macsAdded = new ArrayList<String>(); private final SnapshotsManager snapshotsManager = new SnapshotsManager(); + public ImportVmCommand(ImportVmParameters parameters) { super(parameters); @@ -121,6 +122,14 @@ } @Override + protected Map<String, Pair<String, String>> getSharedLocks() { + return Collections.singletonMap(getVmId().toString(), + LockMessagesMatchUtil.makeLockingPair( + LockingGroup.REMOTE_VM, + getVmIsBeingImportedMessage())); + } + + @Override protected Map<String, Pair<String, String>> getExclusiveLocks() { if (!StringUtils.isBlank(getParameters().getVm().getName())) { return Collections.singletonMap(getParameters().getVm().getName(), @@ -129,6 +138,12 @@ return null; } + private String getVmIsBeingImportedMessage() { + StringBuilder builder = new StringBuilder(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_BEING_IMPORTED.name()); + builder.append(String.format("$VmName %1$s", getVmName())); + return builder.toString(); + } + protected ImportVmCommand(Guid commandId) { super(commandId); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromImportExportCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromImportExportCommand.java index 00bb9f1..e126bb7 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromImportExportCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromImportExportCommand.java @@ -25,9 +25,11 @@ import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.common.locks.LockingGroup; import org.ovirt.engine.core.common.queries.GetAllFromExportDomainQueryParameters; import org.ovirt.engine.core.common.queries.VdcQueryReturnValue; import org.ovirt.engine.core.common.queries.VdcQueryType; +import org.ovirt.engine.core.common.utils.Pair; import org.ovirt.engine.core.common.vdscommands.RemoveVMVDSCommandParameters; import org.ovirt.engine.core.common.vdscommands.VDSCommandType; import org.ovirt.engine.core.compat.Guid; @@ -35,6 +37,7 @@ import org.ovirt.engine.core.dal.dbbroker.DbFacade; @NonTransactiveCommandAttribute +@LockIdNameAttribute(isReleaseAtEndOfExecute = false) public class RemoveVmFromImportExportCommand<T extends RemoveVmFromImportExportParamenters> extends RemoveVmCommand<T> implements TaskHandlerCommand<RemoveVmFromImportExportParamenters>{ // this is needed since overriding getVmTemplate() @@ -42,12 +45,26 @@ public RemoveVmFromImportExportCommand(T parameters) { super(parameters); - super.setVmId(parameters.getVmId()); + setVmId(parameters.getVmId()); parameters.setEntityInfo(new EntityInfo(VdcObjectType.VM, parameters.getVmId())); setStorageDomainId(parameters.getStorageDomainId()); } @Override + protected Map<String, Pair<String, String>> getExclusiveLocks() { + return Collections.singletonMap(getVmId().toString(), + LockMessagesMatchUtil.makeLockingPair( + LockingGroup.REMOTE_VM, + getVmIsBeingRemovedFromExportDomainMessage())); + } + + private String getVmIsBeingRemovedFromExportDomainMessage() { + StringBuilder builder = new StringBuilder(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_BEING_REMOVED_FROM_EXPORT_DOMAIN.name()); + builder.append(String.format("$VmName %1$s", getVmName())); + return builder.toString(); + } + + @Override protected boolean canDoAction() { StorageDomain storage = DbFacade.getInstance().getStorageDomainDao().getForStoragePool( getParameters().getStorageDomainId(), getParameters().getStoragePoolId()); 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 0b896f6..f400f50 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 @@ -529,7 +529,9 @@ ACTION_TYPE_FAILED_DISK_IS_USED_FOR_CREATE_VM(ErrorType.CONFLICT), ACTION_TYPE_FAILED_DISK_IS_BEING_REMOVED(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), + ACTION_TYPE_FAILED_VM_IS_BEING_REMOVED_FROM_EXPORT_DOMAIN(ErrorType.CONFLICT), ACTION_TYPE_FAILED_DISK_IS_USED_BY_GET_ALIGNMENT(ErrorType.CONFLICT), VM_OR_TEMPLATE_ILLEGAL_PRIORITY_VALUE(ErrorType.BAD_PARAMETERS), 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 9c9264a..8c54de8 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -550,6 +550,7 @@ 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_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. ACTION_TYPE_FAILED_DISK_IS_USED_BY_GET_ALIGNMENT=Cannot ${action} ${type}. Disk ${DiskAlias} alignment is currently being determined. NETWORK_BOND_HAVE_ATTACHED_VLANS=Bond attached to vlan, remove bonds vlan first 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 a8dfb0f..422a340 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 @@ -1480,9 +1480,15 @@ @DefaultStringValue("Cannot ${action} ${type}. Template ${TemplateName} is being exported.") String ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED(); + @DefaultStringValue("Cannot ${action} ${type}. VM ${VmName} is being imported.") + String ACTION_TYPE_FAILED_VM_IS_BEING_IMPORTED(); + @DefaultStringValue("Cannot ${action} ${type}. Template ${TemplateName} is being removed.") String ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_REMOVED(); + @DefaultStringValue("Cannot ${action} ${type}. VM ${VmName} is being removed from export domain.") + String ACTION_TYPE_FAILED_VM_IS_BEING_REMOVED_FROM_EXPORT_DOMAIN(); + @DefaultStringValue("Bond attached to vlan, remove bonds vlan first") String NETWORK_BOND_HAVE_ATTACHED_VLANS(); 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 63373a5..9ea5b1d 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 @@ -535,6 +535,7 @@ 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_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. ACTION_TYPE_FAILED_DISK_IS_USED_BY_GET_ALIGNMENT=Cannot ${action} ${type}. Disk ${DiskAlias} alignment is currently being determined. NETWORK_BOND_HAVE_ATTACHED_VLANS=Bond attached to vlan, remove bonds vlan first 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 839defb..44895b3 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 @@ -552,6 +552,7 @@ 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_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. ACTION_TYPE_FAILED_DISK_IS_USED_BY_GET_ALIGNMENT=Cannot ${action} ${type}. Disk ${DiskAlias} alignment is currently being determined. NETWORK_BOND_HAVE_ATTACHED_VLANS=Bond attached to vlan, remove bonds vlan first -- To view, visit http://gerrit.ovirt.org/16955 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5c34612f754375a9d15816e9972671e1b13023ad Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
