Arik Hadas has posted comments on this change. Change subject: core: remove read lock of template being exported ......................................................................
Patch Set 2: (3 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmTemplateCommand.java Line 98: } else { Line 99: endVmTemplateRelatedOps(); Line 100: } Line 101: setSucceeded(true); Line 102: } As I wrote in the previous patch, I don't mind splitting the patches to as much patches as you want, but there is no consistency if you ask to merge this patch with the previous one - patch that only move this method has no meaning by itself either. my logic was to split the previous this patch from the previous one because the previous was a general concept and the follow ups will be implementations that use this concept, are you suggesting other criteria to divide patches? is there a definition of such criteria? Line 103: Line 104: private String getTemplateIsBeingExportedMessage() { Line 105: if (cachedTemplateIsBeingExportedMessage == null) { Line 106: StringBuilder builder = new StringBuilder(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED.name()); .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyTemplateCommand.java Line 156 Line 157 Line 158 Line 159 Line 160 it is related to the previous comment - this method content was moved to ExportVmTemplateCommand class .................................................... File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties Line 492: ACTION_TYPE_FAILED_OBJECT_LOCKED=Cannot ${action} ${type}. Related operation is currently in progress. Please try again later. Line 493: ACTION_TYPE_FAILED_TEMPLATE_IS_USED_FOR_CREATE_VM=Cannot ${action} ${type}. This template is currently in use to create VM ${VmName}. Line 494: ACTION_TYPE_FAILED_DISK_IS_USED_FOR_CREATE_VM=Cannot ${action} ${type}. This disk is currently in use to create VM ${VmName}. Line 495: ACTION_TYPE_FAILED_DISK_IS_BEING_REMOVED=Cannot ${action} ${type}. Disk ${DiskName} is being removed. Line 496: ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_EXPORTED=Cannot ${action} ${type}. Template ${TemplateName} is being exported. it was made before mike's suggestion to merge similar messages. I agree that it would be better, but if we'll agree on the different comments and no other patch-set will be necessary for this patch, I prefer to make this message more generic in the patch that implements the export/import VM without read lock because this one is already verified (and there won't be duplicate entries until then) Line 497: ACTION_TYPE_FAILED_TEMPLATE_IS_BEING_REMOVED=Cannot ${action} ${type}. Template ${TemplateName} is being removed. Line 498: NETWORK_BOND_HAVE_ATTACHED_VLANS=Bond attached to vlan, remove bonds vlan first Line 499: NETWORK_INTERFACE_CONNECT_TO_VLAN=Cannot attach non vlan network to vlan interface Line 500: NETWORK_CANNOT_REMOVE_NETWORK_IN_USE_BY_VM=Cannot remove network '${NetworkName}', it's in use by a VM -- To view, visit http://gerrit.ovirt.org/13109 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I830cfef82063d2dbac3689a46758090da8b10df3 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches