Arik Hadas has uploaded a new change for review. Change subject: core: minor cleanups ......................................................................
core: minor cleanups * Remove _destinationVds from RunVmCommandBase and put relevant fields in RunVmCommand & MigrateVmCommand. It is better in terms of encapsulation since they are now private * Refactor RunVmCommand#getDestinationVds to be more readable * Change the visibility of forcedMigrationForNonMigratableVM in MigrateVmCommand to private * Few renames Change-Id: I2fa22569d9195f69cec7fa9c0a4931b2975e41ec Signed-off-by: Arik Hadas <[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/MigrateVmToServerCommand.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 4 files changed, 42 insertions(+), 35 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/60/24160/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 2af93be..1b28042 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 @@ -38,15 +38,16 @@ @LockIdNameAttribute(isReleaseAtEndOfExecute = false) public class MigrateVmCommand<T extends MigrateVmParameters> extends RunVmCommandBase<T> { - private Guid vdsDestinationId; - protected boolean forcedMigrationForNonMigratableVM; + private Guid destinationVdsId; + private VDS cachedDestinationVds; + private boolean forcedMigrationForNonMigratableVm; /** Used to log the migration error. */ private VdcBllErrors migrationErrorCode; public MigrateVmCommand(T parameters) { super(parameters); - forcedMigrationForNonMigratableVM = parameters.isForceMigrationForNonMigratableVM(); + forcedMigrationForNonMigratableVm = parameters.isForceMigrationForNonMigratableVM(); } /** @@ -71,10 +72,10 @@ @Override protected VDS getDestinationVds() { - if (_destinationVds == null && vdsDestinationId != null) { - _destinationVds = DbFacade.getInstance().getVdsDao().get(vdsDestinationId); + if (cachedDestinationVds == null && destinationVdsId != null) { + cachedDestinationVds = DbFacade.getInstance().getVdsDao().get(destinationVdsId); } - return _destinationVds; + return cachedDestinationVds; } @Override @@ -108,15 +109,15 @@ new ArrayList<String>(), new VdsFreeMemoryChecker(this), getCorrelationId()); - setVdsDestinationId(vdsToRunOn); + setDestinationVdsId(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. - _destinationVds = null; - if (vdsDestinationId != null && vdsDestinationId.equals(Guid.Empty)) { + cachedDestinationVds = null; + if (destinationVdsId != null && destinationVdsId.equals(Guid.Empty)) { throw new VdcBLLException(VdcBllErrors.RESOURCE_MANAGER_CANT_ALLOC_VDS_MIGRATION); } @@ -133,12 +134,12 @@ } private void perform() { - getVm().setMigratingToVds(vdsDestinationId); + getVm().setMigratingToVds(destinationVdsId); getParameters().setStartTime(new Date()); // Starting migration at src VDS - boolean connectToLunDiskSuccess = connectLunDisks(vdsDestinationId); + boolean connectToLunDiskSuccess = connectLunDisks(destinationVdsId); if (connectToLunDiskSuccess) { setActionReturnValue(Backend .getInstance() @@ -164,7 +165,7 @@ getDestinationVds().getHostName(), getDestinationVds().getPort()); - return new MigrateVDSCommandParameters(getVdsId(), getVmId(), srcVdsHost, vdsDestinationId, + return new MigrateVDSCommandParameters(getVdsId(), getVmId(), srcVdsHost, destinationVdsId, dstVdsHost, MigrationMethod.ONLINE, isTunnelMigrationUsed(), getMigrationNetworkIp(), getVds().getVdsGroupCompatibilityVersion(), getMaximumMigrationDowntime()); } @@ -282,12 +283,12 @@ return AuditLogType.VM_MIGRATION_FAILED; } - protected Guid getVdsDestinationId() { - return vdsDestinationId; + protected Guid getDestinationVdsId() { + return destinationVdsId; } - protected void setVdsDestinationId(Guid value) { - vdsDestinationId = value; + protected void setDestinationVdsId(Guid value) { + destinationVdsId = value; } @Override @@ -308,7 +309,7 @@ } if (vm.getMigrationSupport() == MigrationSupport.IMPLICITLY_NON_MIGRATABLE - && !forcedMigrationForNonMigratableVM) { + && !forcedMigrationForNonMigratableVm) { return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NON_MIGRTABLE_AND_IS_NOT_FORCED_BY_USER_TO_MIGRATE); } @@ -347,7 +348,7 @@ getVm(), getVdsBlackList(), getParameters().getInitialHosts(), - getVdsDestinationId(), + getDestinationVdsId(), getReturnValue().getCanDoActionMessages()); } @@ -365,7 +366,7 @@ // if vm is up and rerun is called then it got up on the source, try to rerun if (getVm() != null && getVm().getStatus() == VMStatus.Up) { determineMigrationFailureForAuditLog(); - setVdsDestinationId(null); + setDestinationVdsId(null); super.rerun(); } else { // vm went down on the destination and source, migration failed. @@ -394,8 +395,8 @@ finally { // Decrement the pending counters here as this is the only place which // is called consistently regardless of the migration result. - if (getVdsDestinationId() != null) { - decreasePendingVms(getVdsDestinationId()); + if (getDestinationVdsId() != null) { + decreasePendingVms(getDestinationVdsId()); } freeLock(); @@ -418,7 +419,7 @@ @Override protected Guid getCurrentVdsId() { - return getVdsDestinationId() != null ? getVdsDestinationId() : super.getCurrentVdsId(); + return getDestinationVdsId() != null ? getDestinationVdsId() : super.getCurrentVdsId(); } public String getDuration() { diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java index d5e3601..e0d461c 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmToServerCommand.java @@ -13,12 +13,12 @@ public class MigrateVmToServerCommand<T extends MigrateVmToServerParameters> extends MigrateVmCommand<T> { public MigrateVmToServerCommand(T parameters) { super(parameters); - setVdsDestinationId(parameters.getVdsId()); + setDestinationVdsId(parameters.getVdsId()); } @Override protected boolean canDoAction() { - Guid destinationId = getVdsDestinationId(); + Guid destinationId = getDestinationVdsId(); VDS vds = getVdsDAO().get(destinationId); if (vds == null) { return failCanDoAction(VdcBllMessages.VDS_INVALID_SERVER_ID); @@ -52,6 +52,6 @@ @Override protected List<Guid> getVdsWhiteList() { - return Arrays.asList(getVdsDestinationId()); + return Arrays.asList(getDestinationVdsId()); } } 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 f27cde0..0053ba7 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,6 +94,7 @@ /** 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"; @@ -111,17 +112,23 @@ @Override protected VDS getDestinationVds() { - if (_destinationVds == null) { - Guid vdsId = - getParameters().getDestinationVdsId() != null ? getParameters().getDestinationVdsId() - : getVm().getDedicatedVmForVds() != null ? new Guid(getVm().getDedicatedVmForVds() - .toString()) - : null; + 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) { - _destinationVds = getVdsDAO().get(vdsId); + cachedDestinationVds = getVdsDAO().get(vdsId); } } - return _destinationVds; + return cachedDestinationVds; } private void initRunVmCommand() { 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 27e10ab..18168e7 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 @@ -54,8 +54,7 @@ IVdsAsyncCommand, RunVmDelayer { private static Log log = LogFactory.getLog(RunVmCommandBase.class); - protected boolean _isRerun = false; - protected VDS _destinationVds; + protected boolean _isRerun; private SnapshotsValidator snapshotsValidator=new SnapshotsValidator(); private final List<Guid> runVdsList = new ArrayList<Guid>(); -- To view, visit http://gerrit.ovirt.org/24160 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I2fa22569d9195f69cec7fa9c0a4931b2975e41ec Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
