Arik Hadas has uploaded a new change for review.
Change subject: core: minor cleanups in MemoryImageRemover class
......................................................................
core: minor cleanups in MemoryImageRemover class
1. Define constant instead of using '6' explicitly in the code
2. Rename shouldRemoveMemorySnapshotVolumes method to
isMemoryStateRemovable
3. Add log printing when the given memory state representation is not in
the expected format (doesn't have 6 UUIDs separated by comma)
4. Rename local fields
5. Remove 'vm' and 'cachedPostZero' since they are not used by this
class (so they are moved to the subclasses)
6. Add a boolean class field which indicates whether to start polling
the created tasks or not, instead of indicating that by setting a
parameter of the removeMemoryVolumes method
Change-Id: I14cb0bb13d0d103bbc170abcf0348a6909051f2c
Signed-off-by: Arik Hadas <aha...@redhat.com>
---
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
3 files changed, 54 insertions(+), 42 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/28/16828/1
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 a4282b4..6410d66 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
@@ -8,34 +8,39 @@
import org.ovirt.engine.core.bll.VmCommand;
import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand;
import org.ovirt.engine.core.common.VdcObjectType;
-import org.ovirt.engine.core.common.businessentities.VM;
import
org.ovirt.engine.core.common.vdscommands.DeleteImageGroupVDSCommandParameters;
import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
import org.ovirt.engine.core.compat.Guid;
-import org.ovirt.engine.core.dal.dbbroker.DbFacade;
import org.ovirt.engine.core.utils.GuidUtils;
+import org.ovirt.engine.core.utils.log.Log;
+import org.ovirt.engine.core.utils.log.LogFactory;
public abstract class MemoryImageRemover {
+ private static final Log log = LogFactory.getLog(MemoryImageRemover.class);
+ private static final int NUM_OF_UUIDS_IN_MEMORT_STATE = 6;
private TaskHandlerCommand<?> enclosingCommand;
- protected VM vm;
- protected Boolean cachedPostZero;
+ private boolean startPollingTasks;
- public MemoryImageRemover(VM vm, TaskHandlerCommand<?> enclosingCommand) {
+ public MemoryImageRemover(TaskHandlerCommand<?> enclosingCommand) {
this.enclosingCommand = enclosingCommand;
- this.vm = vm;
}
- protected abstract boolean shouldRemoveMemorySnapshotVolumes(String
memoryVolume);
+ public MemoryImageRemover(TaskHandlerCommand<?> enclosingCommand, boolean
startPollingTasks) {
+ this(enclosingCommand);
+ this.startPollingTasks = startPollingTasks;
+ }
+
+ protected abstract boolean isMemoryStateRemovable(String memoryVolume);
protected abstract DeleteImageGroupVDSCommandParameters
buildDeleteMemoryImageParams(List<Guid> guids);
protected abstract DeleteImageGroupVDSCommandParameters
buildDeleteMemoryConfParams(List<Guid> guids);
public void removeMemoryVolume(String memoryVolumes) {
- if (shouldRemoveMemorySnapshotVolumes(memoryVolumes)) {
- removeMemoryVolumes(memoryVolumes, false);
+ if (isMemoryStateRemovable(memoryVolumes)) {
+ removeMemoryVolumes(memoryVolumes);
}
}
@@ -45,34 +50,34 @@
}
}
- protected boolean removeMemoryVolumes(String memVols, boolean
startPollingTasks) {
+ private boolean removeMemoryVolumes(String memVols) {
List<Guid> guids = GuidUtils.getGuidListFromString(memVols);
- if (guids.size() == 6) {
-
- Guid guid1 = removeMemoryImage(guids);
- if (guid1 == null) {
- return false;
- }
-
- Guid guid2 = removeConfImage(guids);
-
- if (startPollingTasks) {
- AsyncTaskManager.getInstance().StartPollingTask(guid1);
-
- if (guid2 != null) {
- AsyncTaskManager.getInstance().StartPollingTask(guid2);
- }
- }
-
- return guid2 != null;
+ if (guids.size() != NUM_OF_UUIDS_IN_MEMORT_STATE) {
+ log.warnFormat("Cannot remove memory volumes, invalid format:
{0}", memVols);
+ return true;
}
- return true;
+ Guid memoryImageRemovalTaskId = removeMemoryImage(guids);
+ if (memoryImageRemovalTaskId == null) {
+ return false;
+ }
+
+ Guid confImageRemovalTaskId = removeConfImage(guids);
+
+ if (startPollingTasks) {
+
AsyncTaskManager.getInstance().StartPollingTask(memoryImageRemovalTaskId);
+
+ if (confImageRemovalTaskId != null) {
+
AsyncTaskManager.getInstance().StartPollingTask(confImageRemovalTaskId);
+ }
+ }
+
+ return confImageRemovalTaskId != null;
}
protected Guid removeMemoryImage(List<Guid> guids) {
- Guid taskId1 =
enclosingCommand.persistAsyncTaskPlaceHolder(VmCommand.DELETE_PRIMARY_IMAGE_TASK_KEY);
+ Guid taskId =
enclosingCommand.persistAsyncTaskPlaceHolder(VmCommand.DELETE_PRIMARY_IMAGE_TASK_KEY);
// delete first image
// the next 'DeleteImageGroup' command should also take care of the
image removal:
VDSReturnValue vdsRetValue =
@@ -87,15 +92,15 @@
return null;
}
- Guid guid1 =
- enclosingCommand.createTask(taskId1,
vdsRetValue.getCreationInfo(),
+ Guid guid =
+ enclosingCommand.createTask(taskId,
vdsRetValue.getCreationInfo(),
enclosingCommand.getActionType(),
VdcObjectType.Storage, guids.get(0));
- enclosingCommand.getTaskIdList().add(guid1);
- return guid1;
+ enclosingCommand.getTaskIdList().add(guid);
+ return guid;
}
protected Guid removeConfImage(List<Guid> guids) {
- Guid taskId2 =
enclosingCommand.persistAsyncTaskPlaceHolder(VmCommand.DELETE_SECONDARY_IMAGES_TASK_KEY);
+ Guid taskId =
enclosingCommand.persistAsyncTaskPlaceHolder(VmCommand.DELETE_SECONDARY_IMAGES_TASK_KEY);
// delete second image
// the next 'DeleteImageGroup' command should also take care of the
image removal:
VDSReturnValue vdsRetValue =
@@ -110,9 +115,9 @@
return null;
}
- Guid guid2 = enclosingCommand.createTask(taskId2,
vdsRetValue.getCreationInfo(),
+ Guid guid = enclosingCommand.createTask(taskId,
vdsRetValue.getCreationInfo(),
enclosingCommand.getActionType());
- enclosingCommand.getTaskIdList().add(guid2);
- return guid2;
+ enclosingCommand.getTaskIdList().add(guid);
+ return guid;
}
}
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 66d5605..225ca63 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
@@ -12,10 +12,13 @@
private Guid storagePoolId;
private Guid storageDomainId;
+ protected Boolean cachedPostZero;
+ private VM vm;
public MemoryImageRemoverFromExportDomain(VM vm, TaskHandlerCommand<?>
enclosingCommand,
Guid storagePoolId, Guid storageDomainId) {
- super(vm, enclosingCommand);
+ super(enclosingCommand);
+ this.vm = vm;
this.storagePoolId = storagePoolId;
this.storageDomainId = storageDomainId;
}
@@ -54,7 +57,7 @@
}
@Override
- protected boolean shouldRemoveMemorySnapshotVolumes(String memoryVolume) {
+ protected boolean isMemoryStateRemovable(String memoryVolume) {
return !memoryVolume.isEmpty();
}
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 9f3229c..dd79e2b 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
@@ -11,8 +11,12 @@
public class MemoryImageRemoverOnDataDomain extends MemoryImageRemover {
+ protected Boolean cachedPostZero;
+ private VM vm;
+
public MemoryImageRemoverOnDataDomain(VM vm, TaskHandlerCommand<?>
enclosingCommand) {
- super(vm, enclosingCommand);
+ super(enclosingCommand);
+ this.vm = vm;
}
@Override
@@ -47,7 +51,7 @@
* volumes should be removed only if the only snapshot that points to them
is removed
*/
@Override
- protected boolean shouldRemoveMemorySnapshotVolumes(String memoryVolume) {
+ protected boolean isMemoryStateRemovable(String memoryVolume) {
return !memoryVolume.isEmpty() &&
getDbFacade().getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 1;
}
--
To view, visit http://gerrit.ovirt.org/16828
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I14cb0bb13d0d103bbc170abcf0348a6909051f2c
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
