Maor Lipchuk has posted comments on this change.
Change subject: core: Remove image when VDSM returns not exist.
......................................................................
Patch Set 3: (9 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyImageGroupCommand.java
Line 209: // Setting the image as the monitored entity, so there
will not be dependency
Line 210: VdcReturnValueBase returnValue =
Line 211:
checkAndPerformRollbackUsingCommand(VdcActionType.RemoveImage,
removeImageParams);
Line 212: if (returnValue.getFault().getError() ==
VdcBllErrors.ImageDoesNotExistInDomainError
Line 213: && getParameters().getParentCommand() !=
VdcActionType.ImportVm) {
I tend to agree about it, it is already being working on (Liron's patch) It
would be in a separate patch
Line 214: endSuccessfully();
Line 215: } else if (returnValue.getSucceeded()) {
Line 216: // Starting to monitor the the tasks - RemoveImage is
an internal command
Line 217: // which adds the taskId on the internal task ID list
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveAllVmImagesCommand.java
Line 64:
Backend.getInstance().runInternalAction(VdcActionType.RemoveImage,
Line 65: tempVar,
Line 66:
ExecutionHandler.createDefaultContexForTasks(getExecutionContext()));
Line 67:
Line 68: if (vdcReturnValue.getFault().getError() ==
VdcBllErrors.ImageDoesNotExistInDomainError) {
if I will switch the conditions now it will cause a bug, since the
vdcReturnValue returns true.
How do you suggest that it should be done?
Line 69: imageDoesNotExistsFault =
vdcReturnValue.getFault();
Line 70: } else if (vdcReturnValue.getSucceeded()) {
Line 71: isAllDisksNotExistsInDomain = false;
Line 72:
getReturnValue().getInternalTaskIdList().addAll(vdcReturnValue.getInternalTaskIdList());
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveAllVmTemplateImageTemplatesCommand.java
Line 57: tempVar,
Line 58:
ExecutionHandler.createDefaultContexForTasks(getExecutionContext()));
Line 59:
Line 60: if (vdcReturnValue.getFault().getError() ==
VdcBllErrors.ImageDoesNotExistInDomainError) {
Line 61: imageDoesNotExistsFault =
vdcReturnValue.getFault();
same answer
Line 62: } else if (vdcReturnValue.getSucceeded()) {
Line 63: isAllTemplateDisksNotExistsInDomain = false;
Line 64:
getReturnValue().getInternalTaskIdList().addAll(vdcReturnValue.getInternalTaskIdList());
Line 65: } else {
....................................................
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.
will fix line 310 == 297
I'm not sure I'm following, you wrote that in RemoveImageCommand, the command
should be marked with success = true (Which it is) and in the above comment,
you wrote that vdcReturnValue.getSucceeded() should be false if there is fault.
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());
It will be mark with success, see line 106
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 =
No change, I simply removed the old code from previous patch and left the
formatted code by accident.
I will remove it from this patch
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();
We get ImageDoesNotExistInDomainError fault only if we didn't managed to create
any task. because if no task created there will be no trigger to end the
command.
If one image is not exist, but we still achieved to create a task for another
image, then it is safe to wait for the end command that will be executed from
the task.
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) {
The fault is only initialized when all images doesn't exist (See
removeAllVmImagesCommand and removeAllVmTemplateImageTemplates)
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.
see previous answer
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
