Arik Hadas has uploaded a new change for review. Change subject: core: [cleanup] organize canMigrateVm method ......................................................................
core: [cleanup] organize canMigrateVm method This patch organize MigrateVmCommand#canMigrateVm method: 1. replaced the unused paramter vmGuid with VM instance. that way, the method is more testable (can be tested without mocking, just insert a different VM to check as parameter) 2. remove the can-do-action error messages list from the parameters, and use addCanDoActionMessage & failCanDoAction methods inside the method instead 3. set the action and type for the can-do-action messages in the standard way by override setActionMessageParameters method Change-Id: Ica36ecc272a295ee82ec51d7b62a90d84489d8c9 Signed-off-by: Arik Hadas <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InternalMigrateVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MigrateVmCommand.java 2 files changed, 44 insertions(+), 42 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/25/14125/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InternalMigrateVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InternalMigrateVmCommand.java index 54fb9e9..3441892 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InternalMigrateVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InternalMigrateVmCommand.java @@ -3,7 +3,7 @@ import org.ovirt.engine.core.common.action.InternalMigrateVmParameters; import org.ovirt.engine.core.common.action.MigrateVmParameters; import org.ovirt.engine.core.common.businessentities.MigrationSupport; -import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.dal.VdcBllMessages; @InternalCommandAttribute @@ -30,9 +30,9 @@ * the internal migration command should fail */ @Override - protected boolean canMigrateVm(Guid vmGuid, java.util.List<String> reasons) { + protected boolean canMigrateVm(VM vm) { if (getVm().getMigrationSupport() == MigrationSupport.MIGRATABLE) { - return super.canMigrateVm(vmGuid, reasons); + return super.canMigrateVm(vm); } else { return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NON_MIGRTABLE); 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 5c71733..0ebaa04 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 @@ -213,56 +213,58 @@ @Override protected boolean canDoAction() { - return canMigrateVm(getVmId(), getReturnValue().getCanDoActionMessages()); + return canMigrateVm(getVm()); } - protected boolean canMigrateVm(@SuppressWarnings("unused") Guid vmGuid, List<String> reasons) { - boolean retValue = true; - VM vm = getVm(); + @Override + protected void setActionMessageParameters() { + addCanDoActionMessage(VdcBllMessages.VAR__ACTION__MIGRATE); + addCanDoActionMessage(VdcBllMessages.VAR__TYPE__VM); + } + + protected boolean canMigrateVm(VM vm) { if (vm == null) { - retValue = false; - reasons.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_FOUND.name()); - } else { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_FOUND); + } + else { // If VM is pinned to host, no migration can occur if (vm.getMigrationSupport() == MigrationSupport.PINNED_TO_HOST) { - retValue = false; - reasons.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_PINNED_TO_HOST.name()); - } else if (vm.getMigrationSupport() == MigrationSupport.IMPLICITLY_NON_MIGRATABLE - && !forcedMigrationForNonMigratableVM) { - retValue = false; - reasons.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NON_MIGRTABLE_AND_IS_NOT_FORCED_BY_USER_TO_MIGRATE - .toString()); - } else if (vm.getStatus() == VMStatus.MigratingFrom) { - retValue = false; - reasons.add(VdcBllMessages.ACTION_TYPE_FAILED_MIGRATION_IN_PROGRESS.name()); - } else if (vm.getStatus() == VMStatus.NotResponding) { - retValue = false; - reasons.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_STATUS_ILLEGAL.name()); - } else if (vm.getStatus() == VMStatus.Paused) { - retValue = false; - reasons.add(VdcBllMessages.MIGRATE_PAUSED_VM_IS_UNSUPPORTED.name()); - } else if (!vm.isQualifyToMigrate()) { - retValue = false; - reasons.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_RUNNING.name()); - } else if (getDestinationVds() != null && getDestinationVds().getStatus() != VDSStatus.Up) { - retValue = false; - reasons.add(VdcBllMessages.VAR__HOST_STATUS__UP.name()); - reasons.add(VdcBllMessages.ACTION_TYPE_FAILED_VDS_STATUS_ILLEGAL.name()); + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_PINNED_TO_HOST); } - retValue = retValue + if (vm.getMigrationSupport() == MigrationSupport.IMPLICITLY_NON_MIGRATABLE + && !forcedMigrationForNonMigratableVM) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NON_MIGRTABLE_AND_IS_NOT_FORCED_BY_USER_TO_MIGRATE); + } + + if (vm.getStatus() == VMStatus.MigratingFrom) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_MIGRATION_IN_PROGRESS); + } + + if (vm.getStatus() == VMStatus.NotResponding) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_STATUS_ILLEGAL); + } + + if (vm.getStatus() == VMStatus.Paused) { + return failCanDoAction(VdcBllMessages.MIGRATE_PAUSED_VM_IS_UNSUPPORTED); + } + + if (!vm.isQualifyToMigrate()) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_RUNNING); + } + + if (getDestinationVds() != null && getDestinationVds().getStatus() != VDSStatus.Up) { + addCanDoActionMessage(VdcBllMessages.VAR__HOST_STATUS__UP); + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VDS_STATUS_ILLEGAL); + } + + return // This check was added to prevent migration of VM while its disks are being // migrated or snapshot is taken for them. TODO: replace it with a better solution - && validate(new DiskImagesValidator(ImagesHandler.getPluggedImagesForVm(vm.getId())).diskImagesNotLocked()) + validate(new DiskImagesValidator(ImagesHandler.getPluggedImagesForVm(vm.getId())).diskImagesNotLocked()) && validate(new SnapshotsValidator().vmNotDuringSnapshot(vm.getId())) - && getVdsSelector().canFindVdsToRunOn(reasons, true); + && getVdsSelector().canFindVdsToRunOn(getReturnValue().getCanDoActionMessages(), true); } - - if (!retValue) { - reasons.add(VdcBllMessages.VAR__ACTION__MIGRATE.toString()); - reasons.add(VdcBllMessages.VAR__TYPE__VM.toString()); - } - return retValue; } @Override -- To view, visit http://gerrit.ovirt.org/14125 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ica36ecc272a295ee82ec51d7b62a90d84489d8c9 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
