Tomas Jelinek has uploaded a new change for review. Change subject: engine: cannot unpause VM started in paused mode ......................................................................
engine: cannot unpause VM started in paused mode Have 2 clusters, in each have one host. In one of this clusters create a VM. Run it in pause mode, wait until it starts in the pause mode and tham start it. The VM will not start. The problem was in VdsSelector in the check where it tried to find out, if the specific VM is migrating to the same host. This check makes sense for the migrating of the VM, but not for running it. Fixed by making the checks aware of if the VM is migrating or not. This patch also removes one duplication - one of the checks has been copy-pasted twice. There was also a workaround which ignored the situation that the VM would be migrated to the same host - it is already implmented in the HostValidator so removed. Change-Id: I8cca521f79bfb7d5762f2c996200916a10afc36a Bug-Url: https://bugzilla.redhat.com/872205 Signed-off-by: Tomas Jelinek <[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/PowerClientMigrateOnConnectCheckCommand.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/VdsSelector.java 4 files changed, 28 insertions(+), 49 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/94/8994/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 2d933f8..36ff593 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 @@ -80,7 +80,7 @@ protected void initVdss() { setVdsIdRef(new Guid(getVm().getrun_on_vds().toString())); - setVdsDestinationId(getVdsSelector().getVdsToRunOn()); + setVdsDestinationId(getVdsSelector().getVdsToRunOn(true)); // make _destinationVds null in order to refresh it from db in case it // changed. _destinationVds = null; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/PowerClientMigrateOnConnectCheckCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/PowerClientMigrateOnConnectCheckCommand.java index b2b0b06..baa8860 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/PowerClientMigrateOnConnectCheckCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/PowerClientMigrateOnConnectCheckCommand.java @@ -98,7 +98,7 @@ && Config.<Boolean> GetValue(ConfigValues.PowerClientAutoMigrateToPowerClientOnConnect)) { // check if we can run (migrate) the vm to the power client. - Guid checkVds_id = getVdsSelector().getVdsToRunOn(); + Guid checkVds_id = getVdsSelector().getVdsToRunOn(true); if (!(checkVds_id).equals(getPowerClient().getId())) { log.infoFormat("VdcBll.PowerClientMigrateOnConnectCheck - Can't migrate to power client, since getVdsToRunOn rejected the run"); // return; // no return, so we can continue and migrate from a 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 ae9a158..35a5b09 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 @@ -521,7 +521,7 @@ protected boolean getVdsToRunOn() { // use destination vds or default vds or none - setVdsId(getVdsSelector().getVdsToRunOn()); + setVdsId(getVdsSelector().getVdsToRunOn(false)); setVds(null); setVdsName(null); if (getVdsId().equals(Guid.Empty)) { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsSelector.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsSelector.java index 13c36d1..003d5b8 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsSelector.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsSelector.java @@ -76,22 +76,22 @@ this.memoryChecker = memoryChecker; } - public Guid getVdsToRunOn() { + public Guid getVdsToRunOn(boolean isMigrate) { Guid result = Guid.Empty; if (getDestinationVdsId() != null) { if (getCheckDestinationFirst()) { - result = getVdsRunOnDestination(); + result = getVdsRunOnDestination(isMigrate); if (result.equals(Guid.Empty) && privateVm.getMigrationSupport() != MigrationSupport.PINNED_TO_HOST) { - result = getAnyVdsToRunOn(); + result = getAnyVdsToRunOn(isMigrate); } } else { - result = getAnyVdsToRunOn(); + result = getAnyVdsToRunOn(isMigrate); if (result.equals(Guid.Empty)) { - result = getVdsRunOnDestination(); + result = getVdsRunOnDestination(isMigrate); } } } else { - result = getAnyVdsToRunOn(); + result = getAnyVdsToRunOn(isMigrate); } return result; @@ -126,7 +126,7 @@ * getDestinationVdsId() must not be null. * @return */ - private Guid getVdsRunOnDestination() { + private Guid getVdsRunOnDestination(boolean isMigrate) { Guid result = Guid.Empty; if (getDestinationVdsId() != null) { VDS target_vds = DbFacade.getInstance().getVdsDao().get(getDestinationVdsId()); @@ -140,16 +140,16 @@ "VdcBLL.RunVmCommandBase.getVdsToRunOn - VM {0} has no tools - skipping power client check", getVm().getId()); } else { - result = getVdsToRunOn(new ArrayList<VDS>(Arrays.asList(new VDS[] { target_vds }))); + result = getVdsToRunOn(new ArrayList<VDS>(Arrays.asList(new VDS[] { target_vds })), isMigrate); } } return result; } - private Guid getAnyVdsToRunOn() { + private Guid getAnyVdsToRunOn(boolean isMigrate) { return getVdsToRunOn(DbFacade.getInstance() .getVdsDao() - .getAllOfTypes(new VDSType[] { VDSType.VDS, VDSType.oVirtNode })); + .getAllOfTypes(new VDSType[] { VDSType.VDS, VDSType.oVirtNode }), isMigrate); } private boolean canRunOnDestinationVds(List<String> messages, boolean isMigrate) { @@ -187,13 +187,9 @@ boolean noVDSs = true; StringBuilder sb = new StringBuilder(); for (VDS curVds : vdss) { - if (isMigrate && getVm().getrun_on_vds() != null && getVm().getrun_on_vds().equals(curVds.getId())) { - continue; - } - noVDSs = false; - ValidationResult result = validateHostIsReadyToRun(curVds, sb); + ValidationResult result = validateHostIsReadyToRun(curVds, sb, isMigrate); if (result.isValid()) { return true; } else { @@ -229,7 +225,7 @@ } interface HostValidator { - VdcBllMessages validate(VDS vds, StringBuilder sb); + VdcBllMessages validate(VDS vds, StringBuilder sb, boolean isMigrate); } @SuppressWarnings("serial") @@ -238,7 +234,7 @@ add(new HostValidator() { @Override - public VdcBllMessages validate(VDS vds, StringBuilder sb) { + public VdcBllMessages validate(VDS vds, StringBuilder sb, boolean isMigrate) { if ((!vds.getvds_group_id().equals(getVm().getvds_group_id())) || (vds.getstatus() != VDSStatus.Up)) { sb.append("is not in up status or belongs to the VM's cluster"); return VdcBllMessages.ACTION_TYPE_FAILED_VDS_VM_CLUSTER; @@ -249,12 +245,10 @@ add(new HostValidator() { @Override - public VdcBllMessages validate(VDS vds, StringBuilder sb) { + public VdcBllMessages validate(VDS vds, StringBuilder sb, boolean isMigrate) { // If Vm in Paused mode - no additional memory allocation needed if (getVm().getstatus() != VMStatus.Paused && !memoryChecker.evaluate(vds, getVm())) { // not enough memory - // In case we are using this function in migration we make sure we - // don't allocate the same VDS sb.append("has insufficient memory to run the VM"); return VdcBllMessages.ACTION_TYPE_FAILED_VDS_VM_MEMORY; } @@ -264,25 +258,10 @@ add(new HostValidator() { @Override - public VdcBllMessages validate(VDS vds, StringBuilder sb) { - // If Vm in Paused mode - no additional memory allocation needed - if (getVm().getstatus() != VMStatus.Paused && !memoryChecker.evaluate(vds, getVm())) { - // not enough memory - // In case we are using this function in migration we make sure we - // don't allocate the same VDS - sb.append("has insufficient memory to run the VM"); - return VdcBllMessages.ACTION_TYPE_FAILED_VDS_VM_MEMORY; - } - return null; - } - }); - add(new HostValidator() { - - @Override - public VdcBllMessages validate(VDS vds, StringBuilder sb) { + public VdcBllMessages validate(VDS vds, StringBuilder sb, boolean isMigrate) { // In case we are using this function in migration we make sure we // don't allocate the same VDS - if ((getVm().getrun_on_vds() != null && getVm().getrun_on_vds().equals(vds.getId()))) { + if (isMigrate && (getVm().getrun_on_vds() != null && getVm().getrun_on_vds().equals(vds.getId()))) { sb.append("is the same host the VM is currently running on"); return VdcBllMessages.ACTION_TYPE_FAILED_MIGRATION_TO_SAME_HOST; } @@ -292,7 +271,7 @@ add(new HostValidator() { @Override - public VdcBllMessages validate(VDS vds, StringBuilder sb) { + public VdcBllMessages validate(VDS vds, StringBuilder sb, boolean isMigrate) { // check capacity to run power clients if (!RunVmCommandBase.hasCapacityToRunVM(vds)) { sb.append("has insuffient capacity to run power client"); @@ -303,7 +282,7 @@ }); add(new HostValidator() { @Override - public VdcBllMessages validate(VDS vds, StringBuilder sb) { + public VdcBllMessages validate(VDS vds, StringBuilder sb, boolean isMigrate) { if (vds.getcpu_cores() != null && getVm().getnum_of_cpus() > vds.getcpu_cores()) { sb.append("has less cores(").append(vds.getcpu_cores()).append(") than ").append(getVm().getnum_of_cpus()); return VdcBllMessages.ACTION_TYPE_FAILED_VDS_VM_CPUS; @@ -314,7 +293,7 @@ add(new HostValidator() { @Override - public VdcBllMessages validate(VDS vds, StringBuilder sb) { + public VdcBllMessages validate(VDS vds, StringBuilder sb, boolean isMigrate) { // if vm has more vCpus then vds physical cpus - dont allow to run if (!isVMSwapValueLegal(vds)) { sb.append("swap value is illegal"); @@ -326,7 +305,7 @@ add(new HostValidator() { @Override - public VdcBllMessages validate(VDS vds, StringBuilder sb) { + public VdcBllMessages validate(VDS vds, StringBuilder sb, boolean isMigrate) { if (!areRequiredNetworksAvailable(vds)) { sb.append("is missing networks required by VM nics ").append(Entities.interfacesByNetworkName(getVmNICs()) .keySet()); @@ -338,7 +317,7 @@ add(new HostValidator() { @Override - public VdcBllMessages validate(VDS vds, StringBuilder sb) { + public VdcBllMessages validate(VDS vds, StringBuilder sb, boolean isMigrate) { if (isVdsFailedToRunVm(vds.getId())) { sb.append("have failed running this VM in the current selection cycle"); return VdcBllMessages.ACTION_TYPE_FAILED_VDS_VM_CLUSTER; @@ -349,12 +328,12 @@ } }); - private ValidationResult validateHostIsReadyToRun(final VDS vds, StringBuilder sb) { + private ValidationResult validateHostIsReadyToRun(final VDS vds, StringBuilder sb, boolean isMigrate) { // buffer the mismatches as we go sb.append(" VDS ").append(vds.getvds_name()).append(" ").append(vds.getId()).append(" "); for(HostValidator validator : this.hostValidators) { - VdcBllMessages result = validator.validate(vds, sb); + VdcBllMessages result = validator.validate(vds, sb, isMigrate); if(result != null) { return new ValidationResult(result); } @@ -405,11 +384,11 @@ .<Integer> GetValue(ConfigValues.BlockMigrationOnSwapUsagePercentage); } - private Guid getVdsToRunOn(Iterable<VDS> vdss) { + private Guid getVdsToRunOn(Iterable<VDS> vdss, boolean isMigrate) { StringBuilder sb = new StringBuilder(); final List<VDS> readyToRun = new ArrayList<VDS>(); for (VDS curVds : vdss) { - if (validateHostIsReadyToRun(curVds,sb) == ValidationResult.VALID) { + if (validateHostIsReadyToRun(curVds, sb, isMigrate) == ValidationResult.VALID) { readyToRun.add(curVds); } } -- To view, visit http://gerrit.ovirt.org/8994 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I8cca521f79bfb7d5762f2c996200916a10afc36a Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Tomas Jelinek <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
