Arik Hadas has posted comments on this change. Change subject: core: managed removal of memory volumes in negative flows ......................................................................
Patch Set 3: (3 comments) http://gerrit.ovirt.org/#/c/23222/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java: Line 1178: private void removeMemoryVolumes(String memoryVolume, Guid vmId) { Line 1179: VdcReturnValueBase retVal = getBackend().runInternalAction( Line 1180: VdcActionType.RemoveMemoryVolumes, Line 1181: new RemoveMemoryVolumesParameters(memoryVolume, vmId), Line 1182: ExecutionHandler.createDefaultContexForTasks(getExecutionContext())); > seems you're right, I'll change Done Line 1183: Line 1184: if (!retVal.getSucceeded()) { Line 1185: log.errorFormat("Failed to remove memory volumes: {0}", memoryVolume); Line 1186: } http://gerrit.ovirt.org/#/c/23222/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java: Line 145: private RemoveMemoryVolumesParameters buildRemoveMemoryVolumesParameters(String memoryState, Guid vmId) { Line 146: RemoveMemoryVolumesParameters parameters = Line 147: new RemoveMemoryVolumesParameters(memoryState, getVmId()); Line 148: parameters.setParentCommand(getActionType()); Line 149: parameters.setParentParameters(getParameters()); > The parent parameters? because we want the RemoveVmCommand to poll those ta that was a wrong answer, the answer is that inside the RemoveMemoryVolumeCommand we need to know in which flow we are in order to be able to check if we can remove the volumes or not (if they are unused) Line 150: parameters.setEntityInfo(getParameters().getEntityInfo()); Line 151: Line 152: return parameters; Line 153: } http://gerrit.ovirt.org/#/c/23222/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/memory/MemoryImageRemoverOnDataDomain.java: Line 51: Line 52: // from RemoveVmCommand this command is called after the snapshot Line 53: // is removed from the DB Line 54: return enclosingCommand.getParameters().getParentCommand() == VdcActionType.RemoveVm ? Line 55: getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 0 > I prefer to keep it that way because that way it is clear on which flow we I think I understand the second part and I moved the call to the method in the DAO class to be invoked before Line 56: : getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 1; Line 57: } Line 58: Line 59: protected SnapshotDao getSnapshotDao() { -- To view, visit http://gerrit.ovirt.org/23222 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4b270ec0e1ab41cae34459dde9f9cf47b1b5bdf Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
