Arik Hadas has uploaded a new change for review. Change subject: core: improve logging on memory volumes removal failure ......................................................................
core: improve logging on memory volumes removal failure Change the logging that is printed when failing to remove memory volumes to provide more information. This patch also contains some refactoring which should make the 'endVmCommand' method in create-all-snapshots command more simple and readable. Change-Id: I67897b5cd07033aff624242cdaa67d517eb14bf9 Signed-off-by: Arik Hadas <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemover.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Snapshot.java 3 files changed, 29 insertions(+), 36 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/39/19639/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java index 7bfb51f..c32936b 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java @@ -213,13 +213,11 @@ @Override protected void endVmCommand() { - boolean taskGroupSucceeded = getParameters().getTaskGroupSuccess(); Snapshot createdSnapshot = getSnapshotDao().get(getVmId(), getParameters().getSnapshotType(), SnapshotStatus.LOCKED); - // if the snapshot was not created the command should be - // handled as a failure - if (createdSnapshot == null) { - taskGroupSucceeded = false; - } + // if the snapshot was not created in the DB + // the command should also be handled as a failure + boolean taskGroupSucceeded = createdSnapshot != null && getParameters().getTaskGroupSuccess(); + if (taskGroupSucceeded) { getSnapshotDao().updateStatus(createdSnapshot.getId(), SnapshotStatus.OK); @@ -227,13 +225,12 @@ performLiveSnapshot(createdSnapshot); } else { // If the created snapshot contains memory, remove the memory volumes as - // they are not in use since no live snapshot was created - if (!createdSnapshot.getMemoryVolume().isEmpty()) { + // they are not going to be in use since no live snapshot is created + if (createdSnapshot.containsMemory()) { logMemorySavingFailed(); - if (!removeMemoryFromSnapshot(createdSnapshot, true)) { - log.errorFormat("Failed to remove unused memory {0} of snapshot {1}", - createdSnapshot.getMemoryVolume(), createdSnapshot.getId()); - } + + getSnapshotDao().removeMemoryFromSnapshot(createdSnapshot.getId()); + removeMemoryVolumesOfSnapshot(createdSnapshot); } } } else { @@ -241,10 +238,7 @@ revertToActiveSnapshot(createdSnapshot.getId()); // If the removed snapshot contained memory, remove the memory volumes // Note that the memory volumes might not have been created - if (!removeMemoryFromSnapshot(createdSnapshot, false)) { - log.warnFormat("Failed to remove memory {0} of snapshot {1}", - createdSnapshot.getMemoryVolume(), createdSnapshot.getId()); - } + removeMemoryVolumesOfSnapshot(createdSnapshot); } else { log.warnFormat("No snapshot was created for VM {0} which is in LOCKED status", getVmId()); } @@ -263,29 +257,21 @@ AuditLogDirector.log(this, AuditLogType.USER_CREATE_LIVE_SNAPSHOT_NO_MEMORY_FAILURE); } - private boolean removeMemoryFromSnapshot(Snapshot snapshot, boolean clearFromDB) { - final String memoryVolume = snapshot.getMemoryVolume(); - if (memoryVolume.isEmpty()) { - return true; - } - - if (clearFromDB) { - getSnapshotDao().removeMemoryFromSnapshot(snapshot.getId()); - } - - return removeMemoryVolumes(memoryVolume); - } - - private boolean removeMemoryVolumes(String memoryVolumes) { - RemoveMemoryVolumesParameters parameters = new RemoveMemoryVolumesParameters(memoryVolumes, getVmId()); + private void removeMemoryVolumesOfSnapshot(Snapshot snapshot) { + RemoveMemoryVolumesParameters parameters = new RemoveMemoryVolumesParameters(snapshot.getMemoryVolume(), getVmId()); parameters.setParentCommand(getActionType()); parameters.setEntityInfo(getParameters().getEntityInfo()); parameters.setParentParameters(getParameters()); - return getBackend().runInternalAction( + VdcReturnValueBase retVal = getBackend().runInternalAction( VdcActionType.RemoveMemoryVolumes, parameters, - ExecutionHandler.createDefaultContexForTasks(getExecutionContext())).getSucceeded(); + ExecutionHandler.createDefaultContexForTasks(getExecutionContext())); + + if (!retVal.getSucceeded()) { + log.errorFormat("Failed to remove memory volumes of snapshot {0} ({1})", + snapshot.getDescription(), snapshot.getId()); + } } private boolean isLiveSnapshotApplicable() { 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 bc66680..f876fbc 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 @@ -116,11 +116,14 @@ return guid; } else { + boolean imageDoesNotExist = vdsRetValue.getVdsError().getCode() == VdcBllErrors.ImageDoesNotExistInDomainError; + if (!imageDoesNotExist) { + log.errorFormat("Could not remove memory image: {0}", parameters); + } enclosingCommand.deleteAsyncTaskPlaceHolder(taskKey); - // otherwise, if the command failed because the image already does not exist, + // otherwise, if the command failed because the image does not exist, // no need to create and monitor task, so we return empty guid to mark this state - return vdsRetValue.getVdsError().getCode() == VdcBllErrors.ImageDoesNotExistInDomainError ? - Guid.Empty : null; + return imageDoesNotExist ? Guid.Empty : null; } } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Snapshot.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Snapshot.java index bd91744..56ecadf 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Snapshot.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/Snapshot.java @@ -197,6 +197,10 @@ return memoryVolume; } + public boolean containsMemory() { + return !memoryVolume.isEmpty(); + } + public void setMemoryVolume(String memoryVolume) { this.memoryVolume = memoryVolume == null ? "" : memoryVolume; } -- To view, visit http://gerrit.ovirt.org/19639 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I67897b5cd07033aff624242cdaa67d517eb14bf9 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
