Ayal Baron has posted comments on this change. Change subject: core: avoid having SD active on vdsm and not in engine ......................................................................
Patch Set 3: (3 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ReconstructMasterDomainCommand.java Line 36: @NonTransactiveCommandAttribute(forceCompensation = true) Line 37: public class ReconstructMasterDomainCommand<T extends ReconstructMasterParameters> extends Line 38: DeactivateStorageDomainCommand<T> { Line 39: Line 40: protected boolean reconstructOpsSucceeded; it's reconstructOp not Ops. Line 41: /** Line 42: * Constructor for command creation when compensation is applied on startup Line 43: * Line 44: * @param commandId Line 133: try { Line 134: reconstructOpsSucceeded = reconstructMaster(); Line 135: connectAndRefreshAllUpHosts(reconstructOpsSucceeded); Line 136: Line 137: if (!_isLastMaster && reconstructOpsSucceeded) { couldn't have connectAndRefreshAllUpHosts disconnected the storage and induce failure of updateVmInSpm? Line 138: SearchParameters p = new SearchParameters( Line 139: MessageFormat.format(DesktopsInStoragePoolQuery, Line 140: getStoragePool().getname()), SearchType.VM); Line 141: Line 217: returnValue.getVdsError().getMessage()); Line 218: } Line 219: } Line 220: // only if we deactivate the storage domain we want to disconnect from it. Line 221: if (!getParameters().isInactive()) { You might get here if _isLastMaster is true and commandSucceeded. Iiuc though, you should not disconnect if commandSucceeded is true. In fact, I'm not sure what this piece of code is doing here at all. It seems logically unrelated to the above part, it doesn't correspond with the method name and I'm not clear on why we'd want to disconnect to begin with. Line 222: StorageHelperDirector.getInstance() Line 223: .getItem(getStorageDomain().getstorage_type()) Line 224: .DisconnectStorageFromDomainByVdsId(getStorageDomain(), vds.getId()); Line 225: } -- To view, visit http://gerrit.ovirt.org/8390 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ida51b027b8078555ffab5b80d2ffe54572fa6cca Gerrit-PatchSet: 3 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: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Michael Kublin <mkub...@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