Maor Lipchuk has posted comments on this change. Change subject: [wip] core: move image group command ......................................................................
Patch Set 1: (4 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveImageGroupCommand.java Line 15: import org.ovirt.engine.core.utils.transaction.TransactionSupport; Line 16: Line 17: @SuppressWarnings("serial") Line 18: @InternalCommandAttribute Line 19: public class MoveImageGroupCommand<T extends MoveOrCopyImageGroupParameters> extends MoveOrCopyImageGroupCommand<T> { Isn't that inheritance is weird that move inherit from copy? Why not simply call copyCommnd and then remove in the end action. Line 20: public MoveImageGroupCommand(T parameters) { Line 21: super(parameters); Line 22: } Line 23: Line 27: removeImageParams.setStorageDomainId(getParameters().getSourceDomainId().getValue()); Line 28: removeImageParams.setEntityId(new ImageStorageDomainMapId(getParameters().getImageId(), getParameters().getSourceDomainId().getValue())); Line 29: removeImageParams.setParentCommand(VdcActionType.RemoveImage); Line 30: removeImageParams.setPerformDbOperationsStage(PerformOperationStage.NEVER); Line 31: VdcReturnValueBase returnValue = Backend.getInstance().runInternalAction( Use getBackend() instead Line 32: VdcActionType.RemoveImage, Line 33: removeImageParams, Line 34: ExecutionHandler.createDefaultContexForTasks(getExecutionContext())); Line 35: if (returnValue.getSucceeded()) { .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyImageGroupCommand.java Line 92 Line 93 Line 94 Line 95 Line 96 I tend to agree with Allon's comment in the next patch, this change is related to the rename patch there for should be squashed. .................................................... File backend/manager/modules/dal/src/main/resources/bundles/AuditLogMessages.properties Line 75: USER_ADD_DISK=Add-Disk operation of '${DiskAlias}' was initiated by ${UserName}. Line 76: USER_ADD_DISK_FINISHED_SUCCESS=The disk '${DiskAlias}' was successfully added. Line 77: USER_ADD_DISK_FINISHED_FAILURE=Operation Add-Disk failed to complete. Line 78: USER_FAILED_ADD_DISK=Operation Add-Disk failed (User: ${UserName}). Line 79: MOVE_IMAGE_GROUP_FAILED_TO_DELETE_SRC_IMAGE=Possible failure when deleting ${DiskAlias} from the source Storage Domain ${StorageDomainName} during the move operation. the Storage Domain may be manually cleaned-up from possible leftovers. Don't you want to add the username here, I think the user name is already in the session. and if you will add it, the user can search the audit log in the events tab Line 80: USER_REMOVE_DISK_FROM_VM=Disk was removed from VM ${VmName} by ${UserName}. Line 81: USER_FAILED_REMOVE_DISK_FROM_VM=Failed to remove Disk from VM ${VmName} (User: ${UserName}). Line 82: USER_UPDATE_VM_DISK=VM ${VmName} ${DiskAlias} disk was updated by ${UserName}. Line 83: USER_FAILED_UPDATE_VM_DISK=Failed to update VM ${VmName} disk ${DiskAlias} (User: ${UserName}). -- To view, visit http://gerrit.ovirt.org/13042 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id9068d66df3986c9bb16b266bb5bef396964a706 Gerrit-PatchSet: 1 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