Idan Shaby has uploaded a new change for review. Change subject: core: WAD should be Ignored on File Domain Disks ......................................................................
core: WAD should be Ignored on File Domain Disks Since file systems do the wiping for us, there is no need for doing it again for disks on file domains. Thus, the only case we should wipe a disk after deleting it is when we have a disk on a block domain with 'wipeAfterDelete' = true. Change-Id: Ie8a9932725ef861525f5102e8617938f0e0a71c7 Bug-Url: https://bugzilla.redhat.com/1097820 Signed-off-by: Idan Shaby <ish...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CopyImageGroupCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateCloneOfTemplateCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateImageTemplateCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMemoryVolumesCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotSingleDiskCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveTemplateSnapshotCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmFromImportExportCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreFromSnapshotCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/CreateImagePlaceholderTaskHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/VmReplicateDiskFinishTaskHandler.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/HibernationVolumesRemover.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemover.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverFromExportDomain.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverTest.java 21 files changed, 197 insertions(+), 79 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/06/30706/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java index 8104612..73bedec 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java @@ -128,7 +128,9 @@ params.setVolumeType(diskImage.getVolumeType()); params.setUseCopyCollapse(true); params.setSourceDomainId(srcStorageDomainId); - params.setWipeAfterDelete(diskImage.isWipeAfterDelete()); + setStorageDomainId(srcStorageDomainId); + params.setWipeAfterDelete(ImagesHandler.shouldWipeAfterDelete( + diskImage.isWipeAfterDelete(), getStorageDomain().getStorageType())); params.setEntityInfo(new EntityInfo(VdcObjectType.VM, getVmId())); params.setParentParameters(getParameters()); params.setParentCommand(parentCommandType); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CopyImageGroupCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CopyImageGroupCommand.java index 973a0d7..3bf5a1e 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CopyImageGroupCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CopyImageGroupCommand.java @@ -124,8 +124,15 @@ } private boolean isWipeAfterDelete() { - return getDestinationDiskImage() != null ? getDestinationDiskImage().isWipeAfterDelete() - : getParameters().getWipeAfterDelete(); + boolean isWipeAfterDelete; + if (getDestinationDiskImage() == null) { + isWipeAfterDelete = getParameters().getWipeAfterDelete(); + } + else { + isWipeAfterDelete = getDestinationDiskImage().isWipeAfterDelete(); + } + return ImagesHandler.shouldWipeAfterDelete( + isWipeAfterDelete, getStorageDomain().getStorageType()); } /** diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateCloneOfTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateCloneOfTemplateCommand.java index d0a2d0c..5d9dd01 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateCloneOfTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateCloneOfTemplateCommand.java @@ -54,13 +54,14 @@ VDSReturnValue vdsReturnValue = null; Guid taskId = persistAsyncTaskPlaceHolder(VdcActionType.AddVmFromTemplate); try { + boolean wipeAfterDelete = ImagesHandler.shouldWipeAfterDelete( + getDiskImage().isWipeAfterDelete(), getStorageDomain().getStorageType()); vdsReturnValue = runVdsCommand(VDSCommandType.CopyImage, new CopyImageVDSCommandParameters(storagePoolID, getParameters().getStorageDomainId(), getVmTemplateId(), getDiskImage().getId(), getImage().getImageId(), newDiskImage.getId(), getDestinationImageId(), "", getDestinationStorageDomainId(), CopyVolumeType.LeafVol, - newDiskImage.getVolumeFormat(), newDiskImage.getVolumeType(), - getDiskImage().isWipeAfterDelete(), false)); + newDiskImage.getVolumeFormat(), newDiskImage.getVolumeType(), wipeAfterDelete, false)); } catch (VdcBLLException e) { log.errorFormat("Failed creating snapshot from image id -'{0}'", getImage().getImageId()); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateImageTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateImageTemplateCommand.java index a0d5537..a061ed2 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateImageTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateImageTemplateCommand.java @@ -52,14 +52,15 @@ fillVolumeInformation(newImage); Guid taskId = getAsyncTaskId(); + boolean wipeAfterDelete = ImagesHandler.shouldWipeAfterDelete( + getDiskImage().isWipeAfterDelete(), getStorageDomain().getStorageType()); VDSReturnValue vdsReturnValue = runVdsCommand(VDSCommandType.CopyImage, new CopyImageVDSCommandParameters(storagePoolId, getParameters().getStorageDomainId(), - getParameters().getVmId(), imageGroupId, snapshotId, destinationImageGroupID, - getDestinationImageId(), StringUtils.defaultString(newImage.getDescription()), getParameters() - .getDestinationStorageDomainId(), CopyVolumeType.SharedVol, newImage - .getVolumeFormat(), newImage.getVolumeType(), getDiskImage() - .isWipeAfterDelete(), false)); + getParameters().getVmId(), imageGroupId, snapshotId, destinationImageGroupID, + getDestinationImageId(), StringUtils.defaultString(newImage.getDescription()), + getParameters().getDestinationStorageDomainId(), CopyVolumeType.SharedVol, + newImage.getVolumeFormat(), newImage.getVolumeType(), wipeAfterDelete, false)); getReturnValue().getInternalVdsmTaskIdList().add( createTask(taskId, diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommand.java index 50ce643..70f0c17 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommand.java @@ -58,12 +58,14 @@ } private VDSParametersBase createVDSParameters() { + boolean isPostZero = ImagesHandler.shouldWipeAfterDelete( + getParameters().isPostZero(), getStorageDomain().getStorageType()); return new DestroyImageVDSCommandParameters( getParameters().getStoragePoolId(), getParameters().getStorageDomainId(), getParameters().getImageGroupId(), getParameters().getImageList(), - getParameters().isPostZero(), + isPostZero, getParameters().isForce()); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java index 3ac42ee..86cadf6 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java @@ -808,4 +808,16 @@ } return snapshot; } + + /** + * Decides whether an image disk should be wiped after it's being deleted or not. + * A disk on a file domain, for example, should not be wiped even if it holds 'wipeAfterDelete' = true. + * @param shouldDiskBeWipedAfterDelete the disk's 'wipeAfterDelete' property. + * @param type the disk's type. + * @return true iff the disk should be wiped after its deletion, + * i.e. only if the disk is on a block domain and holds 'wipeAfterDelete' = true. + */ + public static boolean shouldWipeAfterDelete (boolean shouldDiskBeWipedAfterDelete, StorageType type) { + return shouldDiskBeWipedAfterDelete && type.isBlockDomain(); + } } 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 f1086b1..efd5fc5 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 @@ -291,10 +291,13 @@ ImageStatus.ILLEGAL, getCompensationContext()); } + + boolean wipeAfterDelete = ImagesHandler.shouldWipeAfterDelete( + getDiskImage().isWipeAfterDelete(), getStorageDomain().getStorageType()); return runVdsCommand(VDSCommandType.DeleteImageGroup, new DeleteImageGroupVDSCommandParameters(getDiskImage().getStoragePoolId(), getStorageDomainId(), getDiskImage().getId(), - getDiskImage().isWipeAfterDelete(), getParameters().getForceDelete())); + wipeAfterDelete, getParameters().getForceDelete())); } protected VmDeviceDAO getVmDeviceDAO() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMemoryVolumesCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMemoryVolumesCommand.java index d9fc5ec..05817e6 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMemoryVolumesCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMemoryVolumesCommand.java @@ -37,9 +37,7 @@ @Override protected void executeCommand() { MemoryImageRemoverOnDataDomain memoryImageRemover = - new MemoryImageRemoverOnDataDomain( - getParameters().getVmId(), - this); + new MemoryImageRemoverOnDataDomain(getParameters().getVmId(), this); setSucceeded(memoryImageRemover.remove( Collections.singleton(getParameters().getMemoryVolumes()))); diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotSingleDiskCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotSingleDiskCommand.java index a022331..4a4aa28 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotSingleDiskCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotSingleDiskCommand.java @@ -28,12 +28,14 @@ protected void executeCommand() { Guid storagePoolId = (getDiskImage().getStoragePoolId() != null) ? getDiskImage().getStoragePoolId() : Guid.Empty; + setStoragePoolId(storagePoolId); Guid storageDomainId = !CollectionUtils.isEmpty(getDiskImage().getStorageIds()) ? getDiskImage().getStorageIds().get(0) : Guid.Empty; + setStorageDomainId(storageDomainId); Guid taskId = persistAsyncTaskPlaceHolder(getParameters().getParentCommand()); - VDSReturnValue vdsReturnValue = mergeSnapshots(storagePoolId, storageDomainId); + VDSReturnValue vdsReturnValue = mergeSnapshots(); if (vdsReturnValue != null && vdsReturnValue.getCreationInfo() != null) { getReturnValue().getInternalVdsmTaskIdList().add(createTask(taskId, vdsReturnValue, storageDomainId)); setSucceeded(vdsReturnValue.getSucceeded()); @@ -42,10 +44,12 @@ } } - protected VDSReturnValue mergeSnapshots(Guid storagePoolId, Guid storageDomainId) { - MergeSnapshotsVDSCommandParameters params = new MergeSnapshotsVDSCommandParameters(storagePoolId, - storageDomainId, getVmId(), getDiskImage().getId(), getDiskImage().getImageId(), - getDestinationDiskImage().getImageId(), getDiskImage().isWipeAfterDelete()); + protected VDSReturnValue mergeSnapshots() { + boolean wipeAfterDelete = ImagesHandler.shouldWipeAfterDelete( + getDiskImage().isWipeAfterDelete(), getStorageDomain().getStorageType()); + MergeSnapshotsVDSCommandParameters params = new MergeSnapshotsVDSCommandParameters(getStoragePoolId(), + getStorageDomainId(), getVmId(), getDiskImage().getId(), getDiskImage().getImageId(), + getDestinationDiskImage().getImageId(), wipeAfterDelete); return runVdsCommand(VDSCommandType.MergeSnapshots, params); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveTemplateSnapshotCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveTemplateSnapshotCommand.java index b8229ed..3bcbdbc 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveTemplateSnapshotCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveTemplateSnapshotCommand.java @@ -24,11 +24,12 @@ protected void executeCommand() { Guid taskId = persistAsyncTaskPlaceHolder(VdcActionType.RemoveVmTemplate); - VDSReturnValue vdsReturnValue = runVdsCommand( - VDSCommandType.DeleteImageGroup, - new DeleteImageGroupVDSCommandParameters(getParameters().getStoragePoolId(), getParameters() - .getStorageDomainId(), getParameters().getImageGroupID(), getParameters() - .getWipeAfterDelete(), false)); + boolean wipeAfterDelete = ImagesHandler.shouldWipeAfterDelete( + getParameters().getWipeAfterDelete(), getStorageDomain().getStorageType()); + VDSReturnValue vdsReturnValue = runVdsCommand(VDSCommandType.DeleteImageGroup, + new DeleteImageGroupVDSCommandParameters(getParameters().getStoragePoolId(), + getParameters().getStorageDomainId(), getParameters().getImageGroupID(), + wipeAfterDelete, false)); if (vdsReturnValue.getSucceeded()) { getReturnValue().getInternalVdsmTaskIdList().add( 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 8c242c7..0c6634d 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 @@ -132,9 +132,8 @@ } private void removeMemoryImages() { - new MemoryImageRemoverFromExportDomain(getVm(), this, - getParameters().getStoragePoolId(), getParameters().getStorageDomainId()) - .remove(); + new MemoryImageRemoverFromExportDomain(getVm(), this, + getParameters().getStoragePoolId(), getParameters().getStorageDomainId()).remove(); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreFromSnapshotCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreFromSnapshotCommand.java index c0a20b5..aa74dcd 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreFromSnapshotCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreFromSnapshotCommand.java @@ -93,19 +93,25 @@ try { Guid storagePoolId = getDiskImage().getStoragePoolId() != null ? getDiskImage().getStoragePoolId() : Guid.Empty; + setStoragePoolId(storagePoolId); + Guid storageDomainId = getDiskImage().getStorageIds() != null && !getDiskImage().getStorageIds().isEmpty() ? getDiskImage().getStorageIds() .get(0) : Guid.Empty; + setStorageDomainId(storageDomainId); + Guid imageGroupId = getDiskImage().getimage_group_id() != null ? getDiskImage().getimage_group_id() : Guid.Empty; Guid taskId = persistAsyncTaskPlaceHolder(VdcActionType.RestoreAllSnapshots); + boolean wipeAfterDelete = ImagesHandler.shouldWipeAfterDelete( + getDiskImage().isWipeAfterDelete(), getStorageDomain().getStorageType()); vdsReturnValue = runVdsCommand( - VDSCommandType.DestroyImage, - new DestroyImageVDSCommandParameters(storagePoolId, storageDomainId, imageGroupId, - _imagesToDelete, getDiskImage().isWipeAfterDelete(), true)); + VDSCommandType.DestroyImage, + new DestroyImageVDSCommandParameters(storagePoolId, storageDomainId, imageGroupId, + _imagesToDelete, wipeAfterDelete, true)); if (vdsReturnValue.getSucceeded()) { getReturnValue().getInternalVdsmTaskIdList().add( diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java index 05a5227..33eb791 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmCommand.java @@ -319,6 +319,7 @@ List<Guid> guids = GuidUtils.getGuidListFromString(memVols); if (guids.size() == 6) { + setStorageDomainId(guids.get(0)); // get all vm disks in order to check post zero - if one of the // disks is marked with wipe_after_delete boolean postZero = @@ -326,7 +327,8 @@ new Predicate<Disk>() { @Override public boolean eval(Disk disk) { - return disk.isWipeAfterDelete(); + return ImagesHandler.shouldWipeAfterDelete( + disk.isWipeAfterDelete(), getStorageDomain().getStorageType()); } }).size() > 0; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/CreateImagePlaceholderTaskHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/CreateImagePlaceholderTaskHandler.java index a3cb7ef..23ec1ce 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/CreateImagePlaceholderTaskHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/CreateImagePlaceholderTaskHandler.java @@ -77,14 +77,16 @@ @Override protected VDSParametersBase getRevertVDSParameters() { + boolean wipeAfterDelete = ImagesHandler.shouldWipeAfterDelete( + DbFacade.getInstance().getDiskImageDao().get(getEnclosingCommand(). + getParameters().getDestinationImageId()).isWipeAfterDelete(), + DbFacade.getInstance().getStorageDomainDao().get(getEnclosingCommand(). + getParameters().getStorageDomainId()).getStorageType()); return new DeleteImageGroupVDSCommandParameters( getEnclosingCommand().getParameters().getStoragePoolId(), getEnclosingCommand().getParameters().getTargetStorageDomainId(), getEnclosingCommand().getParameters().getImageGroupID(), - DbFacade.getInstance() - .getDiskImageDao() - .get(getEnclosingCommand().getParameters().getDestinationImageId()) - .isWipeAfterDelete(), + wipeAfterDelete, getEnclosingCommand().getParameters().getForceDelete()); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/VmReplicateDiskFinishTaskHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/VmReplicateDiskFinishTaskHandler.java index dc2768e..447ba43 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/VmReplicateDiskFinishTaskHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/VmReplicateDiskFinishTaskHandler.java @@ -136,15 +136,16 @@ @Override protected VDSParametersBase getVDSParameters() { + boolean wipeAfterDelete = ImagesHandler.shouldWipeAfterDelete( + DbFacade.getInstance().getDiskImageDao().get(getEnclosingCommand(). + getParameters().getDestinationImageId()).isWipeAfterDelete(), + DbFacade.getInstance().getStorageDomainDao().get(getEnclosingCommand(). + getParameters().getStorageDomainId()).getStorageType()); return new DeleteImageGroupVDSCommandParameters( getEnclosingCommand().getParameters().getStoragePoolId(), getEnclosingCommand().getParameters().getSourceStorageDomainId(), getEnclosingCommand().getParameters().getImageGroupID(), - DbFacade.getInstance() - .getDiskImageDao() - .get(getEnclosingCommand().getParameters().getDestinationImageId()) - .isWipeAfterDelete(), - getEnclosingCommand().getParameters().getForceDelete()); + wipeAfterDelete, getEnclosingCommand().getParameters().getForceDelete()); } @Override diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/HibernationVolumesRemover.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/HibernationVolumesRemover.java index a6435d0..e2527ab 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/HibernationVolumesRemover.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/HibernationVolumesRemover.java @@ -9,17 +9,17 @@ public class HibernationVolumesRemover extends MemoryImageRemover { - private Boolean cachedPostZero; private Guid vmId; private String memoryState; public HibernationVolumesRemover(String memoryState, Guid vmId, - TaskHandlerCommand<?> enclosingCommand) { + TaskHandlerCommand<?> enclosingCommand) { this(memoryState, vmId, enclosingCommand, false); } public HibernationVolumesRemover(String memoryState, Guid vmId, - TaskHandlerCommand<?> enclosingCommand, boolean startPollingTasks) { + TaskHandlerCommand<?> enclosingCommand, + boolean startPollingTasks) { super(enclosingCommand, startPollingTasks); this.memoryState = memoryState; this.vmId = vmId; @@ -32,13 +32,13 @@ @Override protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryImageParams(List<Guid> guids) { return new DeleteImageGroupVDSCommandParameters( - guids.get(1), guids.get(0), guids.get(2), isPostZero(), false); + guids.get(1), getStorageDomainId(guids), guids.get(2), isPostZero(guids), false); } @Override protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryConfParams(List<Guid> guids) { return new DeleteImageGroupVDSCommandParameters( - guids.get(1), guids.get(0), guids.get(4), isPostZero(), false); + guids.get(1), getStorageDomainId(guids), guids.get(4), isPostZero(guids), false); } @Override @@ -49,10 +49,7 @@ enclosingCommand.getParameters().getParentCommand()); } - protected boolean isPostZero() { - if (cachedPostZero == null) { - cachedPostZero = isDiskWithWipeAfterDeleteExist(getDiskDao().getAllForVm(vmId)); - } - return cachedPostZero; + protected boolean isPostZero(List<Guid> guids) { + return isPostZero(getDiskDao().getAllForVm(vmId), getStorageDomainId(guids)); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemover.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemover.java index e4bf159..4c6225f 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemover.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemover.java @@ -5,10 +5,12 @@ import java.util.Set; import org.ovirt.engine.core.bll.Backend; +import org.ovirt.engine.core.bll.ImagesHandler; import org.ovirt.engine.core.bll.VmCommand; import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand; import org.ovirt.engine.core.bll.tasks.TaskManagerUtil; import org.ovirt.engine.core.common.businessentities.Disk; +import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.common.errors.VDSError; import org.ovirt.engine.core.common.errors.VdcBLLException; import org.ovirt.engine.core.common.errors.VdcBllErrors; @@ -27,13 +29,15 @@ private static final int NUM_OF_UUIDS_IN_MEMORY_STATE = 6; protected final TaskHandlerCommand<?> enclosingCommand; + private Boolean cachedPostZero; private boolean startPollingTasks; public MemoryImageRemover(TaskHandlerCommand<?> enclosingCommand) { this.enclosingCommand = enclosingCommand; } - public MemoryImageRemover(TaskHandlerCommand<?> enclosingCommand, boolean startPollingTasks) { + public MemoryImageRemover(TaskHandlerCommand<?> enclosingCommand, + boolean startPollingTasks) { this(enclosingCommand); this.startPollingTasks = startPollingTasks; } @@ -41,6 +45,10 @@ protected abstract DeleteImageGroupVDSCommandParameters buildDeleteMemoryImageParams(List<Guid> guids); protected abstract DeleteImageGroupVDSCommandParameters buildDeleteMemoryConfParams(List<Guid> guids); + + protected Guid getStorageDomainId(List<Guid> guids) { + return guids.get(0); + } protected Guid createTask(Guid taskId, VDSReturnValue vdsRetValue) { return enclosingCommand.createTask( @@ -174,6 +182,26 @@ return false; } + /** + * We set the post zero field on memory image deletion from export domain as we do + * when it is deleted from data domain even though the export domain is NFS and NFS + * storage do the wipe on its own, in order to be compliance with the rest of the + * code that do the same, and to be prepared for supporting export domains which + * are not NFS. + */ + protected boolean isPostZero(Collection<Disk> disks, Guid sdId) { + if (cachedPostZero == null) { + cachedPostZero = ImagesHandler.shouldWipeAfterDelete( + isDiskWithWipeAfterDeleteExist(disks), + getStorageType(sdId)); + } + return cachedPostZero; + } + + private StorageType getStorageType(Guid sdId) { + return DbFacade.getInstance().getStorageDomainStaticDao().get(sdId).getStorageType(); + } + protected DiskDao getDiskDao() { return DbFacade.getInstance().getDiskDao(); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverFromExportDomain.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverFromExportDomain.java index 9a60621..4bc5065 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverFromExportDomain.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverFromExportDomain.java @@ -11,11 +11,12 @@ private Guid storagePoolId; private Guid storageDomainId; - protected Boolean cachedPostZero; private VM vm; - public MemoryImageRemoverFromExportDomain(VM vm, TaskHandlerCommand<?> enclosingCommand, - Guid storagePoolId, Guid storageDomainId) { + public MemoryImageRemoverFromExportDomain(VM vm, + TaskHandlerCommand<?> enclosingCommand, + Guid storagePoolId, + Guid storageDomainId) { super(enclosingCommand); this.vm = vm; this.storagePoolId = storagePoolId; @@ -36,26 +37,16 @@ @Override protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryImageParams(List<Guid> guids) { return new DeleteImageGroupVDSCommandParameters( - guids.get(1), guids.get(0), guids.get(2), isPostZero(), false); + guids.get(1), getStorageDomainId(guids), guids.get(2), isPostZero(guids), false); } @Override protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryConfParams(List<Guid> guids) { return new DeleteImageGroupVDSCommandParameters( - guids.get(1), guids.get(0), guids.get(4), isPostZero(), false); + guids.get(1), getStorageDomainId(guids), guids.get(4), isPostZero(guids), false); } - /** - * We set the post zero field on memory image deletion from export domain as we do - * when it is deleted from data domain even though the export domain is NFS and NFS - * storage do the wipe on its own, in order to be compliance with the rest of the - * code that do the same, and to be prepared for supporting export domains which - * are not NFS. - */ - protected boolean isPostZero() { - if (cachedPostZero == null) { - cachedPostZero = isDiskWithWipeAfterDeleteExist(vm.getDiskMap().values()); - } - return cachedPostZero; + protected boolean isPostZero(List<Guid> guids) { + return isPostZero(vm.getDiskMap().values(), getStorageDomainId(guids)); } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java index 825f7da..ac6893d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java @@ -12,12 +12,12 @@ public class MemoryImageRemoverOnDataDomain extends MemoryImageRemover { - protected Boolean cachedPostZero; private Guid vmId; private boolean removeOnlyIfNotUsedAtAll; public MemoryImageRemoverOnDataDomain(Guid vmId, - TaskHandlerCommand<? extends RemoveMemoryVolumesParameters> enclosingCommand) { + TaskHandlerCommand + <? extends RemoveMemoryVolumesParameters> enclosingCommand) { super(enclosingCommand, false); this.vmId = vmId; removeOnlyIfNotUsedAtAll = enclosingCommand.getParameters().isRemoveOnlyIfNotUsedAtAll(); @@ -30,21 +30,17 @@ @Override protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryImageParams(List<Guid> guids) { return new DeleteImageGroupVDSCommandParameters( - guids.get(1), guids.get(0), guids.get(2), isPostZero(), false); + guids.get(1), getStorageDomainId(guids), guids.get(2), isPostZero(guids), false); } @Override protected DeleteImageGroupVDSCommandParameters buildDeleteMemoryConfParams(List<Guid> guids) { return new DeleteImageGroupVDSCommandParameters( - guids.get(1), guids.get(0), guids.get(4), isPostZero(), false); + guids.get(1), getStorageDomainId(guids), guids.get(4), isPostZero(guids), false); } - protected boolean isPostZero() { - if (cachedPostZero == null) { - cachedPostZero = isDiskWithWipeAfterDeleteExist(getDiskDao().getAllForVm(vmId)); - } - - return cachedPostZero; + protected boolean isPostZero(List<Guid> guids) { + return isPostZero(getDiskDao().getAllForVm(vmId), getStorageDomainId(guids)); } @Override diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java index f8f2e79..b699a64 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImagesHandlerTest.java @@ -1,8 +1,10 @@ package org.ovirt.engine.core.bll; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import java.text.MessageFormat; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -11,6 +13,7 @@ import org.junit.Before; import org.junit.Test; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.StorageType; import org.ovirt.engine.core.compat.Guid; /** A test case for {@link ImagesHandler} */ @@ -118,4 +121,15 @@ assertTrue("Intersection should contain only disk1", intersection.size() == 1 && intersection.contains(disk1)); } + + @Test + public void testShouldWipeAfterDelete() { + for (StorageType storageType : StorageType.values()) { + // Only block domains with wipeAfterDelete = true should be wiped. + assertEquals(MessageFormat.format("Wrong result when requesting to wipe {0}.", storageType), + storageType.isBlockDomain(), ImagesHandler.shouldWipeAfterDelete(true, storageType)); + assertFalse(MessageFormat.format("Wrong result when requesting not to wipe {0}.", storageType), + ImagesHandler.shouldWipeAfterDelete(false, storageType)); + } + } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverTest.java new file mode 100644 index 0000000..0e20e3b --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverTest.java @@ -0,0 +1,51 @@ +package org.ovirt.engine.core.bll.memory; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Before; +import org.junit.Test; +import org.ovirt.engine.core.common.businessentities.Disk; +import org.ovirt.engine.core.common.businessentities.DiskImage; +import java.util.Arrays; + +public class MemoryImageRemoverTest { + + private Disk diskWithWipe, diskWithoutWipe; + + @Before + public void setUp() { + diskWithWipe = new DiskImage(); + diskWithWipe.setWipeAfterDelete(true); + diskWithoutWipe = new DiskImage(); + diskWithoutWipe.setWipeAfterDelete(false); + } + + @Test + public void testIsDiskWithWipeAfterDeleteExistFalseFalse() throws Exception { + assertFalse("isDiskWithWipeAfterDeleteExist should return false " + + "when it gets two disks with 'wipeAfterDelete' = false.", + MemoryImageRemover.isDiskWithWipeAfterDeleteExist(Arrays.asList(diskWithoutWipe, diskWithoutWipe))); + } + + @Test + public void testIsDiskWithWipeAfterDeleteExistTrueTrue() throws Exception { + assertTrue("isDiskWithWipeAfterDeleteExist should return true " + + "when it gets two disks 'wipeAfterDelete' = true.", + MemoryImageRemover.isDiskWithWipeAfterDeleteExist(Arrays.asList(diskWithWipe, diskWithWipe))); + } + + @Test + public void testIsDiskWithWipeAfterDeleteExistTrueFalse() throws Exception { + assertTrue("isDiskWithWipeAfterDeleteExist should return true when it gets one disk " + + "with 'wipeAfterDelete' = true and another one with 'wipeAfterDelete' = false.", + MemoryImageRemover.isDiskWithWipeAfterDeleteExist(Arrays.asList(diskWithWipe, diskWithoutWipe))); + } + + @Test + public void testIsDiskWithWipeAfterDeleteExistFalseTrue() throws Exception { + assertTrue("isDiskWithWipeAfterDeleteExist should return true when it gets one disk " + + "with 'wipeAfterDelete' = false and another one with 'wipeAfterDelete' = true.", + MemoryImageRemover.isDiskWithWipeAfterDeleteExist(Arrays.asList(diskWithoutWipe, diskWithWipe))); + } +} -- To view, visit http://gerrit.ovirt.org/30706 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie8a9932725ef861525f5102e8617938f0e0a71c7 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Idan Shaby <ish...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
