Greg Padgett has posted comments on this change.

Change subject: core: introduce RemoveSnapshotSingleDiskLive BLL command
......................................................................


Patch Set 15:

(4 comments)

http://gerrit.ovirt.org/#/c/26909/15/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommand.java:

Line 43:                             VdcObjectType.Storage, 
getParameters().getStorageDomainId());
Line 44:             getReturnValue().getInternalVdsmTaskIdList().add(result);
Line 45:             getParameters().getVdsmTaskIds().add(result);
Line 46:             setSucceeded(vdsReturnValue.getSucceeded());
Line 47:             persistCommand(getParameters().getParentCommand(), false);
> the above should be
Done
Line 48:         } else {
Line 49:             setSucceeded(false);
Line 50:             setCommandStatus(CommandStatus.FAILED);
Line 51:         }


Line 76: 
Line 77:     @Override
Line 78:     public CommandCallBack getCallBack() {
Line 79:         //return new DestroyImageCommandCallback();
Line 80:         return null;
> This methods can be omitted if none of the parents override this method
Done
Line 81:     }


http://gerrit.ovirt.org/#/c/26909/15/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommandCallback.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/DestroyImageCommandCallback.java:

Line 12: import org.ovirt.engine.core.utils.log.LogFactory;
Line 13: 
Line 14: // TODO This class is probably not needed, and will be removed if that 
is indeed the case.
Line 15: // TODO For now, leaving it here in case it's helpful for debugging 
DestroyImageCommand.
Line 16: public class DestroyImageCommandCallback extends CommandCallBack {
> This class can be removed
Done
Line 17:     private static final Log log = 
LogFactory.getLog(DestroyImageCommand.class);
Line 18: 
Line 19:     @Override
Line 20:     public void doPolling(Guid cmdId, List<Guid> childCmdIds) {


http://gerrit.ovirt.org/#/c/26909/15/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 66:             case SUCCEEDED:
Line 67:                 vdcReturnValue = 
TaskManagerUtil.getCommandReturnValue(childCommandIds.get(0));
Line 68:                 if (vdcReturnValue != null && 
vdcReturnValue.getSucceeded()) {
Line 69:                     
getParameters().setCommandStep(getParameters().getNextCommandStep());
Line 70:                     
TaskManagerUtil.removeCommand(childCommandIds.get(0));
> If you remove child here, on restart childCOmmandIds will be empty and vdcR
Nice catch, done
Line 71:                     break;
Line 72:                 } else {
Line 73:                     log.errorFormat("Failed to merge, child command 
{0} failed: {1}",
Line 74:                             getParameters().getCommandStep(),


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic47eb91a0ea1fe150e3b2152e2c9d5f1f2eb3678
Gerrit-PatchSet: 15
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadg...@redhat.com>
Gerrit-Reviewer: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to