Omer Frenkel has posted comments on this change. Change subject: core : Adding functionality for moving and coping disks of vm/templates ......................................................................
Patch Set 1: (4 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java Line 48: retValue = false; why not return false here (like done in the first check)? and loose the retValue.. Line 74: if (retValue && getParameters().getOperation() == ImageOperation.Move there is a double check for retValue here (i still prefer not using it :) ) also why this is checked only for move? is there any logic to copy disk to the same domain? do we support this? Line 80: retValue = retValue could have been false, and it is overridden by the result of the validator... .................................................... File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java Line 116: MoveOrCopyDisk(226, ActionGroup.CONFIGURE_VM_STORAGE), the problem with doing only one command for both actions (copy and move) is the permissions, now users would not be able to copy template disks. we need to think if to separate for 2 actions, or create new action group and make it good for both template and vm -- To view, visit http://gerrit.ovirt.org/2555 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ief57abf659e012d73226f8d68211d3832fe2f036 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Michael Kublin <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Michael Kublin <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
