Tomas Jelinek has posted comments on this change. Change subject: engine: support non-unique vm and template names across DCs ......................................................................
Patch Set 2: (6 comments) https://gerrit.ovirt.org/#/c/41263/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java: Line 101: 'Instance' an instance type Line 104: if (isInstanceWithSameNameExists(getVmTemplateName())) { Line 105: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NAME_ALREADY_USED); Line 106: } Line 107: } else { Line 108: if (isVmTemlateWithSameNameExist(getVmTemplateName(), getVdsGroup().getStoragePoolId())) { - what about blank template? Newly the "Blank" template can be edited and renamed, but it's not connected to any DC (e.g. getVdsGroup() will be null). - what happens if you will try to rename a regular template to "Blank"? Will the test pass? It should not but I suspect it will because on no DC you will have a "Blank" even it will be there. Line 109: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NAME_ALREADY_USED); Line 110: } Line 111: } Line 112: } https://gerrit.ovirt.org/#/c/41263/2/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmTemplateDAO.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmTemplateDAO.java: Line 54: * @param isFiltered Line 55: * Whether the results should be filtered according to the user's permissions Line 56: * @return The Instance which has this name of null if no such exists. Line 57: */ Line 58: public VmTemplate getInstanceByName(String name, Guid userID, boolean isFiltered); please rename to getInstanceTypeByName - the instance is a specific VM based on this instance type. Imagine it is like this: InstanceType is the class and the VM in an object(instance of the class) Line 59: Line 60: /** Line 61: * Retrieves all templates with optional filtering. Line 62: * @param entityType https://gerrit.ovirt.org/#/c/41263/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/TemplateListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/TemplateListModel.java: Line 618: boolean isNameUnique = (Boolean) returnValue; Line 619: templateListModel.postNameUniqueCheck(isNameUnique); Line 620: Line 621: } Line 622: }), name, model.getDataCenterWithClustersList().getSelectedItem().getDataCenter().getId()); - you can simplify this to model.getSelectedDataCenter().getId() - I believe for Blank template the model.getSelectedDataCenter() returns null and you will fail on NPE here. Line 623: } else { Line 624: postNameUniqueCheck(true); Line 625: } Line 626: } https://gerrit.ovirt.org/#/c/41263/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java: Line 653: } Line 654: Line 655: } Line 656: }), Line 657: name, model.getDataCenterWithClustersList().getSelectedItem().getDataCenter().getId()); you can simplify this to model.getSelectedDataCenter().getId() Line 658: } Line 659: } Line 660: Line 661: private void postNameUniqueCheck(UserPortalListModel userPortalListModel) https://gerrit.ovirt.org/#/c/41263/2/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java: Line 1345: } Line 1346: Line 1347: } Line 1348: }), Line 1349: name, model.getDataCenterWithClustersList().getSelectedItem().getDataCenter().getId()); you can simplify this to getSelectedDataCenter().getId() Line 1350: } Line 1351: } Line 1352: Line 1353: private void postNameUniqueCheck() -- 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: 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
