Michael Kublin has posted comments on this change.
Change subject: core: Remove image when VDSM returns not exist.
......................................................................
Patch Set 3: I would prefer that you didn't submit this
(6 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java
Line 296: if (vdcReturnValue.getSucceeded()) {
Line 297:
getReturnValue().getTaskIdList().addAll(vdcReturnValue.getInternalTaskIdList());
Line 298: setSucceeded(vdcReturnValue.getSucceeded());
Line 299:
Line 300: // If VDSM returns imageDoesNotExistsInDomain then
there is nothing to delete.
Logic is wrong, if we have fault - it is means that
vdcReturnValue.getSucceeded() == false. If we have fault and
vdcReturnValue.getSucceeded() == true it is a bug.
Also line 310 == line 297
Line 301: if (vdcReturnValue.getFault().getError() ==
VdcBllErrors.ImageDoesNotExistInDomainError) {
Line 302: TransactionSupport.executeInNewTransaction(new
TransactionMethod<Void>() {
Line 303: @Override
Line 304: public Void runInTransaction() {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java
Line 87: if (e.getErrorCode() ==
VdcBllErrors.ImageDoesNotExistInDomainError) {
Line 88: getReturnValue().setFault(new VdcFault(e,
e.getVdsError().getCode()));
Line 89: log.errorFormat("Image {0} does not exist in
domain {1}. Removing disk from DB.",
Line 90: getDiskImage().getId(),
Line 91: getParameters().getStorageDomainId());
Maybe you actually want to mark a command as successes, we tried to remove
image but it doesn't exist - we success. I think command should be marked as
successful and log is enough for such case
Line 92: endCommand();
Line 93: } else {
Line 94: throw e;
Line 95: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveTemplateSnapshotCommand.java
Line 18: }
Line 19:
Line 20: @Override
Line 21: protected void executeCommand() {
Line 22: VDSReturnValue vdsReturnValue =
This change is not clear to me, if already done , please introduce
runVdsCommand()
Line 23: Backend
Line 24: .getInstance()
Line 25: .getResourceManager()
Line 26: .RunVdsCommand(
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java
Line 224:
ExecutionHandler.createDefaultContexForTasks(getExecutionContext()));
Line 225:
Line 226: // If all disks of VM returned with
imageDoesNotExistsInDomain then finish the command.
Line 227: if (vdcRetValue.getFault().getError() ==
VdcBllErrors.ImageDoesNotExistInDomainError) {
Line 228: endCommandWithoutTasks();
If only one image is not exist? What this code means?
If RemoveAllVmImages successes and no task remove from db.
Line 229: } else if (vdcRetValue.getSucceeded()) {
Line 230:
getReturnValue().getTaskIdList().addAll(vdcRetValue.getInternalTaskIdList());
Line 231: }
Line 232:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateCommand.java
Line 233: if (!vdcReturnValue.getSucceeded()) {
Line 234: setSucceeded(false);
Line 235: getReturnValue().setFault(vdcReturnValue.getFault());
Line 236: return false;
Line 237: } else if (vdcReturnValue.getFault().getError() ==
VdcBllErrors.ImageDoesNotExistInDomainError) {
same here, one image doesn't exist.
Line 238: HandleEndAction();
Line 239: }
Line 240:
Line 241:
getReturnValue().getTaskIdList().addAll(vdcReturnValue.getInternalTaskIdList());
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateFromImportExportCommand.java
Line 118:
Backend.getInstance().runInternalAction(VdcActionType.RemoveAllVmImages,
Line 119: tempVar2,
Line 120:
ExecutionHandler.createDefaultContexForTasks(getExecutionContext()));
Line 121:
Line 122: // If all disks of template returned with
imageDoesNotExistsInDomain then finish the command.
same here
Line 123: if (getReturnValue().getFault().getError() ==
VdcBllErrors.ImageDoesNotExistInDomainError) {
Line 124: EndRemoveTemplate();
Line 125: } else if (vdcRetValue.getSucceeded()) {
Line 126:
getReturnValue().getTaskIdList().addAll(vdcRetValue.getInternalTaskIdList());
--
To view, visit http://gerrit.ovirt.org/12077
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I36915c44e65e20e3ce222683650a562603e4bb05
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: liron aravot <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
