Liron Aravot has posted comments on this change.

Change subject: core: add locks to VmTemplate commands (#761515)
......................................................................


Patch Set 6: (10 inline comments)

replied mkublin and amureini comments

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
Line 289:         if (getVmTemplate() == null) {
Line 290:             
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST);
Line 291:             return false;
Line 292:         }
Line 293: 
can't find where you pointed on a method that performs this check, can you 
elaborate?
Line 294:         if (!verifyVmTemplateStatusForUse(getVmTemplateId())) {
Line 295:             getReturnValue().getCanDoActionMessages()
Line 296:                     
.add(VdcBllMessages.VM_TEMPLATE_IMAGE_IS_LOCKED.toString());
Line 297:         }


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java
Line 35:         return true;
Line 36:     }
Line 37: 
Line 38:     @Override
Line 39:     protected Map<String, String> getExclusiveLocks() {
it can't be shared at the moment, as we have to lock the vm in the DB in order 
to prevent deletion of it while cloning vm from it for example (see discussion 
in bug for futher info), right now an exlusive lock is required as we are 
missing shared DB lock.
Line 40:         Map<String, String> locks = new HashMap<String, String>();
Line 41:         Map<String, String> parentLocks = super.getExclusiveLocks();
Line 42:         if (parentLocks != null) {
Line 43:             locks.putAll(parentLocks);


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ExportVmTemplateCommand.java
Line 84:     protected boolean canDoAction() {
Line 85:         if (getVmTemplate() != null) {
Line 86:             setDescription(getVmTemplateName());
Line 87:         }
Line 88: 
can you please elaborate? don't see that it's already done in this flow.
Line 89:         if 
(!VmTemplateHandler.isTemplateStatusIsNotLocked(getVmTemplateId())) {
Line 90:             getReturnValue().getCanDoActionMessages()
Line 91:             
.add(VdcBllMessages.VM_TEMPLATE_IMAGE_IS_LOCKED.toString());
Line 92:         }


Line 87:         }
Line 88: 
Line 89:         if 
(!VmTemplateHandler.isTemplateStatusIsNotLocked(getVmTemplateId())) {
Line 90:             getReturnValue().getCanDoActionMessages()
Line 91:             
.add(VdcBllMessages.VM_TEMPLATE_IMAGE_IS_LOCKED.toString());
Done
Line 92:         }
Line 93: 
Line 94:         StorageDomainValidator storageDomainValidator = new 
StorageDomainValidator(getStorageDomain());
Line 95:         boolean retVal = 
storageDomainValidator.isDomainExistAndActive(getReturnValue().getCanDoActionMessages());


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmTemplateCommand.java
Line 90:             retVal = false;
Line 91:         } else {
Line 92:             setDescription(getVmTemplateName());
Line 93:         }
Line 94: 
as we don't have at the moment any efficient way to handle the export domain 
vmtemplate commands, the handle in those commands was removed from the newer 
patch version until we will have a way to solve it.
Line 95:         if (retVal && 
!VmTemplateHandler.isTemplateStatusIsNotLocked(getVmTemplateId())) {
Line 96:             getReturnValue().getCanDoActionMessages()
Line 97:                     
.add(VdcBllMessages.VM_TEMPLATE_IMAGE_IS_LOCKED.toString());
Line 98:         }


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateCommand.java
Line 69:         if (VmTemplateHandler.BlankVmTemplateId.equals(vmTemplateId)) {
Line 70:             
addCanDoActionMessage(VdcBllMessages.VMT_CANNOT_REMOVE_BLANK_TEMPLATE);
Line 71:             return false;
Line 72:         }
Line 73: 
where?
Line 74:         if 
(!VmTemplateHandler.isTemplateStatusIsNotLocked(getVmTemplateId())) {
Line 75:             getReturnValue().getCanDoActionMessages()
Line 76:                     
.add(VdcBllMessages.VM_TEMPLATE_IMAGE_IS_LOCKED.toString());
Line 77:         }


Line 185:     @Override
Line 186:     protected void executeCommand() {
Line 187:         // Set VM to lock status immediately, for reducing race 
condition.
Line 188:         
VmTemplateHandler.lockVmTemplateInTransaction(getVmTemplateId(), 
getCompensationContext());
Line 189:         // if for some reason template doesn't have images, remove it 
now and not in end action
Done
Line 190:         final boolean hasImages = imageTemplates.size() > 0;
Line 191:         if 
(RemoveTemplateInSpm(getVmTemplate().getstorage_pool_id().getValue(), 
getVmTemplateId())) {
Line 192:             if (hasImages) {
Line 193:                 TransactionSupport.executeInNewTransaction(new 
TransactionMethod<Void>() {


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveVmTemplateFromImportExportCommand.java
Line 50:         if (getVmTemplate() == null) {
Line 51:             retVal = false;
Line 52:             
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_DOES_NOT_EXIST);
Line 53:         }
Line 54: 
handling of this file was removed from this patch.
Line 55:         if (retVal && 
!VmTemplateHandler.isTemplateStatusIsNotLocked(getVmTemplateId())) {
Line 56:             getReturnValue().getCanDoActionMessages()
Line 57:                     
.add(VdcBllMessages.VM_TEMPLATE_IMAGE_IS_LOCKED.toString());
Line 58:             retVal = false;


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateCommand.java
Line 177:     protected String getDescription() {
Line 178:         return getVmTemplateName();
Line 179:     }
Line 180: 
Line 181: 
Done
Line 182:     /**
Line 183:      * This method create OVF for each template in list and call 
updateVm in SPM
Line 184:      *
Line 185:      * @param storagePoolId


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmTemplateHandler.java
Line 58:      *
Line 59:      * @return True if template is at valid state, false otherwise.
Line 60:      */
Line 61:     public static boolean isTemplateStatusIsNotLocked(Guid id) {
Line 62:         boolean toReturn = 
VmTemplateHandler.BlankVmTemplateId.equals(id);
Done
Line 63:         if (!toReturn) {
Line 64:             VmTemplate template = 
DbFacade.getInstance().getVmTemplateDAO().get(id);
Line 65:             if (!((template != null) && (template.getstatus() != 
VmTemplateStatus.Locked))) {
Line 66:                 toReturn = true;


--
To view, visit http://gerrit.ovirt.org/8013
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2d5b6518bbf19077ae5822b9359a6147c87d0f46
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to