Daniel Erez has uploaded a new change for review. Change subject: core: support Cinder disk removal ......................................................................
core: support Cinder disk removal * Added 'deleteVolume' method to OpenStackVolumeProviderProxy. * Added 'deleteDisk' method to CinderBroker. * Added RemoveCinderDiskCommand/RemoveCinderDiskCallback. Change-Id: I7a3803072ade96a241e8f71a030360f4168a1b71 Bug-Url: https://bugzilla.redhat.com/1185826 Signed-off-by: Daniel Erez <de...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/storage/OpenStackVolumeProviderProxy.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/CinderBroker.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveCinderDiskCommand.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveCinderDiskCommandCallback.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java M backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties 8 files changed, 293 insertions(+), 13 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/35/39135/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java index d124de1..1885dd6 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java @@ -6,6 +6,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.context.CommandContext; @@ -13,6 +15,7 @@ import org.ovirt.engine.core.bll.quota.QuotaStorageConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaStorageDependent; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; +import org.ovirt.engine.core.bll.tasks.CommandCoordinatorUtil; import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.bll.validator.storage.DiskImagesValidator; import org.ovirt.engine.core.bll.validator.storage.DiskValidator; @@ -47,8 +50,10 @@ import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.TransactionScopeOption; import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.dao.BaseDiskDao; import org.ovirt.engine.core.dao.DiskDao; import org.ovirt.engine.core.dao.DiskImageDAO; +import org.ovirt.engine.core.dao.DiskImageDynamicDAO; import org.ovirt.engine.core.dao.VmDeviceDAO; import org.ovirt.engine.core.utils.transaction.TransactionMethod; import org.ovirt.engine.core.utils.transaction.TransactionSupport; @@ -65,6 +70,11 @@ public RemoveDiskCommand(T parameters) { this(parameters, null); + } + + public RemoveDiskCommand(T parameters, CommandContext commandContext) { + super(parameters, commandContext); + setStorageDomainId(getParameters().getStorageDomainId()); } public RemoveDiskCommand(T parameters, CommandContext commandContext) { @@ -115,7 +125,8 @@ // currently, only images have specific checks. // In the future, if LUNs get specific checks, // or additional storage types are added, other else-if clauses should be added. - if (getDisk().getDiskStorageType() == DiskStorageType.IMAGE) { + if (getDisk().getDiskStorageType() == DiskStorageType.IMAGE || + getDisk().getDiskStorageType() == DiskStorageType.CINDER) { return canRemoveDiskBasedOnImageStorageCheck(); } @@ -278,23 +289,41 @@ return DbFacade.getInstance().getDiskImageDao(); } - protected DiskDao getDiskDao() { + public DiskDao getDiskDao() { return DbFacade.getInstance().getDiskDao(); } @Override protected void executeCommand() { - if (getDisk().getDiskStorageType() == DiskStorageType.IMAGE) { - VdcReturnValueBase vdcReturnValue = - runInternalActionWithTasksContext(VdcActionType.RemoveImage, - buildRemoveImageParameters(getDiskImage())); - if (vdcReturnValue.getSucceeded()) { - incrementVmsGeneration(); - getReturnValue().getVdsmTaskIdList().addAll(vdcReturnValue.getInternalVdsmTaskIdList()); - setSucceeded(true); - } - } else { - removeLunDisk(); + switch (getDisk().getDiskStorageType()) { + case IMAGE: + VdcReturnValueBase vdcReturnValue = + runInternalActionWithTasksContext(VdcActionType.RemoveImage, + buildRemoveImageParameters(getDiskImage())); + if (vdcReturnValue.getSucceeded()) { + incrementVmsGeneration(); + getReturnValue().getVdsmTaskIdList().addAll(vdcReturnValue.getInternalVdsmTaskIdList()); + setSucceeded(true); + } + break; + case LUN: + removeLunDisk(); + break; + case CINDER: + Future<VdcReturnValueBase> future = CommandCoordinatorUtil.executeAsyncCommand( + VdcActionType.RemoveCinderDisk, + new RemoveDiskParameters(getParameters().getDiskId()), + cloneContextAndDetachFromParent()); + try { + setReturnValue(future.get()); + setSucceeded(getReturnValue().getSucceeded()); + } catch (InterruptedException | ExecutionException e) { + log.error("Error removing Cinder disk '{}': {}", + getDiskImage().getDiskAlias(), + e.getMessage()); + log.debug("Exception", e); + } + break; } } @@ -349,6 +378,9 @@ if (getDisk().getDiskStorageType() == DiskStorageType.LUN) { return getSucceeded() ? AuditLogType.USER_FINISHED_REMOVE_DISK_NO_DOMAIN : AuditLogType.USER_FINISHED_FAILED_REMOVE_DISK_NO_DOMAIN; + } else if (getDisk().getDiskStorageType() == DiskStorageType.CINDER) { + return getSucceeded() ? AuditLogType.USER_REMOVE_DISK_INITIATED + : AuditLogType.USER_FINISHED_FAILED_REMOVE_DISK; } return getSucceeded() ? AuditLogType.USER_FINISHED_REMOVE_DISK : AuditLogType.USER_FINISHED_FAILED_REMOVE_DISK; @@ -478,4 +510,12 @@ @Override public void addQuotaPermissionSubject(List<PermissionSubject> quotaPermissionList) { } + + protected BaseDiskDao getBaseDiskDao() { + return getDbFacade().getBaseDiskDao(); + } + + protected DiskImageDynamicDAO getDiskImageDynamicDAO() { + return getDbFacade().getDiskImageDynamicDao(); + } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/storage/OpenStackVolumeProviderProxy.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/storage/OpenStackVolumeProviderProxy.java index fd1f83c..25aacd1 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/storage/OpenStackVolumeProviderProxy.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/storage/OpenStackVolumeProviderProxy.java @@ -67,6 +67,10 @@ return retCinderVolume.getId(); } + public void deleteVolume(String volumeId) { + getClient(getTenantId()).volumes().delete(volumeId).execute(); + } + public Volume getVolumeById(String id) { return getClient(getTenantId()).volumes().show(id).execute(); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/CinderBroker.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/CinderBroker.java index b9d9290..71b4c45 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/CinderBroker.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/CinderBroker.java @@ -3,6 +3,7 @@ import com.woorea.openstack.base.client.OpenStackResponseException; import com.woorea.openstack.cinder.model.Volume; import com.woorea.openstack.cinder.model.VolumeForCreate; +import org.apache.commons.httpclient.HttpStatus; import org.ovirt.engine.core.bll.provider.storage.OpenStackVolumeProviderProxy; import org.ovirt.engine.core.common.businessentities.storage.CinderDisk; import org.ovirt.engine.core.common.businessentities.storage.CinderVolumeStatus; @@ -50,6 +51,33 @@ }); } + public Void deleteDisk(final CinderDisk cinderDisk) { + return execute(new Callable<Void>() { + @Override + public Void call() { + proxy.deleteVolume(cinderDisk.getId().toString()); + return null; + } + }); + } + + public boolean isDiskExist(final Guid id) { + return execute(new Callable<Boolean>() { + @Override + public Boolean call() { + try { + Volume volume = proxy.getVolumeById(id.toString()); + return volume != null; + } catch (OpenStackResponseException ex) { + if (ex.getStatus() == HttpStatus.SC_NOT_FOUND) { + return false; + } + throw ex; + } + } + }); + } + public ImageStatus getDiskStatus(final Guid id) { return execute(new Callable<ImageStatus>() { @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveCinderDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveCinderDiskCommand.java new file mode 100644 index 0000000..3cea3bd --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveCinderDiskCommand.java @@ -0,0 +1,119 @@ +package org.ovirt.engine.core.bll.storage; + +import org.ovirt.engine.core.bll.ImagesHandler; +import org.ovirt.engine.core.bll.InternalCommandAttribute; +import org.ovirt.engine.core.bll.RemoveDiskCommand; +import org.ovirt.engine.core.bll.context.CommandContext; +import org.ovirt.engine.core.bll.tasks.interfaces.CommandCallback; +import org.ovirt.engine.core.common.action.RemoveDiskParameters; +import org.ovirt.engine.core.common.businessentities.VmDeviceId; +import org.ovirt.engine.core.common.businessentities.storage.CinderDisk; +import org.ovirt.engine.core.common.businessentities.storage.ImageStatus; +import org.ovirt.engine.core.common.utils.Pair; +import org.ovirt.engine.core.compat.CommandStatus; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.compat.TransactionScopeOption; +import org.ovirt.engine.core.dao.ImageDao; +import org.ovirt.engine.core.utils.transaction.TransactionMethod; +import org.ovirt.engine.core.utils.transaction.TransactionSupport; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.Collections; +import java.util.Map; + +@InternalCommandAttribute +public class RemoveCinderDiskCommand<T extends RemoveDiskParameters> extends RemoveDiskCommand<T> { + + private static final Logger log = LoggerFactory.getLogger(RemoveCinderDiskCommand.class); + private CinderBroker cinderBroker; + private Guid storageDomainId; + + public RemoveCinderDiskCommand(T parameters) { + super(parameters, null); + } + + public RemoveCinderDiskCommand(T parameters, CommandContext commandContext) { + super(parameters, commandContext); + } + + @Override + public void executeCommand() { + CinderDisk disk = (CinderDisk) getDisk(); + if (disk.getImageStatus() == ImageStatus.ILLEGAL) { + // Remove disk from DB + setCommandStatus(CommandStatus.SUCCEEDED); + } + else { + // Remove disk from Cinder + ImagesHandler.updateImageStatus(disk.getId(), ImageStatus.LOCKED); + getCinderBroker().deleteDisk(disk); + } + persistCommand(getParameters().getParentCommand(), true); + getReturnValue().setActionReturnValue(disk.getId()); + setSucceeded(true); + } + + protected void removeDiskFromDb() { + final CinderDisk cinderDisk = (CinderDisk) getDisk(); + TransactionSupport.executeInScope(TransactionScopeOption.Required, + new TransactionMethod<Object>() { + @Override + public Object runInTransaction() { + getDiskImageDynamicDAO().remove(cinderDisk.getImageId()); + getImageDao().remove(cinderDisk.getImageId()); + // todo: remove snapshot + getBaseDiskDao().remove(cinderDisk.getId()); + getVmDeviceDAO().remove(new VmDeviceId(cinderDisk.getId(), null)); + return null; + } + }); + } + + protected void endSuccessfully() { + setSucceeded(true); + } + + protected ImageDao getImageDao() { + return getDbFacade().getImageDao(); + } + + @Override + public CommandCallback getCallback() { + return new RemoveCinderDiskCommandCallback(); + } + + @Override + public boolean canDoAction() { + return true; + } + + @Override + protected Map<String, Pair<String, String>> getExclusiveLocks() { + return Collections.emptyMap(); + } + + @Override + protected Map<String, Pair<String, String>> getSharedLocks() { + return Collections.emptyMap(); + } + + protected CinderBroker getCinderBroker() { + if (cinderBroker == null) { + cinderBroker = new CinderBroker(getStorageDomainId(), getReturnValue().getExecuteFailedMessages()); + } + return cinderBroker; + } + + @Override + public Guid getStorageDomainId() { + if (storageDomainId == null) { + storageDomainId = getCinderDisk().getStorageIds().get(0); + } + return storageDomainId; + } + + private CinderDisk getCinderDisk() { + return (CinderDisk) getDisk(); + } +} diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveCinderDiskCommandCallback.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveCinderDiskCommandCallback.java new file mode 100644 index 0000000..0c1afa3 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RemoveCinderDiskCommandCallback.java @@ -0,0 +1,86 @@ +package org.ovirt.engine.core.bll.storage; + +import org.ovirt.engine.core.bll.ImagesHandler; +import org.ovirt.engine.core.bll.tasks.CommandCoordinatorUtil; +import org.ovirt.engine.core.common.AuditLogType; +import org.ovirt.engine.core.common.action.RemoveDiskParameters; +import org.ovirt.engine.core.common.businessentities.storage.CinderDisk; +import org.ovirt.engine.core.common.businessentities.storage.DiskImage; +import org.ovirt.engine.core.common.businessentities.storage.ImageStatus; +import org.ovirt.engine.core.compat.CommandStatus; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; + +import java.util.List; + +public class RemoveCinderDiskCommandCallback extends AbstractCinderDiskCommandCallback<RemoveCinderDiskCommand<RemoveDiskParameters>> { + + @Override + public void doPolling(Guid cmdId, List<Guid> childCmdIds) { + super.doPolling(cmdId, childCmdIds); + + CinderBroker cinderBroker = getCinderBroker(); + if (!cinderBroker.isDiskExist(getDiskId())) { + // Disk has been deleted successfully + getCommand().setCommandStatus(CommandStatus.SUCCEEDED); + return; + } + + ImageStatus imageStatus = cinderBroker.getDiskStatus(getDiskId()); + DiskImage disk = getDisk(); + if (imageStatus != null && imageStatus != disk.getImageStatus()) { + switch (imageStatus) { + case ILLEGAL: + getCommand().getParameters().setShouldBeLogged(true); + getCommand().setCommandStatus(CommandStatus.FAILED); + break; + } + } + } + + @Override + public void onFailed(Guid cmdId, List<Guid> childCmdIds) { + super.onFailed(cmdId, childCmdIds); + + log.error("Failed deleting disk from Cinder. ID: {}", getDiskId()); + if (getCommand().getParameters().getShouldBeLogged()) { + new AuditLogDirector().log(getCommand(), AuditLogType.USER_FINISHED_FAILED_REMOVE_DISK); + } + + ImagesHandler.updateImageStatus(getDiskId(), ImageStatus.ILLEGAL); + getCommand().endAction(); + CommandCoordinatorUtil.removeAllCommandsInHierarchy(cmdId); + } + + @Override + public void onSucceeded(Guid cmdId, List<Guid> childCmdIds) { + super.onSucceeded(cmdId, childCmdIds); + + log.info("Disk has been successfully deleted from Cinder. ID: {}", getDiskId()); + if (getCommand().getParameters().getShouldBeLogged()) { + new AuditLogDirector().log(getCommand(), AuditLogType.USER_FINISHED_REMOVE_DISK); + } + + getCommand().removeDiskFromDb(); + getCommand().endAction(); + CommandCoordinatorUtil.removeAllCommandsInHierarchy(cmdId); + } + + @Override + protected Guid getDiskId() { + return getCommand().getParameters().getDiskId(); + } + + @Override + protected CinderDisk getDisk() { + if (disk == null) { + disk = (CinderDisk) getCommand().getDiskDao().get(getDiskId()); + } + return disk; + } + + @Override + protected CinderBroker getCinderBroker() { + return getCommand().getCinderBroker(); + } +} diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java index 3abff17..bffd09d 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java @@ -242,6 +242,7 @@ USER_FAILED_MOVED_VM_DISK(2009, AuditLogSeverity.ERROR), USER_MOVED_VM_DISK_FINISHED_SUCCESS(2010), USER_MOVED_VM_DISK_FINISHED_FAILURE(2011, AuditLogSeverity.ERROR), + USER_REMOVE_DISK_INITIATED(2038), USER_FINISHED_REMOVE_DISK(2014), USER_FINISHED_FAILED_REMOVE_DISK(2015, AuditLogSeverity.WARNING), USER_FINISHED_REMOVE_DISK_NO_DOMAIN(2012), diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java index 26796ef..b71fdaf 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java @@ -426,6 +426,7 @@ // Cinder AddCinderDisk(3200, ActionGroup.CONFIGURE_VM_STORAGE, false, QuotaDependency.NONE), + RemoveCinderDisk(3201, QuotaDependency.STORAGE), // Host Devices RefreshHostDevices(4000, ActionGroup.MANIPULATE_HOST, false, QuotaDependency.NONE); diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties b/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties index 0121d08c..6dde699 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties @@ -120,6 +120,7 @@ USER_FAILED_MOVED_VM_DISK=User ${UserName} failed to move disk ${DiskAlias} to domain ${StorageDomainName}. USER_MOVED_VM_DISK_FINISHED_SUCCESS=User ${UserName} finished moving disk ${DiskAlias} to domain ${StorageDomainName}. USER_MOVED_VM_DISK_FINISHED_FAILURE=User ${UserName} have failed to move disk ${DiskAlias} to domain ${StorageDomainName}. +USER_REMOVE_DISK_INITIATED=Removal of Disk ${DiskAlias} from domain ${StorageDomainName} was initiated by ${UserName}. USER_FINISHED_REMOVE_DISK=Disk ${DiskAlias} was successfully removed from domain ${StorageDomainName} (User ${UserName}). USER_FINISHED_FAILED_REMOVE_DISK=Failed to remove disk ${DiskAlias} from storage domain ${StorageDomainName} (User: ${UserName}). USER_FINISHED_REMOVE_DISK_NO_DOMAIN=Disk ${DiskAlias} was successfully removed (User ${UserName}). -- To view, visit https://gerrit.ovirt.org/39135 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I7a3803072ade96a241e8f71a030360f4168a1b71 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
