Maor Lipchuk has posted comments on this change. Change subject: wip: core: RemoveImage as sync operation ......................................................................
Patch Set 5: (5 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveDiskCommand.java Line 266: p.setRemoveFromSnapshots(true); Line 267: p.setStorageDomainId(getParameters().getStorageDomainId()); Line 268: p.setForceDelete(getParameters().getForceDelete()); Line 269: if (diskImage.getStorageIds().size() > 1) { Line 270: p.setRemoveFromDB(false); Isn't remove from DB is false by default, why we need this check here? Line 271: } Line 272: VdcReturnValueBase vdcReturnValue = Line 273: Backend.getInstance().runInternalAction(VdcActionType.RemoveImage, Line 274: p, .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java Line 88: } catch (VdcBLLException e) { Line 89: if (e.getErrorCode() != VdcBllErrors.ImageDoesNotExistInDomainError) { Line 90: throw e; Line 91: } Line 92: log.debugFormat("The image group with id {0} wasn't actually deleted from the storage domain {1} because it didn't exist in it", I would do it as warn, but no obligation Line 93: getDiskImage().getId(), Line 94: getStorageDomainId()); Line 95: } Line 96: .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmCommand.java Line 230: public AuditLogType getAuditLogTypeValue() { Line 231: switch (getActionState()) { Line 232: case EXECUTE: Line 233: return getSucceeded() ? AuditLogType.USER_REMOVE_VM_FINISHED : (hasImages ? AuditLogType. Line 234: USER_REMOVE_VM_FINISHED_WITH_ILLEGAL_DISKS : AuditLogType.USER_FAILED_REMOVE_VM); If I'm not mistaking, VM with illegal disks will be printed twice in the audit log, I think it will be better to remove it from the execute and only print it now in the END phase. Line 235: case END_FAILURE: Line 236: case END_SUCCESS: Line 237: default: Line 238: return disksLeftInVm.isEmpty() ? AuditLogType.UNASSIGNED .................................................... Commit Message Line 5: CommitDate: 2013-04-11 09:52:38 +0300 Line 6: Line 7: wip: core: RemoveImage as sync operation Line 8: Line 9: When performing RemoveImage command, the engine can treat the to the vds ...treat the to the...? Line 10: command DeleteImageGroup as synchronous call as afterwards it the image Line 11: is considered to be non exist on the storage side - when receiving an Line 12: ImageDoesNotExist error from vdsm - proceed with removing the image. Line 13: Line 6: Line 7: wip: core: RemoveImage as sync operation Line 8: Line 9: When performing RemoveImage command, the engine can treat the to the vds Line 10: command DeleteImageGroup as synchronous call as afterwards it the image ...afterwards it the image is considered...? Line 11: is considered to be non exist on the storage side - when receiving an Line 12: ImageDoesNotExist error from vdsm - proceed with removing the image. Line 13: Line 14: Change-Id: Ic822a92acb8231d2ca84ae092bd98659507925d8 -- To view, visit http://gerrit.ovirt.org/13611 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic822a92acb8231d2ca84ae092bd98659507925d8 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches