Ori Liel has posted comments on this change.
Change subject: restapi: disk entity export to glance
......................................................................
Patch Set 3:
(3 comments)
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDiskResource.java
Line 53: ExportRepoImageParameters exportParameters = new
ExportRepoImageParameters(asGuid(disk.getImageId()));
Line 54:
exportParameters.setStorageDomainId(asGuid(disk.getStorageDomains().getStorageDomains().get(0).getId()));
Line 55:
exportParameters.setStoragePoolId(getDataCenterId(exportParameters.getStorageDomainId()));
Line 56: exportParameters.setImageGroupID(asGuid(disk.getId()));
Line 57:
exportParameters.setDestinationDomainId(getStorageDomainId(action));
I want to reiterate Michael's comment - all you need to pass to the engine is a
unique identifier of the disk (its ID) and the details of the destination
storage domain. If the command requires more data from the disk itself, this
data can be fetched already on the engine side. Getting the information from
the Engine, setting it in the parameters and sending it back to the Engine, is
redundant.
Line 58:
Line 59: return doAction(VdcActionType.ExportRepoImage,
exportParameters, action);
Line 60: }
Line 61:
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplateDiskResource.java
Line 48: @Override
Line 49: public Response doExport(Action action) {
Line 50: validateParameters(action, "storageDomain.id|name");
Line 51:
Line 52: Disk disk = get();
Same here - no need for get()
Line 53: ExportRepoImageParameters exportParameters = new
ExportRepoImageParameters(asGuid(disk.getImageId()));
Line 54:
exportParameters.setStorageDomainId(asGuid(disk.getStorageDomains().getStorageDomains().get(0).getId()));
Line 55:
exportParameters.setStoragePoolId(getDataCenterId(exportParameters.getStorageDomainId()));
Line 56: exportParameters.setImageGroupID(asGuid(disk.getId()));
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java
Line 121: @Override
Line 122: public Response doExport(Action action) {
Line 123: validateParameters(action, "storageDomain.id|name");
Line 124:
Line 125: Disk disk = get();
Same comment - no need to get the disk.
Michael had another comment - about not duplicating this logic here and in
BackendDiskResource. If you'll move the logic to the Engine, there will also be
no duplication.
Line 126: ExportRepoImageParameters exportParameters = new
ExportRepoImageParameters(asGuid(disk.getImageId()));
Line 127:
exportParameters.setStorageDomainId(asGuid(disk.getStorageDomains().getStorageDomains().get(0).getId()));
Line 128:
exportParameters.setStoragePoolId(getDataCenterId(exportParameters.getStorageDomainId()));
Line 129: exportParameters.setImageGroupID(asGuid(disk.getId()));
--
To view, visit http://gerrit.ovirt.org/17541
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If351d209d7828f0d59926bb98da9317567976d41
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Ori Liel <ol...@redhat.com>
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
