Arik Hadas has uploaded a new change for review. Change subject: core: remove RunVmCommandBase#getDestinationVds ......................................................................
core: remove RunVmCommandBase#getDestinationVds In light of 84fc3c7 RunVmCommandBase#getDestinationVds is now used only by classes which extend RunVmCommandBase and is used differently: MigrateVm returns the VDS that the VM is going to migrate to, RunVm & RunVmOnce return the predefined VDS on which the VM should run on. This patch removes RunVmCommandBase#getDestinationVds method. In RunVm it is replaced by getPredefinedDestinationVdsId method which reflect the result better by its name and returns only the ID of the VDS. In addition, minor improvement was made in MigrateVmCommand#getDestinationVds, and calls to it where we only need the ID of the VDS were replaced by calls to MigrateVmCommand#getDestinationVdsId. Change-Id: Ie1d8fb852a88203378b3bcd3f1d6f96e469e5cb9 Signed-off-by: Arik Hadas <aha...@redhat.com> --- 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/RunVmOnceCommand.java 4 files changed, 24 insertions(+), 39 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/55/24255/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 dc59923..b6344b9 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 @@ -69,11 +69,15 @@ .TranslateErrorTextSingle(migrationErrorCode.name(), true); } - @Override protected VDS getDestinationVds() { - if (cachedDestinationVds == null && destinationVdsId != null) { + if (destinationVdsId == null) { + return cachedDestinationVds = null; + } + + if (cachedDestinationVds == null || !cachedDestinationVds.getId().equals(destinationVdsId)) { cachedDestinationVds = getVdsDAO().get(destinationVdsId); } + return cachedDestinationVds; } @@ -101,13 +105,12 @@ protected void initVdss() { setVdsIdRef(getVm().getRunOnVds()); - VDS destVds = getDestinationVds(); Guid vdsToRunOn = SchedulingManager.getInstance().schedule(getVdsGroup(), getVm(), getVdsBlackList(), getVdsWhiteList(), - destVds == null ? null : destVds.getId(), + getDestinationVdsId(), new ArrayList<String>(), new VdsFreeMemoryChecker(this), getCorrelationId()); @@ -116,9 +119,7 @@ getRunVdssList().add(vdsToRunOn); } VmHandler.updateVmGuestAgentVersion(getVm()); - // make _destinationVds null in order to refresh it from db in case it - // changed. - cachedDestinationVds = null; + if (destinationVdsId != null && destinationVdsId.equals(Guid.Empty)) { throw new VdcBLLException(VdcBllErrors.RESOURCE_MANAGER_CANT_ALLOC_VDS_MIGRATION); } 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 883e67d..75bfa46 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 @@ -94,7 +94,6 @@ /** This flag is used to indicate that the disks might be dirty since the memory * from the active snapshot was restored so the memory should not be used */ private boolean memoryFromSnapshotIrrelevant; - private VDS cachedDestinationVds; public static final String ISO_PREFIX = "iso://"; public static final String STATELESS_SNAPSHOT_DESCRIPTION = "stateless snapshot"; @@ -112,25 +111,8 @@ initRunVmCommand(); } - @Override - protected VDS getDestinationVds() { - if (cachedDestinationVds == null) { - Guid vdsId = null; - - if (getVm().getDedicatedVmForVds() != null) { - vdsId = getVm().getDedicatedVmForVds(); - } - - // destination VDS ID is stronger than the dedicated VDS - if (getParameters().getDestinationVdsId() != null) { - vdsId = getParameters().getDestinationVdsId(); - } - - if (vdsId != null) { - cachedDestinationVds = getVdsDAO().get(vdsId); - } - } - return cachedDestinationVds; + protected Guid getPredefinedDestinationVdsId() { + return getVm().getDedicatedVmForVds(); } private void initRunVmCommand() { @@ -624,14 +606,12 @@ } protected boolean getVdsToRunOn() { - // use destination vds or default vds or none - VDS destinationVds = getDestinationVds(); Guid vdsToRunOn = SchedulingManager.getInstance().schedule(getVdsGroup(), getVm(), getRunVdssList(), getVdsWhiteList(), - destinationVds == null ? null : destinationVds.getId(), + getPredefinedDestinationVdsId(), new ArrayList<String>(), new VdsFreeMemoryChecker(this), getCorrelationId()); @@ -754,7 +734,7 @@ getStoragePool(), getRunVdssList(), getVdsWhiteList(), - getDestinationVds() != null ? getDestinationVds().getId() : null, + getPredefinedDestinationVdsId(), getVdsGroup())) { return false; } 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 9dff5bd..58a47a6 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 @@ -25,7 +25,6 @@ import org.ovirt.engine.core.common.businessentities.LUNs; import org.ovirt.engine.core.common.businessentities.LunDisk; import org.ovirt.engine.core.common.businessentities.StorageServerConnections; -import org.ovirt.engine.core.common.businessentities.VDS; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.common.businessentities.VmStatic; @@ -66,8 +65,6 @@ public RunVmCommandBase(T parameters) { super(parameters); } - - protected abstract VDS getDestinationVds(); public SnapshotsValidator getSnapshotsValidator() { return snapshotsValidator; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java index 0d3c32d..effb235 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java @@ -47,6 +47,14 @@ return true; } + @Override + protected Guid getPredefinedDestinationVdsId() { + // destination VDS ID is stronger than the dedicated VDS + return getParameters().getDestinationVdsId() != null ? + getParameters().getDestinationVdsId() + : super.getPredefinedDestinationVdsId(); + } + /** * Refresh the associated values of the VM boot parameters with the values from the command parameters. The method * is used when VM is reloaded from the DB while its parameters hasn't been persisted (e.g. when running 'as once') @@ -112,11 +120,10 @@ @Override protected List<Guid> getVdsWhiteList() { - VDS destinationVds = getDestinationVds(); - if (destinationVds != null) { - return Arrays.asList(destinationVds.getId()); - } - return super.getVdsWhiteList(); + Guid predefinedDestinationVdsId = getPredefinedDestinationVdsId(); + return predefinedDestinationVdsId != null ? + Arrays.asList(predefinedDestinationVdsId) + : super.getVdsWhiteList(); } @Override -- To view, visit http://gerrit.ovirt.org/24255 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie1d8fb852a88203378b3bcd3f1d6f96e469e5cb9 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches