Alissa Bonas has posted comments on this change.
Change subject: core: Allow force re-election of a specific host as SPM
......................................................................
Patch Set 1: (10 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ReinitializeSPMCommand.java
Line 63: protected void executeCommand() {
Line 64: SpmStopOnIrsVDSCommandParameters params =
Line 65: new
SpmStopOnIrsVDSCommandParameters(getParameters().getStoragePoolId(),
Line 66: getParameters().getPreferredSPMId());
Line 67: runVdsCommand(VDSCommandType.SpmStopOnIrs, params);
Why isn't the ResetIrs command called here directly? SpmStopOnIrs just calls to
resetIrs command, so why is this delegation needed?
Line 68: setSucceeded(true);
Line 69: }
Line 70:
Line 71: @Override
Line 64: SpmStopOnIrsVDSCommandParameters params =
Line 65: new
SpmStopOnIrsVDSCommandParameters(getParameters().getStoragePoolId(),
Line 66: getParameters().getPreferredSPMId());
Line 67: runVdsCommand(VDSCommandType.SpmStopOnIrs, params);
Line 68: setSucceeded(true);
does it always succeed?
Line 69: }
Line 70:
Line 71: @Override
Line 72: protected void setActionMessageParameters() {
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogableBase.java
Line 613: protected NetworkDao getNetworkDAO() {
Line 614: return getDbFacade().getNetworkDao();
Line 615: }
Line 616:
Line 617: public AsyncTaskDAO getAsyncTaskDao() {
why is this changed to public? was it problematic as protected?
Line 618: return getDbFacade().getAsyncTaskDao();
Line 619: }
Line 620:
Line 621: protected DbFacade getDbFacade() {
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
Line 134: private final String storagePoolRefreshJobId;
Line 135: private final java.util.HashSet<Guid> mTriedVdssList = new
java.util.HashSet<Guid>();
Line 136: private Guid mCurrentVdsId;
Line 137:
Line 138: private Guid preferredHost;
consider changing preferredHost to preferredHostId
Line 139:
Line 140: public Guid getCurrentVdsId() {
Line 141: return getIrsProxy() != null ? mCurrentVdsId : Guid.Empty;
Line 142: }
Line 703: // deal with the case that there are several hosts with
the same priority.
Line 704: List<VDS> allVds =
DbFacade.getInstance().getVdsDao().getListForSpmSelection(_storagePoolId);
Line 705: List<VDS> vdsRelevantForSpmSelection = new
ArrayList<VDS>();
Line 706: Guid preferredHost =
getIrsProxyData(_storagePoolId).getPreferredHost();
Line 707: getIrsProxyData(_storagePoolId).setPreferredHost(null);
why the preffered host needs "zeroing" (setting to null) ?
Line 708:
Line 709: for (VDS vds : allVds) {
Line 710: if (!mTriedVdssList.contains(vds.getId()) &&
!vds.getId().equals(curVdsId)) {
Line 711: if (vds.getId().equals(preferredHost)) {
....................................................
Commit Message
Line 6:
Line 7: core: Allow force re-election of a specific host as SPM
Line 8:
Line 9: Allow the user to select a non SPM host and force elect it as SPM and
Line 10: resigning the previous elected SPM
resigning -->resign
previous-->previously
Line 11:
Line 12: Change-Id: Iab31fc7918b10448b923821547a583f3d6c3fcba
Line 13: Bug-Url: https://bugzilla.redhat.com/629034
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
Line 183:
Line 184: @DefaultStringValue("Cannot ${action} ${type}. Internal Error:
Host does not exists in DB.")
Line 185: String VDS_NOT_EXIST();
Line 186:
Line 187: @DefaultStringValue("Cannot ${action} ${type}. The Host
${VdsName} is not active.")
Host should start with lowercase since it is in the middle of a sentence. same
applies to other messages below.
Line 188: String CANNOT_REINIT_SPM_VDS_NOT_UP();
Line 189:
Line 190: @DefaultStringValue("Cannot ${action} ${type}. The Host
${VdsName} is already SPM or contending.")
Line 191: String CANNOT_REINIT_SPM_VDS_ALREADY_SPM();
Line 192:
Line 193: @DefaultStringValue("Cannot ${action} ${type}. The SPM priority
of Host ${VdsName} is set to never. This Host cannot be elected as SPM.")
Line 194: String CANNOT_REINIT_SPM_VDS_MARKED_AS_NEVER_SPM();
Line 195:
Line 196: @DefaultStringValue("Cannot ${action} ${type}. The Storage Pool
has running tasks.")
It's not very clear what that means to user or which action should be taken.
maybe something should be added like "please wait till they complete, or cancel
the tasks ${taskNames}" ?
Line 197: String CANNOT_REINIT_SPM_STORAGE_POOL_HAS_RUNNING_TASKS();
Line 198:
Line 199: @DefaultStringValue("Internal error: Host protocol error.")
Line 200: String VDS_PROTOCOL_ERROR();
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostListModel.java
Line 136: }
Line 137:
Line 138: private UICommand setAsSpmCommand;
Line 139:
Line 140: public UICommand getSetAsSpmCommand()
maybe it's better to rename the UICommand, because the getter method now looks
strange "getSet...." - it's confusing whether it's a getter or a setter.
Line 141: {
Line 142: return setAsSpmCommand;
Line 143: }
Line 144:
Line 141: {
Line 142: return setAsSpmCommand;
Line 143: }
Line 144:
Line 145: private void setSetAsSpmCommand(UICommand value)
same here as above - "setSet" is strange...
Line 146: {
Line 147: setAsSpmCommand = value;
Line 148: }
Line 149:
--
To view, visit http://gerrit.ovirt.org/16105
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iab31fc7918b10448b923821547a583f3d6c3fcba
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
