Piotr Kliczewski has posted comments on this change.

Change subject: engine : Remove Disk command using getVolumeInfo
......................................................................


Patch Set 3:

(2 comments)

https://gerrit.ovirt.org/#/c/39374/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java:

Line 77:             try {
Line 78: //                Guid taskId = 
persistAsyncTaskPlaceHolder(getParameters().getParentCommand());
Line 79: 
Line 80:                 VDSReturnValue vdsReturnValue = 
performImageVdsmOperation();
Line 81: //                getReturnValue().getInternalVdsmTaskIdList().add(
Why comment and not remove?
Line 82: //                        createTask(taskId,
Line 83: //                                vdsReturnValue.getCreationInfo(),
Line 84: //                                getParameters().getParentCommand(),
Line 85: //                                VdcObjectType.Storage,


https://gerrit.ovirt.org/#/c/39374/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommandCallback.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommandCallback.java:

Line 26:             Guid storagePoolId = diskImage.getStoragePoolId() != null 
? diskImage.getStoragePoolId() : Guid.Empty;
Line 27:             Guid imageGroupId = diskImage.getId() != null ? 
diskImage.getId() : Guid.Empty;
Line 28: 
Line 29:             if (irsProxyData == null) {
Line 30:                 irsProxyData = new IrsProxyData(storagePoolId);
When we create new IRsProxyData and then we call getIrsProxy we create new 
connection to vdsm. I am not sure whether we want to do it.

There is IrsBrokerCommand class which has static method to obtain irsProxy for 
storagePoolId.

Additonal comment: Engine practice is not run vdsbroker code directly from bll. 
We use RunVdsCommand from ResourceManager.
Line 31:             }
Line 32: 
Line 33:             imageInfoReturn = 
irsProxyData.getIrsProxy().getVolumeInfo(diskImage.getStorageIds().get(0).toString(),
Line 34:                     storagePoolId.toString(), imageGroupId.toString(),


-- 
To view, visit https://gerrit.ovirt.org/39374
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I458143387f89a5d55bda5f1d4f9d44be4f7fb197
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[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