Daniel Erez has posted comments on this change.

Change subject: core: GetCinderVolumeTypesByStorageDomainIdQuery
......................................................................


Patch Set 3:

(3 comments)

https://gerrit.ovirt.org/#/c/39027/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/storage/OpenStackVolumeProviderProxy.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/storage/OpenStackVolumeProviderProxy.java:

Line 83:         for (VolumeType volumeType : volumeTypes) {
Line 84:             CinderVolumeType cinderVolumeType = new CinderVolumeType();
Line 85:             cinderVolumeType.setId(volumeType.getId());
Line 86:             cinderVolumeType.setName(volumeType.getName());
Line 87:             cinderVolumeType.setExtraSpecs(volumeType.getExtraSpecs());
> Would be nice if we will have a ctor for all of those properties
Done
Line 88:             cinderVolumeTypes.add(cinderVolumeType);
Line 89:         }
Line 90:         return cinderVolumeTypes;
Line 91:     }


Line 109: 
Line 110:     public static OpenStackVolumeProviderProxy 
getFromStorageDomainId(Guid storageDomainId, Guid userID, boolean isFiltered) {
Line 111:         StorageDomain storageDomain = 
getDbFacade().getStorageDomainDao().get(storageDomainId, userID, isFiltered);
Line 112:         if (storageDomain != null) {
Line 113:             Provider provider = 
getDbFacade().getProviderDao().get(new Guid(storageDomain.getStorage()));
> why not calling  getFromStorageDomainStatic
since only 'getStorageDomainDao().get' has permissions check (i.e. get userID, 
isFiltered..)
Line 114:             return 
ProviderProxyFactory.getInstance().create(provider);
Line 115:         }
Line 116:         return null;
Line 117:     }


Line 115:         }
Line 116:         return null;
Line 117:     }
Line 118: 
Line 119:     private static OpenStackVolumeProviderProxy 
getFromStorageDomainStatic(StorageDomainStatic storageDomainStatic) {
> s/getFrom.../getProviderFrom..
Done
Line 120:         Provider provider = getDbFacade().getProviderDao().get(new 
Guid(storageDomainStatic.getStorage()));
Line 121:         return ProviderProxyFactory.getInstance().create(provider);
Line 122:     }
Line 123: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie964dd3e363358abf20f3753fb46082070ab8e07
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
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

Reply via email to