Maor Lipchuk has posted comments on this change. Change subject: core: return created image from scratch ......................................................................
Patch Set 10: (4 comments) Generally, what we usually do is that in the execute phase we get the id from the return value and set it as one of the parameter of the command, so the task infrastructure will be managing this for us. This alternative way to use the end command to return the object is also a way to solve this, though, we need to make sure that the COCO infrastructure will also support this https://gerrit.ovirt.org/#/c/36425/10/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddImageFromScratchCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddImageFromScratchCommand.java: Line 140: Line 141: @Override Line 142: protected void endSuccessfully() { Line 143: super.endSuccessfully(); Line 144: if (getParameters().isReturnCreatedImage()) { I would return the disk no matter if the ReturnCreatedImage is true or not Line 145: setActionReturnValue(queryAddedDisk()); Line 146: } Line 147: } Line 148: Line 141: @Override Line 142: protected void endSuccessfully() { Line 143: super.endSuccessfully(); Line 144: if (getParameters().isReturnCreatedImage()) { Line 145: setActionReturnValue(queryAddedDisk()); Why not using getDestinationDiskImage() instead? Line 146: } Line 147: } Line 148: Line 149: private DiskImage queryAddedDisk() { Line 145: setActionReturnValue(queryAddedDisk()); Line 146: } Line 147: } Line 148: Line 149: private DiskImage queryAddedDisk() { There is no need for calling this query since we already have the destinationDiskImage Line 150: return runInternalQuery( Line 151: VdcQueryType.GetDiskByDiskId, Line 152: new IdQueryParameters(getParameters().getDiskInfo().getId())) Line 153: .getReturnValue(); https://gerrit.ovirt.org/#/c/36425/10/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddImageFromScratchParameters.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddImageFromScratchParameters.java: Line 9: private Guid masterVmId; Line 10: private DiskImageBase diskInfo; Line 11: private boolean shouldRemainIllegalOnFailedExecution; Line 12: /** Whether we should return the created image in end-action or not */ Line 13: private boolean returnCreatedImage; I think we don't need this parameter, we should return the disk all the time Line 14: Line 15: public AddImageFromScratchParameters() { Line 16: masterVmId = Guid.Empty; Line 17: } -- To view, visit https://gerrit.ovirt.org/36425 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6a41d57bfb1bf48f0a2c6a3703e612b27da50db8 Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[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
