Arik Hadas has uploaded a new change for review. Change subject: core: block live migration during live storage migration ......................................................................
core: block live migration during live storage migration This is a best-effort fix for preventing a VM from being migrated during migration of one of its disks (live storage migration), and vice versa. One way is already covered - live storage migration is made only when the VM state is up or paused so it can't be executed when the VM is in migration. This patch is partial fix for the opposite way - a check is added to MigrateVmCommand to verify that none of the VM's disks is locked, which is the case when one of its disks is being migrated (or during snapshot creation for the disk). This fix doesn't cover a situation where the VM migration and storage migration/snapshot creation are taking place simultanously, and not cover the case where the VM migration fails thus being rerun and the VM state is switched to 'UP' in the middle of the process. Change-Id: Id630a01368290ba9ac6c71c5cab2095f84fa018b Bug-Url: https://bugzilla.redhat.com/878131 Signed-off-by: Arik Hadas <aha...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java 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 M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveDisksCommand.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java 5 files changed, 18 insertions(+), 15 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/22/14122/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java index 774471c..74f6f86 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImagesHandler.java @@ -467,6 +467,10 @@ return returnValue; } + public static List<DiskImage> getPluggedImagesForVm(Guid vmId) { + return filterImageDisks(DbFacade.getInstance().getDiskDao().getAllForVm(vmId, true), true, false); + } + private static boolean checkDiskImages(List<String> messages, Guid storagePoolId, boolean checkImagesIllegalInVdsm, 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 9ce0972..54fb9e9 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 @@ -30,7 +30,7 @@ * the internal migration command should fail */ @Override - protected boolean canMigrateVm(Guid vmGuid, java.util.ArrayList<String> reasons) { + protected boolean canMigrateVm(Guid vmGuid, java.util.List<String> reasons) { if (getVm().getMigrationSupport() == MigrationSupport.MIGRATABLE) { return super.canMigrateVm(vmGuid, reasons); } 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 2c7035d..5c71733 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 @@ -1,10 +1,10 @@ package org.ovirt.engine.core.bll; -import java.util.ArrayList; import java.util.List; import org.ovirt.engine.core.bll.job.ExecutionHandler; import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; +import org.ovirt.engine.core.bll.validator.DiskImagesValidator; import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.FeatureSupported; import org.ovirt.engine.core.common.action.MigrateVmParameters; @@ -216,7 +216,7 @@ return canMigrateVm(getVmId(), getReturnValue().getCanDoActionMessages()); } - protected boolean canMigrateVm(@SuppressWarnings("unused") Guid vmGuid, ArrayList<String> reasons) { + protected boolean canMigrateVm(@SuppressWarnings("unused") Guid vmGuid, List<String> reasons) { boolean retValue = true; VM vm = getVm(); if (vm == null) { @@ -250,7 +250,11 @@ reasons.add(VdcBllMessages.ACTION_TYPE_FAILED_VDS_STATUS_ILLEGAL.name()); } - retValue = retValue && validate(new SnapshotsValidator().vmNotDuringSnapshot(vm.getId())) + retValue = retValue + // 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 SnapshotsValidator().vmNotDuringSnapshot(vm.getId())) && getVdsSelector().canFindVdsToRunOn(reasons, true); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveDisksCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveDisksCommand.java index 2215e60..f0a38a4 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveDisksCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveDisksCommand.java @@ -19,7 +19,6 @@ import org.ovirt.engine.core.common.businessentities.ActionGroup; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.VM; -import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.VdcBllMessages; import org.ovirt.engine.core.dao.DiskImageDAO; @@ -100,10 +99,10 @@ return false; } - if (vm == null || isVmDown(vm)) { + if (vm == null || vm.isDown()) { moveParametersList.add(moveDiskParameters); } - else if (isVmRunning(vm)) { + else if (vm.isRunningAndQualifyForDisksMigration()) { MultiValueMapUtils.addToMap(vm.getId(), createLiveMigrateDiskParameters(moveDiskParameters, vm.getId()), vmsLiveMigrateParametersMap); @@ -115,14 +114,6 @@ } return true; - } - - private boolean isVmRunning(VM vm) { - return vm.getStatus().isUpOrPaused() && vm.getRunOnVds() != null && !vm.getRunOnVds().equals(Guid.Empty); - } - - private boolean isVmDown(VM vm) { - return vm.getStatus() == VMStatus.Down; } private LiveMigrateDiskParameters createLiveMigrateDiskParameters(MoveDiskParameters moveDiskParameters, Guid vmId) { diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java index 32bc728..877867c 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java @@ -1417,6 +1417,10 @@ return getStatus().isQualifyToMigrate(); } + public boolean isRunningAndQualifyForDisksMigration() { + return getStatus().isUpOrPaused() && getRunOnVds() != null && !getRunOnVds().equals(Guid.Empty); + } + public boolean isNotRunning() { return getStatus().isNotRunning(); } -- To view, visit http://gerrit.ovirt.org/14122 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id630a01368290ba9ac6c71c5cab2095f84fa018b 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
