Omer Frenkel has posted comments on this change.

Change subject: engine: support non-unique vm and template names across DCs 
(#912263)
......................................................................


Patch Set 5:

(3 comments)

just copied unanswered comments

https://gerrit.ovirt.org/#/c/41263/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetInstanceTypeQuery.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetInstanceTypeQuery.java:

Line 22:         if (params.getName() != null) {
Line 23:             instance = 
vmTemplateDao.getInstanceTypeByName(params.getName(), getUserID(), 
getParameters().isFiltered());
Line 24:         }
Line 25:         else {
Line 26:             instance = vmTemplateDao.get(getParameters().getId(), 
getUserID(), getParameters().isFiltered());
you should use vmTemplateDao.getInstanceType
Line 27:         }
Line 28:         getQueryReturnValue().setReturnValue(instance);
Line 29:     }
Line 30: 


https://gerrit.ovirt.org/#/c/41263/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmTemplateQuery.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmTemplateQuery.java:

Line 16:         VmTemplate vmt;
Line 17:         GetVmTemplateParameters params = getParameters();
Line 18:         if (params.getName() != null) {
Line 19:             Guid storagePoolId = getStoragePoolId(params); // If no DC 
info available, the query will return the first
Line 20:                                                            // VM with 
the given name found.
first *template*
Line 21:             vmt = DbFacade.getInstance().getVmTemplateDao()
Line 22:                     .getByName(params.getName(), storagePoolId, 
getUserID(), params.isFiltered());
Line 23:         }
Line 24:         else {


Line 32:         getQueryReturnValue().setReturnValue(vmt);
Line 33:     }
Line 34: 
Line 35:     // Get the datacenter ID.
Line 36:     private Guid getStoragePoolId(GetVmTemplateParameters params) {
no need to pass params as parameter you can use getParameters() here as well
Line 37:         Guid result = null;
Line 38:         if (params.getDataCenterId() != null) {
Line 39:             result = params.getDataCenterId();
Line 40:         } else if (params.getClusterId() != null) {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5f3244ec1885d54e58b475d0e74f59e26fa492a0
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ori Liel <ol...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Ori Liel <ol...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to