Liron Ar 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> {
move is a copy, which also perform delete in the end - it seems fine to me.

>> Why not simply call copyCommnd and then remove in the end action 
It seems to add here extra overhead, if we want it can always be done - 
currently it just means that we need to take care of extra things like the 
tasks creation (as now we will have command the executes child command that 
executes another child command), need to explicitly execute copy and 
such..currently, I don't see any reason to perform all the above.
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(
Done
Line 32:                 VdcActionType.RemoveImage,
Line 33:                 removeImageParams,
Line 34:                 
ExecutionHandler.createDefaultContexForTasks(getExecutionContext()));
Line 35:         if (returnValue.getSucceeded()) {


....................................................
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.
Good suggestions...thanks - Done
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}).


....................................................
Commit Message
Line 21: * The whole operations can be slower for moving image groups without 
wipe
Line 22: as the delete part is quick (and now we will have two vdsm calls 
rather then one) -
Line 23: this issue is known and accepted as part of this RFE.
Line 24: 
Line 25: * The code hasn't been optimized for moving few 'related' disks at once
Vered, thanks - DOne :)
Line 26: as MoveVm/MoveVmTemplate are deprecated.
Line 27: 
Line 28: Change-Id: Id9068d66df3986c9bb16b266bb5bef396964a706
Line 29: Bug-Url: https://bugzilla.redhat.com/753549


--
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

Reply via email to