Gilad Chaplik has uploaded a new change for review.
Change subject: core: refactoring VdsSelector API
......................................................................
core: refactoring VdsSelector API
Host selection should become stateless,
therefore moving failed host list out of this class (vdsBlackList).
In addition, each vdsSelector call should supply a list of hosts
that the selector should 'work' on.
new VdsSelector API:
// for Validation purposes
canFindVdsToRunOn(List<VDS> vdsList,
List<Guid> vdsBlackList,
List<String> messages,
boolean isMigrate)
// for selecting a host to run on
Guid getVdsToRunOn(List<VDS> vdsList,
List<Guid> vdsBlackList,
boolean isMigrate)
Change-Id: I6d936e1b96628e426e944d77730e3ca08a13570d
Signed-off-by: Gilad Chaplik <[email protected]>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/VdsSelector.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
M
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
7 files changed, 89 insertions(+), 62 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/97/14997/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
index ff83c85..abead6e 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
@@ -14,6 +14,7 @@
import org.ovirt.engine.core.common.businessentities.MigrationSupport;
import org.ovirt.engine.core.common.businessentities.VDS;
import org.ovirt.engine.core.common.businessentities.VDSStatus;
+import org.ovirt.engine.core.common.businessentities.VDSType;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
import org.ovirt.engine.core.common.businessentities.network.Network;
@@ -82,7 +83,12 @@
protected void initVdss() {
setVdsIdRef(new Guid(getVm().getRunOnVds().toString()));
- setVdsDestinationId(getVdsSelector().getVdsToRunOn(true));
+ Guid vdsToRunOn = getVdsSelector().getVdsToRunOn(getVdsDAO()
+ .getAllOfTypes(new VDSType[] { VDSType.VDS, VDSType.oVirtNode
}), getRunVdssList(), true);
+ setVdsDestinationId(vdsToRunOn);
+ if (vdsToRunOn != null && !Guid.Empty.equals(vdsToRunOn)) {
+ getRunVdssList().add(vdsToRunOn);
+ }
VmHandler.UpdateVmGuestAgentVersion(getVm());
// make _destinationVds null in order to refresh it from db in case it
// changed.
@@ -263,7 +269,11 @@
// This check was added to prevent migration of VM while its
disks are being migrated
// TODO: replace it with a better solution
&& validate(new
DiskImagesValidator(ImagesHandler.getPluggedImagesForVm(vm.getId())).diskImagesNotLocked())
- &&
getVdsSelector().canFindVdsToRunOn(getReturnValue().getCanDoActionMessages(),
true);
+ && getVdsSelector().canFindVdsToRunOn(getVdsDAO()
+ .getAllOfTypes(new VDSType[] { VDSType.VDS,
VDSType.oVirtNode }),
+ getRunVdssList(),
+ getReturnValue().getCanDoActionMessages(),
+ true);
}
@Override
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
index 54b43f5..87695b0 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
@@ -44,6 +44,7 @@
import org.ovirt.engine.core.common.businessentities.StorageDomainStatus;
import org.ovirt.engine.core.common.businessentities.StorageDomainType;
import org.ovirt.engine.core.common.businessentities.VDS;
+import org.ovirt.engine.core.common.businessentities.VDSType;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
import org.ovirt.engine.core.common.businessentities.VmPool;
@@ -561,7 +562,13 @@
protected boolean getVdsToRunOn() {
// use destination vds or default vds or none
- setVdsId(getVdsSelector().getVdsToRunOn(false));
+ List<VDS> vdsList = getVdsDAO()
+ .getAllOfTypes(new VDSType[] { VDSType.VDS, VDSType.oVirtNode
});
+ Guid vdsToRunOn = getVdsSelector().getVdsToRunOn(vdsList,
getRunVdssList(), false);
+ setVdsId(vdsToRunOn);
+ if (vdsToRunOn != null && !Guid.Empty.equals(vdsToRunOn)) {
+ getRunVdssList().add(vdsToRunOn);
+ }
VmHandler.UpdateVmGuestAgentVersion(getVm());
setVds(null);
setVdsName(null);
@@ -687,7 +694,8 @@
getParameters().getDiskPath(),
getParameters().getFloppyPath(),
getParameters().getRunAsStateless(),
- getVdsSelector()) &&
+ getVdsSelector(),
+ getRunVdssList()) &&
validateNetworkInterfaces();
// check for Vm Payload
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
index d9262ef..90f7796 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommandBase.java
@@ -56,6 +56,7 @@
protected boolean _isRerun = false;
protected VDS _destinationVds;
private SnapshotsValidator snapshotsValidator=new SnapshotsValidator();
+ private final List<Guid> runVdsList = new ArrayList<Guid>();
protected RunVmCommandBase(Guid commandId) {
super(commandId);
@@ -80,10 +81,10 @@
}
/**
- * List on all vdss, vm run on. In the case of problem to run vm will be
more then one
+ * List on all VDSs, vm run on. In the case of problem to run vm will be
more then one
*/
- private List<Guid> getRunVdssList() {
- return getVdsSelector().getRunVdssList();
+ protected List<Guid> getRunVdssList() {
+ return runVdsList;
}
@Override
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
index 030942d..1b90a1e 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmPoolCommandBase.java
@@ -242,7 +242,8 @@
runVmParams.getDiskPath(),
runVmParams.getFloppyPath(),
runVmParams.getRunAsStateless(),
- vdsSelector);
+ vdsSelector,
+ new ArrayList<Guid>());
}
private static DiskDao getDiskDao() {
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/VdsSelector.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/VdsSelector.java
index 24b5b36..0b69fcb 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/VdsSelector.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/VdsSelector.java
@@ -12,7 +12,6 @@
import org.ovirt.engine.core.common.businessentities.VDS;
import org.ovirt.engine.core.common.businessentities.VDSGroup;
import org.ovirt.engine.core.common.businessentities.VDSStatus;
-import org.ovirt.engine.core.common.businessentities.VDSType;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
import org.ovirt.engine.core.common.businessentities.network.Network;
@@ -32,14 +31,10 @@
import org.ovirt.engine.core.utils.log.LogFactory;
public class VdsSelector {
- private final List<Guid> privateRunVdssList = new ArrayList<Guid>();
private List<VmNetworkInterface> vmNICs;
private boolean displayNetworkInitialized;
private Network displayNetwork;
-
- public List<Guid> getRunVdssList() {
- return privateRunVdssList;
- }
+ private List<Guid> vdsBlackList;
private NGuid privateDestinationVdsId;
@@ -68,48 +63,70 @@
this.memoryChecker = memoryChecker;
}
- public Guid getVdsToRunOn(boolean isMigrate) {
+ public Guid getVdsToRunOn(List<VDS> vdsList,
+ List<Guid> vdsBlackList,
+ boolean isMigrate) {
+ this.vdsBlackList = vdsBlackList;
Guid result = Guid.Empty;
if (getDestinationVdsId() != null) {
- VDS targetVds =
DbFacade.getInstance().getVdsDao().get(getDestinationVdsId());
- log.infoFormat("Checking for a specific VDS only - id:{0},
name:{1}, host_name(ip):{2}",
- getDestinationVdsId(), targetVds.getName(),
targetVds.getHostName());
- result = getVdsToRunOn(new ArrayList<VDS>(Arrays.asList(new VDS[]
{ targetVds })), isMigrate);
+ VDS targetVds = null;
+ if (vdsList != null) {
+ for (VDS vds : vdsList) {
+ if (vds.getId().equals(getDestinationVdsId())) {
+ targetVds = vds;
+ break;
+ }
+ }
+ }
+ if (targetVds != null) {
+ log.infoFormat("Checking for a specific VDS only - id:{0},
name:{1}, host_name(ip):{2}",
+ getDestinationVdsId(), targetVds.getName(),
targetVds.getHostName());
+ result = getVdsToRunOn(new ArrayList<VDS>(Arrays.asList(new
VDS[] { targetVds })), isMigrate);
+ }
}
if (getDestinationVdsId() == null || result.equals(Guid.Empty)
&& privateVm.getMigrationSupport() !=
MigrationSupport.PINNED_TO_HOST) {
- result = getVdsToRunOn(DbFacade.getInstance()
- .getVdsDao()
- .getAllOfTypes(new VDSType[] { VDSType.VDS,
VDSType.oVirtNode }), isMigrate);
+ result = getVdsToRunOn(vdsList, isMigrate);
}
return result;
}
- public boolean canFindVdsToRunOn(List<String> messages, boolean isMigrate)
{
+ public boolean canFindVdsToRunOn(List<VDS> vdsList,
+ List<Guid> vdsBlackList,
+ List<String> messages,
+ boolean isMigrate) {
+ this.vdsBlackList = vdsBlackList;
boolean returnValue = false;
if (getDestinationVdsId() != null) {
- VDS targetVds =
DbFacade.getInstance().getVdsDao().get(getDestinationVdsId());
- log.infoFormat("Checking for a specific VDS only - id:{0},
name:{1}, host_name(ip):{2}",
- getDestinationVdsId(), targetVds.getName(),
targetVds.getHostName());
- returnValue = canFindVdsToRun(messages, isMigrate,
- new ArrayList<VDS>(Arrays.asList(targetVds)));
- }
+ VDS targetVds = null;
+ if (vdsList != null) {
+ for (VDS vds : vdsList) {
+ if (vds.getId().equals(getDestinationVdsId())) {
+ targetVds = vds;
+ break;
+ }
+ }
+ }
- if (!returnValue) {
- if (privateVm.getMigrationSupport() ==
MigrationSupport.PINNED_TO_HOST) {
+ if (targetVds != null) {
+ log.infoFormat("Checking for a specific VDS only - id:{0},
name:{1}, host_name(ip):{2}",
+ getDestinationVdsId(), targetVds.getName(),
targetVds.getHostName());
+ returnValue = canFindVdsToRun(messages, isMigrate,
+ new ArrayList<VDS>(Arrays.asList(targetVds)));
+ }
+ if (!returnValue && privateVm.getMigrationSupport() ==
MigrationSupport.PINNED_TO_HOST) {
messages.add(VdcBllMessages.VM_PINNED_TO_HOST_CANNOT_RUN_ON_THE_DEFAULT_VDS.toString());
- VDS host = getDestinationVdsId() == null ? null :
DbFacade.getInstance()
- .getVdsDao()
- .get(getDestinationVdsId());
-
- messages.add(host == null ?
VdcBllMessages.HOST_NAME_NOT_AVAILABLE.toString()
- : String.format("$VdsName %1$s",
host.getName()));
+ messages.add(targetVds == null ?
VdcBllMessages.HOST_NAME_NOT_AVAILABLE.toString()
+ : String.format("$VdsName %1$s", targetVds.getName()));
return false;
}
+ }
+
+ if (!returnValue) {
returnValue = canFindVdsToRun(messages, isMigrate,
- DbFacade.getInstance().getVdsDao().getAllOfTypes(new
VDSType[] { VDSType.VDS, VDSType.oVirtNode }));
+ vdsList);
}
return returnValue;
@@ -264,7 +281,7 @@
@Override
public VdcBllMessages validate(VDS vds, StringBuilder sb,
boolean isMigrate) {
- if (isVdsFailedToRunVm(vds.getId())) {
+ if (vdsBlackList != null &&
vdsBlackList.contains(vds.getId())) {
sb.append("have failed running this VM in the current
selection cycle");
return
VdcBllMessages.ACTION_TYPE_FAILED_VDS_VM_CLUSTER;
}
@@ -289,27 +306,10 @@
}
/**
- * Determine if specific vds already failed to run vm - to prevent
- * sequential running of vm on problematic vds
- *
- * @param vdsId
- * @return
- */
- private boolean isVdsFailedToRunVm(Guid vdsId) {
- boolean retValue = false;
- if (getRunVdssList() != null && getRunVdssList().contains(vdsId)) {
- retValue = true;
- }
- return retValue;
- }
-
- /**
* Determines whether [is VM swap value legal] [the specified VDS].
- *
* @param vds
* The VDS.
- * @return <c>true</c> if [is VM swap value legal] [the specified VDS];
- * otherwise, <c>false</c>.
+ * @return <c>true</c> if [is VM swap value legal] [the specified VDS];
otherwise, <c>false</c>.
*/
private static boolean isVMSwapValueLegal(VDS vds) {
if (!Config.<Boolean> GetValue(ConfigValues.EnableSwapCheck)) {
@@ -483,7 +483,6 @@
* add chosen vds to running vdss list.
*/
comparer.bestVdsProcedure(bestVDS);
- getRunVdssList().add(bestVDS.getId());
return bestVDS.getId();
}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
index 19ba833..eaa806a 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/RunVmValidator.java
@@ -28,6 +28,7 @@
import org.ovirt.engine.core.common.businessentities.StoragePool;
import org.ovirt.engine.core.common.businessentities.VDS;
import org.ovirt.engine.core.common.businessentities.VDSStatus;
+import org.ovirt.engine.core.common.businessentities.VDSType;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
import org.ovirt.engine.core.common.config.Config;
@@ -330,7 +331,6 @@
/**
* A general method for run vm validations. used in runVmCommand and in
VmPoolCommandBase
- *
* @param vm
* @param messages
* @param vmDisks
@@ -341,6 +341,8 @@
* @param floppyPath
* @param runAsStateless
* @param vdsSelector
+ * @param vdsBlackList
+ * - hosts that we already tried to run on
* @return
*/
public boolean canRunVm(VM vm,
@@ -351,7 +353,9 @@
boolean isInternalExecution,
String diskPath,
String floppyPath,
- Boolean runAsStateless, VdsSelector vdsSelector) {
+ Boolean runAsStateless,
+ VdsSelector vdsSelector,
+ List<Guid> vdsBlackList) {
if (!validateVmProperties(vm, messages)) {
return false;
}
@@ -402,7 +406,10 @@
return false;
}
}
- if (!vdsSelector.canFindVdsToRunOn(messages, false)) {
+ if (!vdsSelector.canFindVdsToRunOn(getVdsDao().getAllOfTypes(new
VDSType[] { VDSType.VDS, VDSType.oVirtNode }),
+ vdsBlackList,
+ messages,
+ false)) {
return false;
}
result = validateVmStatusUsingMatrix(vm);
diff --git
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
index d4b3885..5001f47 100644
---
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
+++
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/RunVmCommandTest.java
@@ -362,7 +362,8 @@
anyString(),
anyString(),
anyBoolean(),
- any(VdsSelector.class))).thenReturn(true);
+ any(VdsSelector.class),
+ Matchers.anyListOf(Guid.class))).thenReturn(true);
doReturn(runVmValidator).when(command).getRunVmValidator();
return runVmValidator;
}
--
To view, visit http://gerrit.ovirt.org/14997
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6d936e1b96628e426e944d77730e3ca08a13570d
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
