Arik Hadas has uploaded a new change for review.
Change subject: core: cleanup in MigrateVmCommand
......................................................................
core: cleanup in MigrateVmCommand
Extract the code in MigrateVmCommand#perform method which is related
only to the parameters initialization to separate methods, to make it
shorter and easier to read.
This patch also includes other minor refactoring in this class.
Change-Id: I138ac8c64e7c0fe70c579e80313b56d4d1d47848
Signed-off-by: Arik Hadas <aha...@redhat.com>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java
1 file changed, 42 insertions(+), 52 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/67/18467/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 b58b2fe..685f0a2 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
@@ -31,12 +31,10 @@
public class MigrateVmCommand<T extends MigrateVmParameters> extends
RunVmCommandBase<T> {
- private Guid _vdsDestinationId;
+ private Guid vdsDestinationId;
protected boolean forcedMigrationForNonMigratableVM;
- /**
- * Used to log the migration error.
- */
+ /** Used to log the migration error. */
private VdcBllErrors migrationErrorCode;
public MigrateVmCommand(T parameters) {
@@ -44,13 +42,11 @@
forcedMigrationForNonMigratableVM =
parameters.isForceMigrationForNonMigratableVM();
}
- // this property is used for audit log events
+ /**
+ * this property is used for audit log events
+ */
public String getVdsDestination() {
- if (getDestinationVds() != null) {
- return getDestinationVds().getName();
- } else {
- return null;
- }
+ return getDestinationVds() != null ? getDestinationVds().getName() :
null;
}
/**
@@ -68,8 +64,8 @@
@Override
protected VDS getDestinationVds() {
- if (_destinationVds == null && _vdsDestinationId != null) {
- _destinationVds =
DbFacade.getInstance().getVdsDao().get(_vdsDestinationId);
+ if (_destinationVds == null && vdsDestinationId != null) {
+ _destinationVds =
DbFacade.getInstance().getVdsDao().get(vdsDestinationId);
}
return _destinationVds;
}
@@ -82,7 +78,7 @@
}
protected void initVdss() {
- setVdsIdRef(new Guid(getVm().getRunOnVds().toString()));
+ setVdsIdRef(getVm().getRunOnVds());
VDS destVds = getDestinationVds();
Guid vdsToRunOn =
SchedulingManager.getInstance().schedule(getVdsGroup(),
@@ -100,14 +96,11 @@
// make _destinationVds null in order to refresh it from db in case it
// changed.
_destinationVds = null;
- if (_vdsDestinationId != null && _vdsDestinationId.equals(Guid.Empty))
{
+ if (vdsDestinationId != null && vdsDestinationId.equals(Guid.Empty)) {
throw new
VdcBLLException(VdcBllErrors.RESOURCE_MANAGER_CANT_ALLOC_VDS_MIGRATION);
}
- if (getDestinationVds() == null) {
- throw new
VdcBLLException(VdcBllErrors.RESOURCE_MANAGER_VDS_NOT_FOUND);
- }
- if (getVds() == null) {
+ if (getDestinationVds() == null || getVds() == null) {
throw new
VdcBLLException(VdcBllErrors.RESOURCE_MANAGER_VDS_NOT_FOUND);
}
}
@@ -127,42 +120,50 @@
}
private void perform() {
- getVm().setMigratingToVds(_vdsDestinationId);
-
- String srcVdsHost = getVds().getHostName();
- String dstVdsHost = String.format("%1$s:%2$s",
getDestinationVds().getHostName(), getDestinationVds()
- .getPort());
- Boolean tunnelMigration = null;
- if
(FeatureSupported.tunnelMigration(getVm().getVdsGroupCompatibilityVersion())) {
- // if vm has no override for tunnel migration (its null),
- // use cluster's setting
- tunnelMigration =
- getVm().getTunnelMigration() != null ?
getVm().getTunnelMigration()
- : getVdsGroup().isTunnelMigration();
- }
+ getVm().setMigratingToVds(vdsDestinationId);
getParameters().setStartTime(new Date());
// Starting migration at src VDS
- boolean connectToLunDiskSuccess = connectLunDisks(_vdsDestinationId);
+ boolean connectToLunDiskSuccess = connectLunDisks(vdsDestinationId);
if (connectToLunDiskSuccess) {
setActionReturnValue(Backend
.getInstance()
.getResourceManager()
.RunAsyncVdsCommand(
VDSCommandType.Migrate,
- new MigrateVDSCommandParameters(getVdsId(),
getVmId(), srcVdsHost, _vdsDestinationId,
- dstVdsHost, MigrationMethod.ONLINE,
tunnelMigration, getMigrationNetworkIp()),
+ getMigrateVDSCommandParameters(),
this)
.getReturnValue());
}
- if (!connectToLunDiskSuccess || (VMStatus) getActionReturnValue() !=
VMStatus.MigratingFrom) {
+ if (!connectToLunDiskSuccess || getActionReturnValue() !=
VMStatus.MigratingFrom) {
getVm().setMigreatingToPort(0);
getVm().setMigreatingFromPort(0);
getVm().setMigratingToVds(null);
throw new
VdcBLLException(VdcBllErrors.RESOURCE_MANAGER_MIGRATION_FAILED_AT_DST);
}
ExecutionHandler.setAsyncJob(getExecutionContext(), true);
+ }
+
+ private MigrateVDSCommandParameters getMigrateVDSCommandParameters() {
+ String srcVdsHost = getVds().getHostName();
+ String dstVdsHost = String.format("%1$s:%2$s",
+ getDestinationVds().getHostName(),
+ getDestinationVds().getPort());
+
+ return new MigrateVDSCommandParameters(getVdsId(), getVmId(),
srcVdsHost, vdsDestinationId,
+ dstVdsHost, MigrationMethod.ONLINE, isTunnelMigrationUsed(),
getMigrationNetworkIp());
+ }
+
+ private Boolean isTunnelMigrationUsed() {
+ if
(FeatureSupported.tunnelMigration(getVm().getVdsGroupCompatibilityVersion())) {
+ // if vm has no override for tunnel migration (its null),
+ // use cluster's setting
+ return getVm().getTunnelMigration() != null ?
+ getVm().getTunnelMigration()
+ : getVdsGroup().isTunnelMigration();
+ }
+ return null;
}
private String getMigrationNetworkIp() {
@@ -221,11 +222,11 @@
}
protected Guid getVdsDestinationId() {
- return _vdsDestinationId;
+ return vdsDestinationId;
}
protected void setVdsDestinationId(Guid value) {
- _vdsDestinationId = value;
+ vdsDestinationId = value;
}
@Override
@@ -293,22 +294,15 @@
@Override
public void rerun() {
- /**
- * make Vm property to null in order to refresh it from db.
- */
+ // make Vm property to null in order to refresh it from db
setVm(null);
determineMigrationFailureForAuditLog();
- /**
- * if vm is up and rerun is called then it got up on the source, try to
- * rerun
- */
+ // 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) {
setVdsDestinationId(null);
super.rerun();
} else {
- /**
- * vm went down on the destination and source, migration failed.
- */
+ // vm went down on the destination and source, migration failed.
decreasePendingVms(getDestinationVds().getId());
_isRerun = true;
setSucceeded(false);
@@ -332,11 +326,7 @@
@Override
protected Guid getCurrentVdsId() {
- if (getVdsDestinationId() != null) {
- return getVdsDestinationId();
- } else {
- return super.getCurrentVdsId();
- }
+ return getVdsDestinationId() != null ? getVdsDestinationId() :
super.getCurrentVdsId();
}
public String getDuration() {
--
To view, visit http://gerrit.ovirt.org/18467
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I138ac8c64e7c0fe70c579e80313b56d4d1d47848
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
