Martin Peřina has submitted this change and it was merged. Change subject: core: add 'completed' method to RollbackHandler ...................................................................... core: add 'completed' method to RollbackHandler motivation: there's missing onComplete method allowing to do some actions only when transaction completed. Current usecase: 'client' returns MAC address back to MAC address pool. After doing that, rollback can occur and in that case we need to put that MAC address back to pool. Simple 'onRollback' listener seems to be sufficient for that at first, but we need to realize, that putting MAC address back to pool makes it immediately available for other threads, which means we may encounter problems in on rollback handler if someone used this MAC address meantime. So correct solution here would be, when returning MAC address back to pool, pool adds this MAC address to some collection to be returned, and does so only when related transaction succeeded. Fixes: • CommandBase implements RollbackHandler. This is from OO point of view wrong, since command is not a RollbackHandler. • Previous point also touches another issue — having method 'rollback' directly on Command class, declared as 'non final' means that one can easily make error and suppress logging in CommandBase.rollback method while overriding it to add some custom handling. This should be encapsulated, no one should have possibility to tinker with CommandBase RollbackHandler. Multiple handler can be registered, so every command requiring it's own code in rollback handler, should declare and register his own. In case where order of handlers matter, we can defined composite RollbackHandler, but I do not expect necessity of doing so in near future. Bug-Url: https://bugzilla.redhat.com/1283062 Change-Id: I352be763c84b2c3e640dbc23d15a5a294a6193e1 Signed-off-by: Martin Mucha <mmucha(a)redhat.com&gt; --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddMacPoolCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveMacPoolCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateMacPoolCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/RemoveSnapshotSingleDiskCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/cinder/RemoveCinderSnapshotDiskCommand.java A backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/transaction/NoOpTransactionCompletionListener.java D backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/transaction/RollbackHandler.java A backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/transaction/TransactionCompletionListener.java M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/transaction/TransactionSupport.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VDSCommandBase.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/AddNetworkVDSCommand.java 12 files changed, 185 insertions(+), 119 deletions(-) Approvals: Martin Mucha: Verified Jenkins CI: Passed CI tests Moti Asayag: Looks good to me, approved -- To view, visit https://gerrit.ovirt.org/48817 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I352be763c84b2c3e640dbc23d15a5a294a6193e1 Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmucha(a)redhat.com&gt; Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Mucha <mmucha(a)redhat.com&gt; Gerrit-Reviewer: Martin Peřina <mperina(a)redhat.com&gt; Gerrit-Reviewer: Moti Asayag <masayag(a)redhat.com&gt; Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org&gt;