Omer Frenkel has posted comments on this change. Change subject: engine: support non-unique vm and template names across DCs ......................................................................
Patch Set 2: (5 comments) https://gerrit.ovirt.org/#/c/41263/2//COMMIT_MSG Commit Message: Line 5: CommitDate: 2015-05-21 17:19:22 +0300 Line 6: Line 7: engine: support non-unique vm and template names across DCs Line 8: Line 9: Change-Id: I5f3244ec1885d54e58b475d0e74f59e26fa492a0 please add bug number as well 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 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; also, DbFacade.getInstance().getVdsGroupDao().get(params.getVdsGroupId()) can return null 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 why not keep using that method and just add a parameter (dcId) to it? 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/UpdateVmTemplateCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmTemplateCommand.java: 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 r since blank has no dc/cluster any more, i think its ok to have another "blank" template in your dc if you like. just like we will now allow having multiple "templateX" - one for each dc.. also you can create template named "big" although there is instance type with this name.. i agree we need to make sure updating blank template doesnt fail because getVdsGroup() == null (or any other template that for some reason doenst have cluster/dc) 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/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 61: protected void executeCommand() { Line 62: throw new NotImplementedException(); Line 63: } Line 64: Line 65: public boolean isVmTemlateWithSameNameExist(String name, Guid datacenterId) { i would leave it static and use it from other places as before Line 66: return DbFacade.getInstance().getVmTemplateDao().getByName(name, datacenterId, null, false) != null; Line 67: } Line 68: Line 69: public boolean isInstanceWithSameNameExists(String name) { -- 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
