Arik Hadas has posted comments on this change.

Change subject: core: return created image from scratch
......................................................................


Patch Set 10:

(4 comments)

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
Done
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?
Done
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 destinati
Done
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 tim
Done
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

Reply via email to