Liron Aravot has posted comments on this change.
Change subject: core : WIP : added ReconstructMasterDomain to
InitVdsOnUpCommand (#840838)
......................................................................
Patch Set 7: (11 inline comments)
answered amureini comments
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java
Line 119: || suppressCheck) {
It's an existing logic that was here before of my change, it can be re factored
in a further patch as I don't want to change the pre existing logic in this
patch.
Line 126: if (!returnValue) {
as in 'regular' execution flow we are not expected to even enter that if - I
don't think that it's necessarily needed to define another boolean inside the
if just for this case and after two lines assign it's value to return value.
please reconsider - if it's still needed by your opinion i'll add it.
Line 130: returnValue = _connectPoolSucceeded =
executeConnectStoragePoolCommand(storagePoolGuid);
It's an existing logic that was here before of my change, it can be re factored
in a further patch as I don't want to change the pre existing logic in this
patch.
Line 137: // reconstruct the master domain as well.
Agreed, I submitted it as WIP to get reviews to the 'logical part' of the patch
- if reviews will approve i'll send a further patch with edited message as well.
Line 154: private void reloadStoragePool() {
I don't think that when reading the code it's clear that nullifying it will
cause the inner method to reload it. In this code we know for sure that we need
to reload it - if in the future it won't be need to be reloaded, this call can
be replaced with nullifying.
Line 159: public boolean reconstructMasterDomain(Guid storagePoolId) {
agreed.
Line 167: .getInstance()
Done
Line 172: } catch (RuntimeException exp) {
It's an existing logic that was here before of my change, it can be re factored
in a further patch as I don't want to change the pre existing logic in this
patch.
Line 179: public boolean executeConnectStoragePoolCommand(Guid
storagePoolId) {
Done
Line 184: .RunVdsCommand(
Done
Line 191: } catch (RuntimeException exp) {
It's an existing logic that was here before of my change, it can be re factored
in a further patch as I don't want to change the pre existing logic in this
patch.
--
To view, visit http://gerrit.ovirt.org/7061
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iec43f368370da88f81dbe211331649a16c53f053
Gerrit-PatchSet: 7
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: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
