Allon Mureinik has posted comments on this change. Change subject: engine: Improve of RecoveryStoragePool ......................................................................
Patch Set 2: (3 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RecoveryStoragePoolCommand.java Line 113: getParameters().setStorageDomainId(getStorageDomainId().getValue()); Line 114: try { Line 115: executeReconstruct(); Line 116: } finally { Line 117: if (!reconstructOpSucceeded) { You removed the explicit set of the StoragePoolStatus in case the reconstruct did not succeed. Not necessarily saying this is wrong, but in this case, what sets it? in what status will it be left? Line 118: getStoragePoolIsoMapDAO().remove(domainPoolMap.getId()); Line 119: } Line 120: } Line 121: } else { .................................................... Commit Message Line 4: Commit: Michael Kublin <mkub...@redhat.com> Line 5: CommitDate: 2013-02-10 13:49:29 +0200 Line 6: Line 7: engine: Improve of RecoveryStoragePool Line 8: General: these lines are really long, please adhere to the 72 chars limits and split in to a a couple of lines. Line 9: The following is done: Line 10: 1. Moved connect storage inside code of event call Line 11: 2. Removed useless null pointer check (copy paste code) Line 12: 3. Removed useless update of storage pool status. No reason to update it Line 10: 1. Moved connect storage inside code of event call Line 11: 2. Removed useless null pointer check (copy paste code) Line 12: 3. Removed useless update of storage pool status. No reason to update it Line 13: after finishing operation. Line 14: 4. Revert of the chenges which are done in commit 916d7df48aba8276f8d3fde884f21e19c6f77a42, the reason s/chenges/changes/ Line 15: is simple - commit is makes code unreadable and introducing regresion (send disconnect to previous master domain), Line 16: also I don't understand a motivation for RecoveryStoragePool to extend a ReconstructMasterDomainCommand and Line 17: in excecute create a new instance of such class - these is bad style of coding Line 18: -- To view, visit http://gerrit.ovirt.org/11868 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0781e45b8ae76abe66fffcfad6654d27342a7c45 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Michael Kublin <mkub...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches