Hello Liron Ar,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/19524
to review the following change.
Change subject: core: revert when failing to export a template
......................................................................
core: revert when failing to export a template
When failing to export a template, the engine can attempt to delete
the already exported images.
Change-Id: I8d80c5d15f7321f67d54e7593cb2da1fd7186a25
Bug-Url: https://bugzilla.redhat.com/967628
Signed-off-by: Liron Aravot <[email protected]>
Signed-off-by: Tal Nisan <[email protected]>
---
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/ExportVmTemplateCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java
5 files changed, 54 insertions(+), 12 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/24/19524/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 a62eefc..fe0cd9f 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
@@ -19,6 +19,7 @@
import org.ovirt.engine.core.common.businessentities.CopyVolumeType;
import org.ovirt.engine.core.common.businessentities.Disk;
import org.ovirt.engine.core.common.businessentities.DiskImage;
+import org.ovirt.engine.core.common.businessentities.ImageDbOperationScope;
import org.ovirt.engine.core.common.businessentities.ImageOperation;
import org.ovirt.engine.core.common.businessentities.StorageDomain;
import org.ovirt.engine.core.common.errors.VdcBLLException;
@@ -68,6 +69,7 @@
srcStorageDomainId,
diskImage.getId(),
diskImage.getImageId(), parentCommandType);
+ parameters.setRevertDbOperationScope(ImageDbOperationScope.IMAGE);
VdcReturnValueBase result = executeChildCopyingCommand(parameters);
handleCopyResult(diskImage, newDiskImage, result);
}
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 265a80d..3bc2bf6 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
@@ -184,9 +184,7 @@
@Override
protected void revertTasks() {
- // Revert should be performed only for AddVmFromSnapshot at this point.
- if (getParameters().getParentCommand() ==
VdcActionType.AddVmFromSnapshot || getParameters().getParentCommand() ==
VdcActionType.ImportVm
- || getParameters().getParentCommand() ==
VdcActionType.ImportVmTemplate) {
+ if (getParameters().getRevertDbOperationScope() != null) {
Guid destImageId = getParameters().getDestinationImageId();
RemoveImageParameters removeImageParams =
new RemoveImageParameters(destImageId);
@@ -196,6 +194,9 @@
} else {
removeImageParams.setParentParameters(removeImageParams);
removeImageParams.setParentCommand(VdcActionType.RemoveImage);
+
removeImageParams.setStorageDomainId(getParameters().getStorageDomainId());
+
removeImageParams.setDbOperationScope(getParameters().getRevertDbOperationScope());
+
removeImageParams.setShouldLockImage(getParameters().isShouldLockImageOnRevert());
}
removeImageParams.setEntityInfo(new EntityInfo(VdcObjectType.Disk,
getDestinationImageId()));
// Setting the image as the monitored entity, so there will not be
dependency
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmTemplateCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmTemplateCommand.java
index c32a9b0..0f416a6 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmTemplateCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmTemplateCommand.java
@@ -11,10 +11,12 @@
import org.ovirt.engine.core.common.VdcObjectType;
import org.ovirt.engine.core.common.action.MoveOrCopyImageGroupParameters;
import org.ovirt.engine.core.common.action.MoveOrCopyParameters;
+import org.ovirt.engine.core.common.action.VdcActionParametersBase;
import org.ovirt.engine.core.common.action.VdcActionType;
import org.ovirt.engine.core.common.action.VdcReturnValueBase;
import org.ovirt.engine.core.common.businessentities.CopyVolumeType;
import org.ovirt.engine.core.common.businessentities.DiskImage;
+import org.ovirt.engine.core.common.businessentities.ImageDbOperationScope;
import org.ovirt.engine.core.common.businessentities.StorageDomainType;
import org.ovirt.engine.core.common.errors.VdcBLLException;
import org.ovirt.engine.core.common.errors.VdcBllMessages;
@@ -64,6 +66,8 @@
p.setVolumeFormat(disk.getVolumeFormat());
p.setVolumeType(disk.getVolumeType());
p.setForceOverride(getParameters().getForceOverride());
+ p.setRevertDbOperationScope(ImageDbOperationScope.NONE);
+ p.setShouldLockImageOnRevert(false);
p.setSourceDomainId(imageFromSourceDomainMap.get(disk.getId()).getStorageIds().get(0));
VdcReturnValueBase vdcRetValue =
Backend.getInstance().runInternalAction(
VdcActionType.CopyImageGroup,
@@ -166,15 +170,28 @@
@Override
protected void incrementDbGeneration() {
- Map<Guid, KeyValuePairCompat<String, List<Guid>>> metaDictionary =
- new HashMap<Guid, KeyValuePairCompat<String, List<Guid>>>();
- OvfDataUpdater.getInstance().loadTemplateData(getVmTemplate());
- VmTemplateHandler.UpdateDisksFromDb(getVmTemplate());
- // update the target (export) domain
-
OvfDataUpdater.getInstance().buildMetadataDictionaryForTemplate(getVmTemplate(),
metaDictionary);
-
OvfDataUpdater.getInstance().executeUpdateVmInSpmCommand(getVmTemplate().getStoragePoolId(),
- metaDictionary,
- getParameters().getStorageDomainId());
+ // we want to export the Template's ovf only in case that all tasks
has succeeded, otherwise we will attempt to
+ // revert
+ // and there's no need for exporting the template's ovf.
+ if (getParameters().getTaskGroupSuccess()) {
+ Map<Guid, KeyValuePairCompat<String, List<Guid>>> metaDictionary =
+ new HashMap<Guid, KeyValuePairCompat<String,
List<Guid>>>();
+ OvfDataUpdater.getInstance().loadTemplateData(getVmTemplate());
+ VmTemplateHandler.UpdateDisksFromDb(getVmTemplate());
+ // update the target (export) domain
+
OvfDataUpdater.getInstance().buildMetadataDictionaryForTemplate(getVmTemplate(),
metaDictionary);
+
OvfDataUpdater.getInstance().executeUpdateVmInSpmCommand(getVmTemplate().getStoragePoolId(),
+ metaDictionary,
+ getParameters().getStorageDomainId());
+ }
+ }
+
+ @Override
+ protected void endActionOnAllImageGroups() {
+ for (VdcActionParametersBase p :
getParameters().getImagesParameters()) {
+ p.setTaskGroupSuccess(getParameters().getTaskGroupSuccess());
+ getBackend().EndAction(getImagesActionType(), p);
+ }
}
@Override
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 df1b9d7..56562ac 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
@@ -46,6 +46,7 @@
import org.ovirt.engine.core.common.businessentities.DiskImageDynamic;
import org.ovirt.engine.core.common.businessentities.DiskInterface;
import org.ovirt.engine.core.common.businessentities.Entities;
+import org.ovirt.engine.core.common.businessentities.ImageDbOperationScope;
import org.ovirt.engine.core.common.businessentities.Snapshot;
import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotStatus;
import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType;
@@ -759,6 +760,7 @@
params.setStoragePoolId(getParameters().getStoragePoolId());
params.setImportEntity(true);
params.setEntityInfo(new EntityInfo(VdcObjectType.VM,
getVm().getId()));
+ params.setRevertDbOperationScope(ImageDbOperationScope.IMAGE);
params.setQuotaId(disk.getQuotaId() != null ? disk.getQuotaId() :
getParameters().getQuotaId());
if (getParameters().getVm().getDiskMap() != null
&&
getParameters().getVm().getDiskMap().containsKey(originalDiskId)) {
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java
index 40c9c71..171e574 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/MoveOrCopyImageGroupParameters.java
@@ -1,6 +1,7 @@
package org.ovirt.engine.core.common.action;
import org.ovirt.engine.core.common.businessentities.CopyVolumeType;
+import org.ovirt.engine.core.common.businessentities.ImageDbOperationScope;
import org.ovirt.engine.core.common.businessentities.ImageOperation;
import org.ovirt.engine.core.common.businessentities.VolumeFormat;
import org.ovirt.engine.core.common.businessentities.VolumeType;
@@ -17,6 +18,8 @@
private boolean forceOverride;
private Guid sourceDomainId;
private Guid destImageGroupId;
+ private ImageDbOperationScope revertDbOperationScope;
+ private boolean shouldLockImageOnRevert;
public MoveOrCopyImageGroupParameters() {
operation = ImageOperation.Unassigned;
@@ -51,6 +54,7 @@
setDestinationImageId(leafSnapshotID);
setDestImageGroupId(imageGroupId);
copyVolumeType = CopyVolumeType.SharedVol;
+ setShouldLockImageOnRevert(true);
}
public MoveOrCopyImageGroupParameters(Guid containerId,
@@ -135,4 +139,20 @@
public void setSourceDomainId(Guid value) {
sourceDomainId = value;
}
+
+ public ImageDbOperationScope getRevertDbOperationScope() {
+ return revertDbOperationScope;
+ }
+
+ public void setRevertDbOperationScope(ImageDbOperationScope
revertDbOperationScope) {
+ this.revertDbOperationScope = revertDbOperationScope;
+ }
+
+ public boolean isShouldLockImageOnRevert() {
+ return shouldLockImageOnRevert;
+ }
+
+ public void setShouldLockImageOnRevert(boolean shouldLockImageOnRevert) {
+ this.shouldLockImageOnRevert = shouldLockImageOnRevert;
+ }
}
--
To view, visit http://gerrit.ovirt.org/19524
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8d80c5d15f7321f67d54e7593cb2da1fd7186a25
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.3
Gerrit-Owner: Tal Nisan <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
