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

Reply via email to