Greg Padgett has uploaded a new change for review. Change subject: core: Enable Live Merge in Snapshots Overview ......................................................................
core: Enable Live Merge in Snapshots Overview Enable Live Merge in the snapshots overview. This required enabling the action in CanDoAction using code similar to RemoveSnapshotCommand, and branching the RemoveDiskSnapshotsCommand task handlers to use CommandExecutor to start Live Merge. Some changes had to be made to adjust the flow after the task handlers were initialized due to backwards merges as well as images being deleted before being dereferenced, which was handled in addition to the new flows. Change-Id: Iaa8c5453dd12193bb5ef3e15ecfe5968d0880069 Bug-Url: https://bugzilla.redhat.com/1133890 Signed-off-by: Greg Padgett <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotTaskHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotsCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RemoveDiskSnapshotsParameters.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VMStatus.java 6 files changed, 189 insertions(+), 31 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/04/33004/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotTaskHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotTaskHandler.java index 0b42863..1594f7e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotTaskHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotTaskHandler.java @@ -1,11 +1,13 @@ package org.ovirt.engine.core.bll; +import java.util.Arrays; + import org.ovirt.engine.core.bll.context.CommandContext; import org.ovirt.engine.core.bll.job.ExecutionHandler; +import org.ovirt.engine.core.bll.tasks.CommandCoordinatorUtil; import org.ovirt.engine.core.bll.tasks.SPMAsyncTaskHandler; -import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand; -import org.ovirt.engine.core.common.action.ImagesContainterParametersBase; import org.ovirt.engine.core.common.action.RemoveDiskSnapshotsParameters; +import org.ovirt.engine.core.common.action.RemoveSnapshotSingleDiskParameters; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VdcReturnValueBase; import org.ovirt.engine.core.common.asynctasks.AsyncTaskType; @@ -20,21 +22,56 @@ public class RemoveDiskSnapshotTaskHandler implements SPMAsyncTaskHandler { private static final Log log = LogFactory.getLog(RemoveDiskSnapshotTaskHandler.class); - private final TaskHandlerCommand<? extends RemoveDiskSnapshotsParameters> enclosingCommand; - private final Guid imageId; + private final RemoveDiskSnapshotsCommand<? extends RemoveDiskSnapshotsParameters> enclosingCommand; + private Guid imageId; private final Guid imageGroupId; - private final Guid vmId; + private Guid childImageId; - public RemoveDiskSnapshotTaskHandler(TaskHandlerCommand<? extends RemoveDiskSnapshotsParameters> enclosingCommand, - Guid imageId, Guid imageGroupId, Guid vmId) { + public RemoveDiskSnapshotTaskHandler(RemoveDiskSnapshotsCommand<? extends RemoveDiskSnapshotsParameters> enclosingCommand, + Guid imageId, Guid imageGroupId) { this.enclosingCommand = enclosingCommand; this.imageId = imageId; this.imageGroupId = imageGroupId; - this.vmId = vmId; + } + + private void setImageId(Guid imageId) { + this.imageId = imageId; + } + + private Guid getImageId() { + return imageId; } @Override public void execute() { + if (enclosingCommand.getVm().isQualifiedForLiveMerge()) { + startLiveMerge(); + } else { + startColdMerge(); + } + } + + private void startLiveMerge() { + if (enclosingCommand.getParameters().getExecutionIndex() == 0) { + enclosingCommand.getParameters().setIsLiveMerge(true); + } + + RemoveSnapshotSingleDiskParameters parameters = + buildRemoveSnapshotSingleDiskParameters(VdcActionType.RemoveSnapshotSingleDiskLive); + if (enclosingCommand.getParameters().getChildImageIds() == null) { + enclosingCommand.getParameters().setChildImageIds(Arrays.asList(new Guid[enclosingCommand.getImages().size()])); + } + enclosingCommand.getParameters().getChildImageIds().set(enclosingCommand.getParameters().getExecutionIndex(), parameters.getDestinationImageId()); + enclosingCommand.persistCommand(enclosingCommand.getParameters().getParentCommand(), true); + + CommandCoordinatorUtil.executeAsyncCommand( + VdcActionType.RemoveSnapshotSingleDiskLive, + parameters, + getCommandContextLive()); + enclosingCommand.getReturnValue().setSucceeded(true); + } + + private void startColdMerge() { if (enclosingCommand.getParameters().getExecutionIndex() == 0) { // lock all disk images in advance updateImagesStatus(ImageStatus.LOCKED); @@ -42,7 +79,7 @@ VdcReturnValueBase vdcReturnValue = Backend.getInstance().runInternalAction( VdcActionType.RemoveSnapshotSingleDisk, - buildRemoveSnapshotSingleDiskParameters(), + buildRemoveSnapshotSingleDiskParameters(VdcActionType.RemoveSnapshotSingleDisk), getCommandContext()); if (vdcReturnValue.getSucceeded()) { @@ -51,16 +88,17 @@ .addAll(vdcReturnValue.getInternalVdsmTaskIdList()); } else { - log.errorFormat("Failed RemoveSnapshotSingleDisk (Image {0} , VM {1})", imageId, vmId); + log.errorFormat("Failed RemoveSnapshotSingleDisk (Image {0} , VM {1})", + imageId, enclosingCommand.getVm().getId()); } ExecutionHandler.setAsyncJob(enclosingCommand.getExecutionContext(), true); enclosingCommand.getReturnValue().setSucceeded(true); } - private ImagesContainterParametersBase buildRemoveSnapshotSingleDiskParameters() { - ImagesContainterParametersBase parameters = new ImagesContainterParametersBase( - imageId, vmId); + private RemoveSnapshotSingleDiskParameters buildRemoveSnapshotSingleDiskParameters(VdcActionType commandType) { + RemoveSnapshotSingleDiskParameters parameters = new RemoveSnapshotSingleDiskParameters( + imageId, enclosingCommand.getVm().getId()); DiskImage dest = DbFacade.getInstance().getDiskImageDao().getAllSnapshotsForParent(imageId).get(0); @@ -69,8 +107,19 @@ parameters.setParentParameters(enclosingCommand.getParameters()); parameters.setParentCommand(enclosingCommand.getActionType()); parameters.setWipeAfterDelete(dest.isWipeAfterDelete()); + parameters.setCommandType(commandType); + parameters.setVdsId(enclosingCommand.getVm().getRunOnVds()); parameters.setSessionId(enclosingCommand.getParameters().getSessionId()); + return parameters; + } + + + private CommandContext getCommandContextLive() { + CommandContext commandContext = enclosingCommand.cloneContextAndDetachFromParent(); + commandContext.getExecutionContext().setShouldEndJob(isLastTaskHandler()); + + return commandContext; } private CommandContext getCommandContext() { @@ -88,29 +137,68 @@ @Override public void endSuccessfully() { - endRemoveSnapshotSingleDisk(true); - enclosingCommand.taskEndSuccessfully(); - if (isLastTaskHandler()) { - // Unlock on job finish - updateImagesStatus(ImageStatus.OK); + if (enclosingCommand.getParameters().getIsLiveMerge()) { + enclosingCommand.taskEndSuccessfully(); + ensureHandlerImageOrder(); + if (isLastTaskHandler()) { + CommandCoordinatorUtil.removeAllCommandsInHierarchy(enclosingCommand.getCommandId()); + } + } else { + endRemoveSnapshotSingleDisk(true); + enclosingCommand.taskEndSuccessfully(); + if (isLastTaskHandler()) { + // Unlock on job finish + updateImagesStatus(ImageStatus.OK); + } } enclosingCommand.getReturnValue().setSucceeded(true); } + /** + * Ensures that after a backwards merge (in which the current snapshot's image takes the + * place of the next snapshot's image), subsequent task handlers will refer to the correct + * image id and not the one that has been removed. + */ + private void ensureHandlerImageOrder() { + int executionIndex = enclosingCommand.getParameters().getExecutionIndex(); + Guid childImageId = enclosingCommand.getParameters().getChildImageIds().get(executionIndex); + if (DbFacade.getInstance().getDiskImageDao().get(childImageId) == null) { + // Swap instances of the removed id with our id + boolean save = false; + for (int i = executionIndex + 1; i < enclosingCommand.getImages().size(); i++) { + RemoveDiskSnapshotTaskHandler handler = (RemoveDiskSnapshotTaskHandler) enclosingCommand.getTaskHandlers().get(i); + if (handler.getImageId().equals(childImageId)) { + handler.setImageId(imageId); + log.debugFormat("Switched handler {0} image guid from {1} to {2} due to backwards merge", + i, childImageId, imageId); + save = true; + } + } + if (save) { + enclosingCommand.persistCommand(enclosingCommand.getParameters().getParentCommand(), true); + } + } + } + @Override public void endWithFailure() { - endRemoveSnapshotSingleDisk(false); - // Unlock all images since failure aborts the entire job - Disk disk = DbFacade.getInstance().getDiskDao().get(imageGroupId); - if (((DiskImage) disk).getImageStatus() == ImageStatus.LOCKED) { - updateImagesStatus(ImageStatus.OK); + if (enclosingCommand.getParameters().getIsLiveMerge()) { + CommandCoordinatorUtil.removeAllCommandsInHierarchy(enclosingCommand.getCommandId()); + } else { + endRemoveSnapshotSingleDisk(false); + // Unlock all images since failure aborts the entire job + Disk disk = DbFacade.getInstance().getDiskDao().get(imageGroupId); + if (((DiskImage) disk).getImageStatus() == ImageStatus.LOCKED) { + updateImagesStatus(ImageStatus.OK); + } } enclosingCommand.preventRollback(); enclosingCommand.getReturnValue().setSucceeded(true); } private void endRemoveSnapshotSingleDisk(boolean taskGroupSuccess) { - ImagesContainterParametersBase parameters = buildRemoveSnapshotSingleDiskParameters(); + RemoveSnapshotSingleDiskParameters parameters = + buildRemoveSnapshotSingleDiskParameters(VdcActionType.RemoveSnapshotSingleDisk); parameters.setTaskGroupSuccess(taskGroupSuccess); VdcReturnValueBase vdcReturnValue = Backend.getInstance().endAction( VdcActionType.RemoveSnapshotSingleDisk, @@ -134,7 +222,7 @@ @Override public AsyncTaskType getRevertTaskType() { - // No implementation - there is no live-merge + // No implementation return null; } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotsCommand.java index 43e84dd..0566b0b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotsCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskSnapshotsCommand.java @@ -7,16 +7,19 @@ import java.util.Map; import org.apache.commons.lang.StringUtils; +import org.ovirt.engine.core.bll.context.CommandContext; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.storage.StoragePoolValidator; import org.ovirt.engine.core.bll.tasks.SPMAsyncTaskHandler; import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand; +import org.ovirt.engine.core.bll.tasks.interfaces.CommandCallBack; import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.bll.validator.DiskImagesValidator; import org.ovirt.engine.core.bll.validator.DiskSnapshotsValidator; import org.ovirt.engine.core.bll.validator.StorageDomainValidator; import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.AuditLogType; +import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.LockProperties; import org.ovirt.engine.core.common.action.LockProperties.Scope; @@ -45,6 +48,10 @@ public RemoveDiskSnapshotsCommand(T parameters) { super(parameters); + } + + public RemoveDiskSnapshotsCommand(T parameters, CommandContext cmdContext) { + super(parameters, cmdContext); } @Override @@ -133,10 +140,16 @@ return false; } - VmValidator vmValidator = createVmValidator(getVm()); - if (isDiskPlugged() && !validate(vmValidator.vmDown())) { - return false; + if (isDiskPlugged()) { + VmValidator vmValidator = createVmValidator(getVm()); + if (FeatureSupported.liveMerge(getVm().getVdsGroupCompatibilityVersion()) + ? (!validate(vmValidator.vmQualifiedForSnapshotMerge()) + || !validate(vmValidator.vmHostCanLiveMerge())) + : !validate(vmValidator.vmDown())) { + return false; + } } + if (!validate(new StoragePoolValidator(getStoragePool()).isUp()) || !validateVmNotDuringSnapshot() || @@ -164,10 +177,20 @@ List<SPMAsyncTaskHandler> taskHandlers = new ArrayList<>(); for (Guid imageId : getParameters().getImageIds()) { - taskHandlers.add(new RemoveDiskSnapshotTaskHandler(this, imageId, getImageGroupId(), getVmId())); + taskHandlers.add(new RemoveDiskSnapshotTaskHandler(this, imageId, getImageGroupId())); } return taskHandlers; + } + + @Override + public CommandCallBack getCallBack() { + // Handle first execution (isLiveMerge not set) and recovery (VM may be down) + if (getVm().isQualifiedForLiveMerge() || getParameters().getIsLiveMerge()) { + return new RemoveDiskSnapshotsCommandCallback(); + } else { + return null; + } } @Override @@ -252,9 +275,19 @@ protected boolean validateAllDiskImages() { List<DiskImage> images = getDiskImageDao().getAllSnapshotsForImageGroup(getDiskImage().getId()); DiskImagesValidator diskImagesValidator = new DiskImagesValidator(images); + DiskImagesValidator illegalDiskImagesValidator; + if (getVm().isQualifiedForLiveMerge()) { + // If the target images for Live Merge are illegal, the operation can be retried + List<DiskImage> otherImages = new ArrayList<>(images); + otherImages.removeAll(getImages()); + illegalDiskImagesValidator = new DiskImagesValidator(otherImages); + } else { + illegalDiskImagesValidator = diskImagesValidator; + } return validate(diskImagesValidator.diskImagesNotLocked()) && - validate(diskImagesValidator.diskImagesNotIllegal()); + validate(illegalDiskImagesValidator.diskImagesNotIllegal()); + } protected boolean validateStorageDomainActive() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java index d9aa8d1..debc246 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java @@ -444,7 +444,9 @@ } private VdcActionType getSnapshotActionType() { - return getVm().isDown() ? VdcActionType.RemoveSnapshotSingleDisk : VdcActionType.RemoveSnapshotSingleDiskLive; + return getVm().isQualifiedForLiveMerge() + ? VdcActionType.RemoveSnapshotSingleDiskLive + : VdcActionType.RemoveSnapshotSingleDisk; } @Override diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RemoveDiskSnapshotsParameters.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RemoveDiskSnapshotsParameters.java index 111420c..dbeda0d 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RemoveDiskSnapshotsParameters.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/RemoveDiskSnapshotsParameters.java @@ -5,12 +5,17 @@ import java.io.Serializable; import java.util.ArrayList; import java.util.Arrays; +import java.util.List; public class RemoveDiskSnapshotsParameters extends ImagesContainterParametersBase implements Serializable { private static final long serialVersionUID = -629355522841577585L; private ArrayList<Guid> imageIds; + + // The following is used to persist data during command execution + private boolean isLiveMerge; + private List<Guid> childImageIds; public RemoveDiskSnapshotsParameters(Guid imageId) { this(new ArrayList<Guid>(Arrays.asList(imageId))); @@ -30,4 +35,20 @@ public void setImageIds(ArrayList<Guid> imageIds) { this.imageIds = imageIds; } + + public boolean getIsLiveMerge() { + return isLiveMerge; + } + + public void setIsLiveMerge(boolean isLiveMerge) { + this.isLiveMerge = isLiveMerge; + } + + public List<Guid> getChildImageIds() { + return childImageIds; + } + + public void setChildImageIds(List<Guid> childImageIds) { + this.childImageIds = childImageIds; + } } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java index 8f3694c..c87f2c5 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java @@ -1662,6 +1662,10 @@ return getStatus().isQualifiedForSnapshotMerge(); } + public boolean isQualifiedForLiveMerge() { + return getStatus().isQualifiedForLiveMerge(); + } + public boolean isRunningAndQualifyForDisksMigration() { return getStatus().isUpOrPaused() && getRunOnVds() != null && !getRunOnVds().equals(Guid.Empty); } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VMStatus.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VMStatus.java index dd19052..8ab9a36 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VMStatus.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VMStatus.java @@ -77,6 +77,16 @@ } /** + * This method reflects whether the VM is in a state where Live Merge can be performed. + * See #isQualifiedForSnapshotMerge() + * + * @return true if this status indicates that a snapshot merge should be a Live Merge + */ + public boolean isQualifiedForLiveMerge() { + return this == Up || this == PoweringUp || this == Paused || this == RebootInProgress; + } + + /** * This method reflects whether the VM is surely running or paused in this status * * @see #isRunning() -- To view, visit http://gerrit.ovirt.org/33004 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iaa8c5453dd12193bb5ef3e15ecfe5968d0880069 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Greg Padgett <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
