Yair Zaslavsky has posted comments on this change.

Change subject: engine : Live Merge fails to find COTR for 
RemoveSnapshotSingleDiskLiveCommand
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

http://gerrit.ovirt.org/#/c/29477/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java:

Line 170:             } else {
Line 171:                 TaskManagerUtil.executeAsyncCommand(
Line 172:                         getSnapshotActionType(),
Line 173:                         
buildRemoveSnapshotSingleDiskParameters(source, dest),
Line 174:                         getContext());
Please duplicate the context object.
In addition, please notice that  by passing the context or by duplicating it, 
you will pass lock, execution and compensation context from parent to child.
Is that really what you want? based on the original, I don't think so.
You have a cloneFromParentContextAndDetach in order to prevent that.
Line 175:             }
Line 176: 
Line 177:             List<Guid> quotasToRemoveFromCache = new 
ArrayList<Guid>();
Line 178:             quotasToRemoveFromCache.add(source.getQuotaId());


http://gerrit.ovirt.org/#/c/29477/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotSingleDiskLiveCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotSingleDiskLiveCommand.java:

Line 137:         }
Line 138: 
Line 139:         persistCommand(getParameters().getParentCommand(), true);
Line 140:         if (nextCommand != null) {
Line 141:             
TaskManagerUtil.executeAsyncCommand(nextCommand.getFirst(), 
nextCommand.getSecond(), getContext());
see previous comment.
Line 142:         }
Line 143:     }
Line 144: 
Line 145:     private MergeParameters buildMergeParameters() {


-- 
To view, visit http://gerrit.ovirt.org/29477
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id914da3ed1a29e651ad2491fc88a2705b7af6e8f
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Greg Padgett <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to