Michael Pasternak has posted comments on this change.

Change subject: restapi: disk entity export to glance
......................................................................


Patch Set 1: Code-Review-1

(8 comments)

....................................................
Commit Message
Line 3: AuthorDate: 2013-07-26 14:00:38 +0200
Line 4: Commit:     Federico Simoncelli <[email protected]>
Line 5: CommitDate: 2013-07-31 17:52:23 +0200
Line 6: 
Line 7: restapi: disk entity export to glance
* please add wiki ref / BZ

* please describe this feature in the FeatureHelper

* please add tests
Line 8: 
Line 9: Change-Id: If351d209d7828f0d59926bb98da9317567976d41


....................................................
File 
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
Line 873:     urlparams: {}
Line 874:     headers:
Line 875:       Content-Type: {value: application/xml|json, required: true}
Line 876:       Correlation-Id: {value: 'any string', required: false}
Line 877:       Filter: {value: true|false, required: false}
having the export impl in the BackendDiskResource will result export() to 
appear in the BackendStorageDomainDiskResource as well
Line 878: - name: /api/capabilities|rel=get
Line 879:   request:
Line 880:     body:
Line 881:       parameterType: null


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendDiskResource.java
Line 54:                     DiskHelper.getExportRepoImageParameters(get(), 
getStorageDomainId(action));
Line 55:             return doAction(VdcActionType.ExportRepoImage, 
exportParameters, action);
Line 56:         } catch (Exception e) {
Line 57:             return handleError(e, action);
Line 58:         }
no need for the failover treatment, this is done by the infra, just return 
doAction()
Line 59:     }
Line 60: 
Line 61:     @Override
Line 62:     public ActionResource getActionSubresource(@PathParam("action") 
String action, @PathParam("oid") String oid) {


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java
Line 127:                     
DiskHelper.getExportRepoImageParameters(getDisk(), getStorageDomainId(action));
Line 128:             return doAction(VdcActionType.ExportRepoImage, 
exportParameters, action);
Line 129:         } catch (Exception e) {
Line 130:             return handleError(e, action);
Line 131:         }
no need for the failover treatment, this is done by the infra, just return 
doAction()
Line 132:     }
Line 133: 
Line 134:     @Override
Line 135:     public Disk update(Disk resource) {


....................................................
File 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/util/DiskHelper.java
Line 4: import org.ovirt.engine.api.model.StorageDomain;
Line 5: import org.ovirt.engine.core.common.action.ExportRepoImageParameters;
Line 6: import org.ovirt.engine.core.compat.Guid;
Line 7: 
Line 8: public class DiskHelper {
this is mapper logic, please move all content to the DiskMapper
Line 9:     public static ExportRepoImageParameters 
getExportRepoImageParameters(Disk disk, Guid destinationStorageDomainId) {
Line 10:         if (disk.isSetLunStorage()) {
Line 11:             throw new UnsupportedOperationException("Direct LUN disks 
are unsupported");
Line 12:         }


Line 7: 
Line 8: public class DiskHelper {
Line 9:     public static ExportRepoImageParameters 
getExportRepoImageParameters(Disk disk, Guid destinationStorageDomainId) {
Line 10:         if (disk.isSetLunStorage()) {
Line 11:             throw new UnsupportedOperationException("Direct LUN disks 
are unsupported");
this should be in the BE command can-do-action
Line 12:         }
Line 13: 
Line 14:         if (disk.getStorageDomains() == null || 
disk.getStorageDomains().getStorageDomains().size() < 1) {
Line 15:             throw new IllegalStateException("Cannot find disk storage 
domain");


Line 11:             throw new UnsupportedOperationException("Direct LUN disks 
are unsupported");
Line 12:         }
Line 13: 
Line 14:         if (disk.getStorageDomains() == null || 
disk.getStorageDomains().getStorageDomains().size() < 1) {
Line 15:             throw new IllegalStateException("Cannot find disk storage 
domain");
this should be in the BE command can-do-action
Line 16:         }
Line 17: 
Line 18:         ExportRepoImageParameters exportParameters = new 
ExportRepoImageParameters(new Guid(disk.getImageId()));
Line 19:         StorageDomain storageDomain = 
disk.getStorageDomains().getStorageDomains().get(0);


....................................................
File 
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/DiskMapper.java
Line 180:             }
Line 181:             for (Guid id : entity.getStorageIds()){
Line 182:                 StorageDomain storageDomain = new StorageDomain();
Line 183:                 storageDomain.setId(id.toString());
Line 184:                 storageDomain.setDataCenter(dataCenter);
this is backlink

<disk>
  <storage_domains>  
    <storage_domain id=xxx href=.../xxx>
  ...

no other fields except of id should appear in SD, also why do you need this?
Line 185:                 
model.getStorageDomains().getStorageDomains().add(storageDomain);
Line 186:             }
Line 187:         }
Line 188:         if (entity.getQuotaId()!=null) {


-- 
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: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[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