Allon Mureinik has posted comments on this change.
Change subject: core: Allow force re-election of a specific host as SPM
......................................................................
Patch Set 4: (11 inline comments)
will REST be provided in another patch?
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ReinitializeSPMCommand.java
Line 16: import org.ovirt.engine.core.compat.Guid;
Line 17: import
org.ovirt.engine.core.vdsbroker.irsbroker.SpmStopOnIrsVDSCommandParameters;
Line 18:
Line 19: @NonTransactiveCommandAttribute
Line 20: public class ReinitializeSPMCommand<T extends
ReinitializeSPMParameters> extends CommandBase<T> {
Not a fan of the name.
How about ForceElectSPMCommand or something of the sort?
Line 21:
Line 22: public ReinitializeSPMCommand(T parameters) {
Line 23: super(parameters);
Line 24: setVdsId(getParameters().getPreferredSPMId());
Line 42: if (getVds().getSpmStatus() != VdsSpmStatus.None) {
Line 43: return
failCanDo(VdcBllMessages.CANNOT_REINIT_SPM_VDS_ALREADY_SPM);
Line 44: }
Line 45:
Line 46: if (getVds().getVdsSpmPriority() ==
BusinessEntitiesDefinitions.HOST_MIN_SPM_PRIORITY) {
why?
Line 47: return
failCanDo(VdcBllMessages.CANNOT_REINIT_SPM_VDS_MARKED_AS_NEVER_SPM);
Line 48: }
Line 49:
Line 50: if
(isAsyncTasksRunningOnPool(getParameters().getStoragePoolId())) {
Line 54: return true;
Line 55: }
Line 56:
Line 57: private boolean failCanDo(VdcBllMessages message) {
Line 58: addCanDoActionMessage(String.format("$%1$s %2$s", "VdsName",
getVds().getName()));
Why can't this be done in setActionMessageParameters ?
Line 59: return failCanDoAction(message);
Line 60: }
Line 61:
Line 62: @Override
Line 75: }
Line 76:
Line 77: private boolean isAsyncTasksRunningOnPool(Guid storagePoolId) {
Line 78: List<Guid> tasks =
getAsyncTaskDao().getAsyncTaskIdsByStoragePoolId(storagePoolId);
Line 79: return !tasks.isEmpty();
I'd inline this
Line 80: }
Line 81:
Line 82: @Override
Line 83: public AuditLogType getAuditLogTypeValue() {
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommandAssertUtils.java
Line 24: assertEquals("Wrong message", expected[i].toString(),
returned.get(i));
Line 25: }
Line 26: }
Line 27:
Line 28: /*
Not javadoc - should have "/**"
Line 29: * This method checks if the return CDA messages contain the
expected messages, it is different from checkMessages
Line 30: * by the fact that it does not check by the order the parameters
were given, the order is irrelevant most of the
Line 31: * time and it does not check that the size of the returned
messages matches the size of the expected messages, this
Line 32: * comes in handy for example in scenarios where the CDA messages
return a resolved parameters (e.g. '$VmName MyVM')
Line 36: for (int i = 0; i < expected.length; i++) {
Line 37: assertTrue("CanDoAction message does not contain the
message " + expected[i],
Line 38: cdaMessages.contains(expected[i].toString()));
Line 39: }
Line 40: }
You have this functionality in ValidationResultMatchers
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ReinitializeSPMCommandTest.java
Line 91: private void mockDAOs() {
Line 92: vdsDAOMock = mock(VdsDAO.class);
Line 93: storagePoolDAOMock = mock(StoragePoolDAO.class);
Line 94: asyncTaskDAOMock = mock(AsyncTaskDAO.class);
Line 95: }
why not do all of these with annotations?
Line 96:
Line 97: private void createVDS() {
Line 98: vds = new VDS();
Line 99: vds.setId(vdsId);
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/ReinitializeSPMParameters.java
Line 2:
Line 3: import org.ovirt.engine.core.compat.Guid;
Line 4:
Line 5: public class ReinitializeSPMParameters extends VdcActionParametersBase {
Line 6:
Probably needs a SerialVersionUUID
Line 7: private Guid preferredSPMId;
Line 8: private Guid storagePoolId = Guid.Empty;
Line 9:
Line 10: public ReinitializeSPMParameters(Guid storagePoolId, Guid
prefferedSPMId) {
Line 6:
Line 7: private Guid preferredSPMId;
Line 8: private Guid storagePoolId = Guid.Empty;
Line 9:
Line 10: public ReinitializeSPMParameters(Guid storagePoolId, Guid
prefferedSPMId) {
ack
Line 11: setStoragePoolId(storagePoolId);
Line 12: setPreferredSPMId(prefferedSPMId);
Line 13: }
Line 14:
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
Line 709: for (VDS vds : allVds) {
Line 710: if (!mTriedVdssList.contains(vds.getId()) &&
!vds.getId().equals(curVdsId)) {
Line 711: if (vds.getId().equals(preferredHost)) {
Line 712: vdsRelevantForSpmSelection.add(0, vds);
Line 713: }
ack
Line 714: else {
Line 715: vdsRelevantForSpmSelection.add(vds);
Line 716: }
Line 717: }
....................................................
Commit Message
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: resign the previously elected SPM
Line 11:
feature page?
Line 12: Change-Id: Iab31fc7918b10448b923821547a583f3d6c3fcba
--
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: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <[email protected]>
Gerrit-Reviewer: A Burden <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
