Eli Mesika has posted comments on this change.

Change subject: engine: support non-unique vm and template names across DCs
......................................................................


Patch Set 2:

(13 comments)

https://gerrit.ovirt.org/#/c/41263/2/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 13:     protected void executeQueryCommand() {
Line 14:         InstanceType instance;
Line 15:         GetVmTemplateParameters params = getParameters();
Line 16:         if (params.getName() != null) {
Line 17:             instance = DbFacade.getInstance().getVmTemplateDao()
You should use injection of the DAO here 

(look in the code for @Inject, you have lot of samples)
Line 18:                     .getInstanceByName(params.getName(), getUserID(), 
getParameters().isFiltered());
Line 19:         }
Line 20:         else {
Line 21:             instance = DbFacade.getInstance().getVmTemplateDao()


Line 17:             instance = DbFacade.getInstance().getVmTemplateDao()
Line 18:                     .getInstanceByName(params.getName(), getUserID(), 
getParameters().isFiltered());
Line 19:         }
Line 20:         else {
Line 21:             instance = DbFacade.getInstance().getVmTemplateDao()
same
Line 22:                     .get(getParameters().getId(), getUserID(), 
getParameters().isFiltered());
Line 23:         }
Line 24:         getQueryReturnValue().setReturnValue(instance);
Line 25:     }


https://gerrit.ovirt.org/#/c/41263/2/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 15:         VmTemplate vmt;
Line 16:         GetVmTemplateParameters params = getParameters();
Line 17:         if (params.getName() != null) {
Line 18:             Guid storagePoolId = getStoragePoolId(params);
Line 19:             vmt = DbFacade.getInstance().getVmTemplateDao()
worth using here CDI @Inject (in a separate patch)
Line 20:                     .getByName(params.getName(), storagePoolId, 
getUserID(), params.isFiltered());
Line 21:         }
Line 22:         else {
Line 23:             vmt = DbFacade.getInstance().getVmTemplateDao()


Line 19:             vmt = DbFacade.getInstance().getVmTemplateDao()
Line 20:                     .getByName(params.getName(), storagePoolId, 
getUserID(), params.isFiltered());
Line 21:         }
Line 22:         else {
Line 23:             vmt = DbFacade.getInstance().getVmTemplateDao()
worth using here CDI @Inject (in a separate patch)
Line 24:                     .get(getParameters().getId(), getUserID(), 
getParameters().isFiltered());
Line 25:         }
Line 26:         if (vmt != null) {
Line 27:             VmTemplateHandler.updateDisksFromDb(vmt);


Line 32: 
Line 33:     // Get the DC ID. If no DC info available, the query will return 
the first VM
Line 34:     // with the given name found.
Line 35:     private Guid getStoragePoolId(GetVmTemplateParameters params) {
Line 36:         return params.getStoragePoolId() != null ? 
params.getStoragePoolId() : params.getVdsGroupId() != null ?  
DbFacade.getInstance().getVdsGroupDao().get(params.getVdsGroupId()).getStoragePoolId()
 : null;
please format this line , its too long ...
Line 37:     }


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

Line 236:         return true;
Line 237:     }
Line 238: 
Line 239:     protected boolean isVmTemplateWithSameNameExist() {
Line 240:         return DbFacade.getInstance()
please inject the DAO
Line 241:                 .getVmTemplateDao()
Line 242:                 .getByName(getParameters().getVmTemplate().getName(), 
getParameters().getStoragePoolId(), null, false) != null;
Line 243:     }
Line 244: 


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

Line 17:                         getParameters().getDatacenterId()));
Line 18:     }
Line 19: 
Line 20:     public boolean isVmTemlateWithSameNameExist(String name, Guid 
datacenterId) {
Line 21:         return 
DbFacade.getInstance().getVmTemplateDao().getByName(name, datacenterId, null, 
false) != null;
please inject the DAO
Line 22:     }


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

Line 62:         throw new NotImplementedException();
Line 63:     }
Line 64: 
Line 65:     public boolean isVmTemlateWithSameNameExist(String name, Guid 
datacenterId) {
Line 66:         return 
DbFacade.getInstance().getVmTemplateDao().getByName(name, datacenterId, null, 
false) != null;
please inject the DAO
Line 67:     }
Line 68: 
Line 69:     public boolean isInstanceWithSameNameExists(String name) {
Line 70:         return 
DbFacade.getInstance().getVmTemplateDao().getInstanceByName(name, null, false) 
!= null;


Line 66:         return 
DbFacade.getInstance().getVmTemplateDao().getByName(name, datacenterId, null, 
false) != null;
Line 67:     }
Line 68: 
Line 69:     public boolean isInstanceWithSameNameExists(String name) {
Line 70:         return 
DbFacade.getInstance().getVmTemplateDao().getInstanceByName(name, null, false) 
!= null;
please inject the DAO
Line 71:     }
Line 72: 
Line 73:     public static boolean isVmTemplateImagesReady(VmTemplate 
vmTemplate,
Line 74:             Guid storageDomainId,


https://gerrit.ovirt.org/#/c/41263/2/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmTemplateParameters.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/GetVmTemplateParameters.java:

Line 6:     private static final long serialVersionUID = 8906662143775124331L;
Line 7: 
Line 8:     private Guid _id;
Line 9:     private String _name;
Line 10:     private Guid vdsGroupId;
vdsGroupId => clusterId
Line 11:     private Guid storagePoolId;
Line 12: 
Line 13:     public GetVmTemplateParameters(Guid id) {
Line 14:         _id = id;


Line 37:     public void setStoragePoolId(Guid storagePoolId) {
Line 38:         this.storagePoolId = storagePoolId;
Line 39:     }
Line 40: 
Line 41:     public Guid getVdsGroupId() {
I would use the new term : Cluster in new code ....
Line 42:         return vdsGroupId;
Line 43:     }
Line 44: 
Line 45:     public void setVdsGroupId(Guid vdsGroupId) {


Line 42:         return vdsGroupId;
Line 43:     }
Line 44: 
Line 45:     public void setVdsGroupId(Guid vdsGroupId) {
Line 46:         this.vdsGroupId = vdsGroupId;
same
Line 47:     }
Line 48: 


https://gerrit.ovirt.org/#/c/41263/2/packaging/dbscripts/vm_templates_sp.sql
File packaging/dbscripts/vm_templates_sp.sql:

Line 463: BEGIN
Line 464:       RETURN QUERY SELECT vm_templates.*
Line 465:       FROM vm_templates_view vm_templates
Line 466:       WHERE name = v_vmt_name
Line 467:       AND   (v_storage_pool_id is null OR storage_pool_id = 
v_storage_pool_id) 
please remove TWS
Line 468:       AND (NOT v_is_filtered OR EXISTS (SELECT 1
Line 469:                                         FROM   
user_vm_template_permissions_view
Line 470:                                         WHERE  user_id = v_user_id 
AND entity_id = vmt_guid));
Line 471: END; $procedure$


-- 
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: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ori Liel <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to