Liron Aravot has posted comments on this change. Change subject: core: move getNewMaster to ReconstructMasterDomain ......................................................................
Patch Set 16: (1 comment) https://gerrit.ovirt.org/#/c/28444/16/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ReconstructMasterDomainCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ReconstructMasterDomainCommand.java: Line 109: } Line 110: Line 111: protected boolean reconstructMaster() { Line 112: proceedStorageDomainTreatmentByDomainType(getNewMaster(), false); Line 113: Rethinking about my comment from patch #12 - I think that there's some small risk in removing the ensurance that we had with newMasterStorageDomainId. for example (_isLastMaster will be set to true while on line 115 getNewMasterId() might return this domain in theory which will cause to an unexpected result). I'd consider to keep the current behavior on that part. I'm not blocking this change, but i'd prefer that we ensure that we attempt to select a new master only once - please consider that. Line 114: // To issue a reconstructMaster you need to set the domain inactive unless the selected domain is the current master Line 115: if (getParameters().isInactive() && !getStorageDomain().getId().equals(getNewMasterId())) { Line 116: executeInNewTransaction(new TransactionMethod<Void>() { Line 117: @Override -- To view, visit https://gerrit.ovirt.org/28444 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I30d342a4c5802cdfbf2c78b50dad4c797c137fe2 Gerrit-PatchSet: 16 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
