Allon Mureinik has uploaded a new change for review.
Change subject: core: simpler syntax for snapshotEntityStatus
......................................................................
core: simpler syntax for snapshotEntityStatus
Added the
CompensationContext#snapshotEntityStatus(BusinessEntityWithStatus)
method and used it wherever possible.
Rationale and benefits:
1. Most of the time (i.e., all the calls except one), when taking a
snapshot of an entity's status, you use the status property of the
entity, which makes for an annoying call:
getCompensationContext().snapshotEntutyStatus(e, e.getStatus());
The proposed syntax is more straight-forward and simpler to
understand:
getCompensationContext().snapshotEntutyStatus(e);
2. There is no type safety in the original syntax, whereas the new
syntax offers some minimal sanity validation (e.g., when taking
a snapshot of a VM's status, you know the status is represented
by the VMStatus enum).
Change-Id: Ib074a96e93f1c68637306b670b70983f26042514
Signed-off-by: Allon Mureinik <amure...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateHandler.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/CompensationContext.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/DefaultCompensationContext.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/NoOpCompensationContext.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java
16 files changed, 40 insertions(+), 21 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/59/18859/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java
index eba6f10..64a9992 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromSnapshotCommand.java
@@ -359,7 +359,7 @@
public Void runInTransaction() {
// Assumption - a snapshot can be locked only if in status OK,
so if canDoAction passed
// this is the status of the snapshot. In addition the newly
added VM is in down status
- getCompensationContext().snapshotEntityStatus(getSnapshot(),
getSnapshot().getStatus());
+ getCompensationContext().snapshotEntityStatus(getSnapshot());
getSnapshotDao().updateStatus(sourceSnapshotId,
SnapshotStatus.LOCKED);
lockVmWithCompensationIfNeeded();
getCompensationContext().stateChanged();
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java
index 24ba417..ad3c1f8 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/BaseImagesCommand.java
@@ -297,7 +297,7 @@
TransactionSupport.executeInNewTransaction(new
TransactionMethod<Void>() {
@Override
public Void runInTransaction() {
-
getCompensationContext().snapshotEntityStatus(diskImage.getImage(),
diskImage.getImageStatus());
+
getCompensationContext().snapshotEntityStatus(diskImage.getImage());
getCompensationContext().stateChanged();
setImageStatus(ImageStatus.LOCKED);
return null;
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommand.java
index ed2f709..560c467 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetDiskAlignmentCommand.java
@@ -189,12 +189,11 @@
protected void acquireExclusiveDiskDbLocks() {
if (isImageExclusiveLockNeeded()) {
final DiskImage diskImage = (DiskImage) getDisk();
- final ImageStatus imgStatus = diskImage.getImageStatus();
TransactionSupport.executeInNewTransaction(new
TransactionMethod<Void>() {
@Override
public Void runInTransaction() {
-
getCompensationContext().snapshotEntityStatus(diskImage.getImage(), imgStatus);
+
getCompensationContext().snapshotEntityStatus(diskImage.getImage());
getCompensationContext().stateChanged();
diskImage.setImageStatus(ImageStatus.LOCKED);
ImagesHandler.updateImageStatus(diskImage.getImageId(),
ImageStatus.LOCKED);
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java
index 2b9447d..1d6d62e 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HibernateVmCommand.java
@@ -100,7 +100,7 @@
new TransactionMethod<Object>() {
@Override
public Object runInTransaction() {
-
getCompensationContext().snapshotEntityStatus(getVm().getDynamicData(),
getVm().getStatus());
+
getCompensationContext().snapshotEntityStatus(getVm().getDynamicData());
// Set the VM to SavingState to lock the VM,to
avoid situation of multi VM hibernation.
getVm().setStatus(VMStatus.PreparingForHibernate);
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 7e3992e..87a5fd5 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
@@ -153,7 +153,7 @@
@Override
public Void runInTransaction() {
- getCompensationContext().snapshotEntityStatus(snapshot,
snapshot.getStatus());
+ getCompensationContext().snapshotEntityStatus(snapshot);
getSnapshotDao().updateStatus(
getParameters().getSnapshotId(),
SnapshotStatus.LOCKED);
getCompensationContext().stateChanged();
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java
index f1e48ee..f0a0eb4 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/StopVmCommandBase.java
@@ -161,7 +161,7 @@
TransactionSupport.executeInNewTransaction(new
TransactionMethod<Void>() {
@Override
public Void runInTransaction() {
-
getCompensationContext().snapshotEntityStatus(getVm().getDynamicData(),getVm().getStatus());
+
getCompensationContext().snapshotEntityStatus(getVm().getDynamicData());
updateVmData(getVm().getDynamicData());
getCompensationContext().stateChanged();
return null;
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java
index fa8ed65..d9cf697 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVdsCommand.java
@@ -169,7 +169,7 @@
// add old vds dynamic data to compensation context. This
// way the status will revert back to what it was before
// starting installation process
-
getCompensationContext().snapshotEntityStatus(_oldVds.getDynamicData(),
_oldVds.getDynamicData().getStatus());
+
getCompensationContext().snapshotEntityStatus(_oldVds.getDynamicData());
getCompensationContext().stateChanged();
return;
}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
index de95898..eea19b2 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
@@ -526,12 +526,11 @@
private void lockImageInDb() {
final DiskImage diskImage = (DiskImage) getOldDisk();
- final ImageStatus imgStatus = diskImage.getImageStatus();
TransactionSupport.executeInNewTransaction(new
TransactionMethod<Void>() {
@Override
public Void runInTransaction() {
-
getCompensationContext().snapshotEntityStatus(diskImage.getImage(), imgStatus);
+
getCompensationContext().snapshotEntityStatus(diskImage.getImage());
getCompensationContext().stateChanged();
diskImage.setImageStatus(ImageStatus.LOCKED);
ImagesHandler.updateImageStatus(diskImage.getImageId(),
ImageStatus.LOCKED);
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
index 2adbd1d..dd4f716 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
@@ -152,7 +152,7 @@
@Override
public Void runInTransaction() {
- compensationContext.snapshotEntityStatus(vm, vm.getStatus());
+ compensationContext.snapshotEntityStatus(vm);
LockVm(vm.getId());
compensationContext.stateChanged();
return null;
@@ -219,7 +219,7 @@
@Override
public Void runInTransaction() {
- compensationContext.snapshotEntityStatus(vm.getDynamicData(),
vm.getStatus());
+ compensationContext.snapshotEntityStatus(vm.getDynamicData());
UnLockVm(vm);
compensationContext.stateChanged();
return null;
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateHandler.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateHandler.java
index 0182766..b5a7053 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateHandler.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateHandler.java
@@ -104,7 +104,7 @@
VmTemplate vmTemplate =
DbFacade.getInstance().getVmTemplateDao().get(vmTemplateGuid);
if (vmTemplate != null) {
if (compensationContext != null) {
- compensationContext.snapshotEntityStatus(vmTemplate,
vmTemplate.getStatus());
+ compensationContext.snapshotEntityStatus(vmTemplate);
}
vmTemplate.setStatus(status);
DbFacade.getInstance().getVmTemplateDao().update(vmTemplate);
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/CompensationContext.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/CompensationContext.java
index f490daf..2e70d39 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/CompensationContext.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/CompensationContext.java
@@ -3,6 +3,7 @@
import java.util.Collection;
import org.ovirt.engine.core.common.businessentities.BusinessEntity;
+import org.ovirt.engine.core.common.businessentities.BusinessEntityWithStatus;
/**
* The compensation context contains information needed for compensating
failed command executions.
@@ -54,7 +55,16 @@
* @param status
* The status to snapshot.
*/
- public void snapshotEntityStatus(BusinessEntity<?> entity, Enum<?> status);
+ public <T extends Enum<?>> void
snapshotEntityStatus(BusinessEntityWithStatus<?, T> entity, T status);
+
+ /**
+ * Snapshot the entity status only, so that in case of compensation for
the entity, the status will be updated to
+ * it's original value.
+ *
+ * @param entity
+ * The entity for which to save the status snapshot.
+ */
+ public <T extends Enum<?>> void
snapshotEntityStatus(BusinessEntityWithStatus<?, T> entity);
/**
* Signify that the command state had changed and the transaction is about
to end, so that the snapshots can
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/DefaultCompensationContext.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/DefaultCompensationContext.java
index 2e3da7f..f27e6b8 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/DefaultCompensationContext.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/DefaultCompensationContext.java
@@ -11,6 +11,7 @@
import org.ovirt.engine.core.common.businessentities.BusinessEntitySnapshot;
import
org.ovirt.engine.core.common.businessentities.BusinessEntitySnapshot.EntityStatusSnapshot;
import
org.ovirt.engine.core.common.businessentities.BusinessEntitySnapshot.SnapshotType;
+import org.ovirt.engine.core.common.businessentities.BusinessEntityWithStatus;
import org.ovirt.engine.core.compat.Guid;
import org.ovirt.engine.core.dal.dbbroker.DbFacade;
import org.ovirt.engine.core.dao.BusinessEntitySnapshotDAO;
@@ -96,13 +97,18 @@
}
@Override
- public void snapshotEntityStatus(BusinessEntity<?> entity, Enum<?> status)
{
+ public <T extends Enum<?>> void
snapshotEntityStatus(BusinessEntityWithStatus<?, T> entity, T status) {
EntityStatusSnapshot snapshot = new EntityStatusSnapshot();
snapshot.setId(entity.getId());
snapshot.setStatus(status);
snapshotEntityInMemory(entity, snapshot,
SnapshotType.CHANGED_STATUS_ONLY);
}
+ @Override
+ public <T extends Enum<?>> void
snapshotEntityStatus(BusinessEntityWithStatus<?, T> entity) {
+ snapshotEntityStatus(entity, entity.getStatus());
+ }
+
/**
* Save a snapshot of the entity but only if it is new to this context.
*
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/NoOpCompensationContext.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/NoOpCompensationContext.java
index b5de37e..1f33a3e 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/NoOpCompensationContext.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/context/NoOpCompensationContext.java
@@ -3,6 +3,7 @@
import java.util.Collection;
import org.ovirt.engine.core.common.businessentities.BusinessEntity;
+import org.ovirt.engine.core.common.businessentities.BusinessEntityWithStatus;
/**
* An implementation of COmpensation Context that does nothing - will be used
by commands that do not implement
@@ -24,7 +25,11 @@
}
@Override
- public void snapshotEntityStatus(BusinessEntity<?> entity, Enum<?> status)
{
+ public <T extends Enum<?>> void
snapshotEntityStatus(BusinessEntityWithStatus<?, T> entity, T status) {
+ }
+
+ @Override
+ public <T extends Enum<?>> void
snapshotEntityStatus(BusinessEntityWithStatus<?, T> entity) {
}
@Override
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
index 28d6fbd..ad9e112 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
@@ -174,7 +174,7 @@
executeInNewTransaction(new TransactionMethod<Object>() {
@Override
public Object runInTransaction() {
-
getCompensationContext().snapshotEntityStatus(getStoragePool(),
getStoragePool().getStatus());
+
getCompensationContext().snapshotEntityStatus(getStoragePool());
getStoragePool().setStatus(StoragePoolStatus.Maintenance);
getStoragePoolDAO().updateStatus(getStoragePool().getId(),
getStoragePool().getStatus());
getCompensationContext().stateChanged();
@@ -281,7 +281,7 @@
_newMasterStorageDomainId = newMaster.getId();
if (!duringReconstruct) {
newMasterMap.setStatus(StorageDomainStatus.Unknown);
-
getCompensationContext().snapshotEntityStatus(newMasterMap,
newMasterMap.getStatus());
+
getCompensationContext().snapshotEntityStatus(newMasterMap);
newMaster.setStatus(StorageDomainStatus.Locked);
getStoragePoolIsoMapDAO().updateStatus(newMasterMap.getId(),
newMasterMap.getStatus());
}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
index 6052e60..957bc1f 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
@@ -208,7 +208,7 @@
if (getStorageDomain() != null &&
getStorageDomain().getStoragePoolId() != null) {
StoragePoolIsoMap map =
getStorageDomain().getStoragePoolIsoMapData();
if(context != null) {
- context.snapshotEntityStatus(map, map.getStatus());
+ context.snapshotEntityStatus(map);
}
getStorageDomain().setStatus(status);
getStoragePoolIsoMapDAO().updateStatus(map.getId(), status);
@@ -315,7 +315,7 @@
@Override
public StoragePoolIsoMap runInTransaction() {
CompensationContext context = getCompensationContext();
- context.snapshotEntityStatus(map, map.getStatus());
+ context.snapshotEntityStatus(map);
map.setStatus(status);
getStoragePoolIsoMapDAO().updateStatus(map.getId(),
map.getStatus());
getCompensationContext().stateChanged();
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java
index 9e743d0..698e752 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java
@@ -260,7 +260,7 @@
CompensationContext context = getCompensationContext();
for (StorageDomain domain : domains) {
for (StoragePoolIsoMap map : getStoragePoolIsoMap(domain))
{
- context.snapshotEntityStatus(map, map.getStatus());
+ context.snapshotEntityStatus(map);
updateStatus(map, status);
}
}
--
To view, visit http://gerrit.ovirt.org/18859
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib074a96e93f1c68637306b670b70983f26042514
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <amure...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
