Allon Mureinik has posted comments on this change. Change subject: core: WIP:AddStoragePoolWithStorages-prevent domains from stay LOCKED ......................................................................
Patch Set 5: I would prefer that you didn't submit this (4 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/AddStoragePoolWithStoragesCommand.java Line 119: } Line 120: Line 121: for (Guid storageDomainId : getParameters().getStorages()) { Line 122: DbFacade.getInstance() Line 123: .getStoragePoolIsoMapDao() We should have a getStoragePoolIsoMapDao() method. Line 124: .updateStatus( Line 125: new StoragePoolIsoMapId(storageDomainId, getStoragePool().getId()), Line 126: StorageDomainStatus.InActive); Line 127: } Line 123: .getStoragePoolIsoMapDao() Line 124: .updateStatus( Line 125: new StoragePoolIsoMapId(storageDomainId, getStoragePool().getId()), Line 126: StorageDomainStatus.InActive); Line 127: } shouldn't this for loop should be /inside/ the transaction? Line 128: // Create pool phase completed, no rollback is needed here, so compensation information needs to be cleared! Line 129: TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { Line 130: @Override Line 131: public Void runInTransaction() { Line 232: try { Line 233: Backend.getInstance() Line 234: .runInternalAction(VdcActionType.ActivateStorageDomain, activateParameters).getSucceeded(); Line 235: } catch (RuntimeException e) { Line 236: log.error(e); perhaps an audit log too? or add the exception details to the return value? Line 237: } Line 238: } Line 239: } Line 240: .................................................... Commit Message Line 5: CommitDate: 2012-10-15 10:31:58 +0200 Line 6: Line 7: core: WIP:AddStoragePoolWithStorages-prevent domains from stay LOCKED Line 8: Line 9: AddStoragePoolWithStoragesCommand execute ActivateStorageDomain command s/execute/executes/ Line 10: for each domain in the pool, failure in the domain activation causes the domain to Line 11: stay in status locked forver. Line 12: Line 13: Change-Id: Ie8b9fc0d324eac2146d4f7d60d48e4dd5eb5d9e0 -- To view, visit http://gerrit.ovirt.org/8536 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie8b9fc0d324eac2146d4f7d60d48e4dd5eb5d9e0 Gerrit-PatchSet: 5 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: Vered Volansky <vvola...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches