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