Daniel Erez has uploaded a new change for review. Change subject: core: move unplugged disks while VM is running ......................................................................
core: move unplugged disks while VM is running Refactor/rewrite of MoveDisksCommand to support the following: * Offline move of unplugged disks while the VM is running. * Add error messages when trying to move/migrate a mixture of plugged and unplugged disks (on a running VM). Change-Id: Ie0fbfd95c0086cfe0d792b3071a2d804ff32c18e Bug-Url: https://bugzilla.redhat.com/987783 Bug-Url: https://bugzilla.redhat.com/957494 Bug-Url: https://bugzilla.redhat.com/957492 Signed-off-by: Daniel Erez <de...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveDisksCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveDisksCommandTest.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommandTest.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 9 files changed, 257 insertions(+), 111 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/05/19105/1 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 fb97e9f..903e27c 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 @@ -2,10 +2,11 @@ import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedList; import java.util.List; import java.util.Map; -import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; +import org.apache.commons.lang.StringUtils; import org.ovirt.engine.core.bll.utils.PermissionSubject; import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.LiveMigrateDiskParameters; @@ -27,9 +28,9 @@ public class MoveDisksCommand<T extends MoveDisksParameters> extends CommandBase<T> { private List<VdcReturnValueBase> vdcReturnValues = new ArrayList<VdcReturnValueBase>(); - private List<MoveDiskParameters> moveParametersList = new ArrayList<MoveDiskParameters>(); - private Map<Guid, List<LiveMigrateDiskParameters>> vmsLiveMigrateParametersMap = - new HashMap<Guid, List<LiveMigrateDiskParameters>>(); + private List<MoveDiskParameters> moveDiskParametersList = new ArrayList<>(); + private List<LiveMigrateVmDisksParameters> liveMigrateVmDisksParametersList = new ArrayList<>(); + private Map<Guid, DiskImage> diskMap = new HashMap<>(); public MoveDisksCommand(Guid commandId) { super(commandId); @@ -41,14 +42,16 @@ @Override protected void executeCommand() { - if (!moveParametersList.isEmpty()) { + updateParameters(); + + if (!moveDiskParametersList.isEmpty()) { vdcReturnValues.addAll(Backend.getInstance().RunMultipleActions(VdcActionType.MoveOrCopyDisk, - getMoveDisksParametersList(), false)); + getParametersArrayList(moveDiskParametersList), false)); } - if (!vmsLiveMigrateParametersMap.isEmpty()) { + if (!liveMigrateVmDisksParametersList.isEmpty()) { vdcReturnValues.addAll(Backend.getInstance().RunMultipleActions(VdcActionType.LiveMigrateVmDisks, - getLiveMigrateDisksParametersList(), false)); + getParametersArrayList(liveMigrateVmDisksParametersList), false)); } handleChildReturnValue(); @@ -68,7 +71,7 @@ return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NO_DISKS_SPECIFIED); } - return updateParameters(); + return true; } @Override @@ -77,41 +80,88 @@ addCanDoActionMessage(VdcBllMessages.VAR__TYPE__VM_DISK); } + private void addDisksDeactivatedMessage(List<MoveDiskParameters> moveDiskParamsList) { + setActionMessageParameters(); + addCanDoActionMessage(String.format("$%1$s %2$s", "diskAliases", + StringUtils.join(getUnpluggedDisksAliases(moveDiskParamsList), ", "))); + addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_DEACTIVATED); + getReturnValue().setCanDoAction(false); + } + + private void addInvalidVmStateMessage(VM vm){ + setActionMessageParameters(); + addCanDoActionMessage(String.format("$%1$s %2$s", "VmName", vm.getName())); + addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_DOWN_OR_UP); + getReturnValue().setCanDoAction(false); + } + /** * For each specified MoveDiskParameters, decide whether it should be moved - * using 'regular' move or using live migrate command. - * - * @return false if the command should fail on canDoAction; otherwise, true + * using offline move or live migrate command. */ - private boolean updateParameters() { - for (MoveDiskParameters moveDiskParameters : getParameters().getParametersList()) { - DiskImage diskImage = getDiskImageDao().get(moveDiskParameters.getImageId()); - if (diskImage == null) { - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_DISK_NOT_EXIST); - } + protected void updateParameters() { + Map<VM, List<MoveDiskParameters>> vmDiskParamsMap = createVmDiskParamsMap(); - List<VM> allVms = getVmDAO().getVmsListForDisk(diskImage.getId()); - VM vm = !allVms.isEmpty() ? allVms.get(0) : null; + for (Map.Entry<VM, List<MoveDiskParameters>> vmDiskParamsEntry : vmDiskParamsMap.entrySet()) { + VM vm = vmDiskParamsEntry.getKey(); + List<MoveDiskParameters> moveDiskParamsList = vmDiskParamsEntry.getValue(); - if (vm != null && !validate(createSnapshotsValidator().vmNotInPreview(vm.getId()))) { - return false; - } - - if (vm == null || vm.isDown()) { - moveParametersList.add(moveDiskParameters); + if (vm == null || vm.isDown() || isAllDisksUnplugged(moveDiskParamsList)) { + // Adding parameters for offline move + moveDiskParametersList.addAll(moveDiskParamsList); } else if (vm.isRunningAndQualifyForDisksMigration()) { - MultiValueMapUtils.addToMap(vm.getId(), - createLiveMigrateDiskParameters(moveDiskParameters, vm.getId()), - vmsLiveMigrateParametersMap); + if (!isAllDisksPlugged(moveDiskParamsList)) { + // Cannot live migrate and move disks concurrently + addDisksDeactivatedMessage(moveDiskParamsList); + continue; + } + + // Adding parameters for live migrate + liveMigrateVmDisksParametersList.add(createLiveMigrateVmDisksParameters(moveDiskParamsList, vm.getId())); } else { - addCanDoActionMessage(String.format("$%1$s %2$s", "VmName", vm.getName())); - return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_DOWN_OR_UP); + // Live migrate / move disk is not applicable according to VM status + addInvalidVmStateMessage(vm); } } + } - return true; + /** + * @return a map of VMs to relevant MoveDiskParameters list. + */ + private Map<VM, List<MoveDiskParameters>> createVmDiskParamsMap() { + Map<VM, List<MoveDiskParameters>> vmDisksMap = new HashMap<>(); + for (MoveDiskParameters moveDiskParameters : getParameters().getParametersList()) { + DiskImage diskImage = getDiskImageDao().get(moveDiskParameters.getImageId()); + + Map<Boolean, List<VM>> allVmsForDisk = getVmDAO().getForDisk(diskImage.getId()); + List<VM> vmsForPluggedDisk = allVmsForDisk.get(Boolean.TRUE); + List<VM> vmsForUnpluggedDisk = allVmsForDisk.get(Boolean.FALSE); + + VM vm = vmsForPluggedDisk != null ? vmsForPluggedDisk.get(0) : + vmsForUnpluggedDisk != null ? vmsForUnpluggedDisk.get(0) : null; + + addDiskToMap(diskImage, vmsForPluggedDisk, vmsForUnpluggedDisk); + MultiValueMapUtils.addToMap(vm, moveDiskParameters, vmDisksMap); + } + + return vmDisksMap; + } + + /** + * Add the specified diskImage to diskMap; with updated 'plugged' value. + * (diskMap contains all disks specified in the parameters). + */ + private void addDiskToMap(DiskImage diskImage, List<VM> vmsForPluggedDisk, List<VM> vmsForUnpluggedDisk) { + if (vmsForPluggedDisk != null) { + diskImage.setPlugged(true); + } + else if (vmsForUnpluggedDisk != null) { + diskImage.setPlugged(false); + } + + diskMap.put(diskImage.getImageId(), diskImage); } private LiveMigrateDiskParameters createLiveMigrateDiskParameters(MoveDiskParameters moveDiskParameters, Guid vmId) { @@ -122,29 +172,63 @@ moveDiskParameters.getQuotaId()); } - protected ArrayList<VdcActionParametersBase> getMoveDisksParametersList() { - for (MoveDiskParameters moveDiskParameters : moveParametersList) { - moveDiskParameters.setSessionId(getParameters().getSessionId()); + private LiveMigrateVmDisksParameters createLiveMigrateVmDisksParameters(List<MoveDiskParameters> moveDiskParamsList, Guid vmId) { + // Create LiveMigrateDiskParameters list + List<LiveMigrateDiskParameters> liveMigrateDiskParametersList = new ArrayList<>(); + for (MoveDiskParameters moveDiskParameters : moveDiskParamsList) { + liveMigrateDiskParametersList.add(createLiveMigrateDiskParameters(moveDiskParameters, vmId)); } - return new ArrayList<VdcActionParametersBase>(moveParametersList); + // Create LiveMigrateVmDisksParameters (multiple disks) + LiveMigrateVmDisksParameters liveMigrateDisksParameters = + new LiveMigrateVmDisksParameters(liveMigrateDiskParametersList, vmId); + liveMigrateDisksParameters.setParentCommand(VdcActionType.MoveDisks); + + return liveMigrateDisksParameters; } - protected ArrayList<VdcActionParametersBase> getLiveMigrateDisksParametersList() { - ArrayList<LiveMigrateVmDisksParameters> liveMigrateDisksParametersList = - new ArrayList<LiveMigrateVmDisksParameters>(); - - for (Map.Entry<Guid, List<LiveMigrateDiskParameters>> entry : vmsLiveMigrateParametersMap.entrySet()) { - LiveMigrateVmDisksParameters liveMigrateDisksParameters = - new LiveMigrateVmDisksParameters(entry.getValue(), entry.getKey()); - - liveMigrateDisksParameters.setParentCommand(VdcActionType.MoveDisks); - liveMigrateDisksParameters.setSessionId(getParameters().getSessionId()); - - liveMigrateDisksParametersList.add(liveMigrateDisksParameters); + private ArrayList<VdcActionParametersBase> getParametersArrayList(List<? extends VdcActionParametersBase> parametersList) { + for (VdcActionParametersBase parameters : parametersList) { + parameters.setSessionId(getParameters().getSessionId()); } - return new ArrayList<VdcActionParametersBase>(liveMigrateDisksParametersList); + return new ArrayList<>(parametersList); + } + + /** + * Return true if all specified disks are plugged; otherwise false. + */ + private boolean isAllDisksPlugged(List<MoveDiskParameters> moveDiskParamsList) { + return isAllDisksPlugged(moveDiskParamsList, true); + } + + /** + * Return false if all specified disks are unplugged; otherwise true. + */ + private boolean isAllDisksUnplugged(List<MoveDiskParameters> moveDiskParamsList) { + return isAllDisksPlugged(moveDiskParamsList, false); + } + + private boolean isAllDisksPlugged(List<MoveDiskParameters> moveDiskParamsList, boolean plugged) { + for (MoveDiskParameters moveDiskParameters : moveDiskParamsList) { + DiskImage diskImage = diskMap.get(moveDiskParameters.getImageId()); + if (diskImage.getPlugged() != plugged) { + return false; + } + } + + return true; + } + + private List<String> getUnpluggedDisksAliases(List<MoveDiskParameters> moveDiskParamsList) { + List<String> unpluggedDisks = new LinkedList<>(); + for (MoveDiskParameters moveDiskParameters : moveDiskParamsList) { + DiskImage diskImage = diskMap.get(moveDiskParameters.getImageId()); + if (!diskImage.getPlugged()) { + unpluggedDisks.add(diskImage.getDiskAlias()); + } + } + return unpluggedDisks; } @Override @@ -167,7 +251,11 @@ return getDbFacade().getDiskImageDao(); } - protected SnapshotsValidator createSnapshotsValidator() { - return new SnapshotsValidator(); + protected List<MoveDiskParameters> getMoveDiskParametersList() { + return moveDiskParametersList; + } + + protected List<LiveMigrateVmDisksParameters> getLiveMigrateVmDisksParametersList() { + return liveMigrateVmDisksParametersList; } } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java index 20573c0..093e19d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommand.java @@ -16,6 +16,7 @@ import org.ovirt.engine.core.bll.quota.QuotaConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaStorageConsumptionParameter; import org.ovirt.engine.core.bll.quota.QuotaStorageDependent; +import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.tasks.SPMAsyncTaskHandler; import org.ovirt.engine.core.bll.tasks.TaskHandlerCommand; import org.ovirt.engine.core.bll.utils.PermissionSubject; @@ -234,7 +235,7 @@ setStoragePoolId(getVm().getStoragePoolId()); if (!isValidParametersList() || !checkImagesStatus() || !isValidSpaceRequirements() - || !isVmNotRunningStateless()) { + || !isVmNotRunningStateless() || !isVmNotInPreview()) { return false; } @@ -360,4 +361,12 @@ protected VmValidator createVmValidator() { return new VmValidator(getVm()); } + + private boolean isVmNotInPreview() { + return validate(createSnapshotsValidator().vmNotInPreview(getVmId())); + } + + protected SnapshotsValidator createSnapshotsValidator() { + return new SnapshotsValidator(); + } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveDisksCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveDisksCommandTest.java index 11711bd..8ef0de0 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveDisksCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveDisksCommandTest.java @@ -3,7 +3,6 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; -import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; @@ -18,17 +17,14 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; -import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.common.action.MoveDiskParameters; import org.ovirt.engine.core.common.action.MoveDisksParameters; import org.ovirt.engine.core.common.businessentities.DiskImage; -import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotStatus; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.VMStatus; import org.ovirt.engine.core.common.errors.VdcBllMessages; import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dao.DiskImageDAO; -import org.ovirt.engine.core.dao.SnapshotDao; import org.ovirt.engine.core.dao.VmDAO; @RunWith(MockitoJUnitRunner.class) @@ -45,9 +41,6 @@ @Mock private VmDAO vmDao; - @Mock - protected SnapshotDao snapshotDao; - /** * The command under test */ @@ -57,7 +50,6 @@ public void setupCommand() { initSpyCommand(); mockDaos(); - mockSnapshotValidator(); } private void initSpyCommand() { @@ -78,40 +70,16 @@ } @Test - public void canDoActionImagesNotFound() { - command.getParameters().setParametersList(createMoveDisksParameters()); - - assertFalse(command.canDoAction()); - assertTrue(command.getReturnValue() - .getCanDoActionMessages() - .contains(VdcBllMessages.ACTION_TYPE_FAILED_DISK_NOT_EXIST.toString())); - } - - @Test public void canDoActionInvalidVmStatus() { command.getParameters().setParametersList(createMoveDisksParameters()); initDiskImage(diskImageId); initVm(VMStatus.Unknown, Guid.newGuid(), diskImageId); - assertFalse(command.canDoAction()); + command.updateParameters(); assertTrue(command.getReturnValue() .getCanDoActionMessages() .contains(VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_DOWN_OR_UP.toString())); - } - - @Test - public void canDoActionVmInPreview() { - command.getParameters().setParametersList(createMoveDisksParameters()); - - initDiskImage(diskImageId); - initVm(VMStatus.Down, null, diskImageId); - setVmInPreview(true); - - assertFalse(command.canDoAction()); - assertTrue(command.getReturnValue() - .getCanDoActionMessages() - .contains(VdcBllMessages.ACTION_TYPE_FAILED_VM_IN_PREVIEW.toString())); } @Test @@ -121,8 +89,8 @@ initDiskImage(diskImageId); initVm(VMStatus.Down, null, diskImageId); - assertTrue(command.canDoAction()); - assertFalse(command.getMoveDisksParametersList().isEmpty()); + command.updateParameters(); + assertTrue(command.getMoveDiskParametersList().size() == 1); } @Test @@ -131,8 +99,8 @@ initDiskImage(diskImageId); - assertTrue(command.canDoAction()); - assertFalse(command.getMoveDisksParametersList().isEmpty()); + command.updateParameters(); + assertTrue(command.getMoveDiskParametersList().size() == 1); } @Test @@ -142,8 +110,8 @@ initDiskImage(diskImageId); initVm(VMStatus.Up, Guid.newGuid(), diskImageId); - assertTrue(command.canDoAction()); - assertFalse(command.getLiveMigrateDisksParametersList().isEmpty()); + command.updateParameters(); + assertTrue(command.getLiveMigrateVmDisksParametersList().size() == 1); } @Test @@ -153,8 +121,30 @@ initDiskImageBasedOnTemplate(diskImageId); initVm(VMStatus.Up, Guid.newGuid(), diskImageId); - assertTrue(command.canDoAction()); - assertFalse(command.getLiveMigrateDisksParametersList().isEmpty()); + command.updateParameters(); + assertTrue(command.getLiveMigrateVmDisksParametersList().size() == 1); + } + + @Test + public void moveUnpluggedDiskVmDown() { + command.getParameters().setParametersList(createMoveDisksParameters()); + + initDiskImage(diskImageId); + initVm(VMStatus.Down, Guid.newGuid(), diskImageId, false); + + command.updateParameters(); + assertTrue(command.getMoveDiskParametersList().size() == 1); + } + + @Test + public void moveUnpluggedDiskVmUp() { + command.getParameters().setParametersList(createMoveDisksParameters()); + + initDiskImage(diskImageId); + initVm(VMStatus.Up, Guid.newGuid(), diskImageId, false); + + command.updateParameters(); + assertTrue(command.getMoveDiskParametersList().size() == 1); } @Test @@ -171,24 +161,54 @@ initVm(VMStatus.Up, Guid.newGuid(), diskImageId1); initVm(VMStatus.Down, Guid.newGuid(), diskImageId2); - assertTrue(command.canDoAction()); - assertFalse(command.getMoveDisksParametersList().isEmpty()); - assertFalse(command.getLiveMigrateDisksParametersList().isEmpty()); + command.updateParameters(); + assertTrue(command.getMoveDiskParametersList().size() == 1); + assertTrue(command.getLiveMigrateVmDisksParametersList().size() == 1); + } + + @Test + public void movePluggedDiskAndUnpluggedDiskVmUp() { + Guid diskImageId1 = Guid.newGuid(); + Guid diskImageId2 = Guid.newGuid(); + + MoveDiskParameters moveDiskParameters1 = new MoveDiskParameters(diskImageId1, srcStorageId, dstStorageId); + MoveDiskParameters moveDiskParameters2 = new MoveDiskParameters(diskImageId2, srcStorageId, dstStorageId); + command.getParameters().setParametersList(Arrays.asList(moveDiskParameters1, moveDiskParameters2)); + + initDiskImage(diskImageId1); + initDiskImage(diskImageId2); + initVm(VMStatus.Up, Guid.newGuid(), diskImageId1, true, diskImageId2, false); + + command.updateParameters(); + assertTrue(command.getReturnValue() + .getCanDoActionMessages() + .contains(VdcBllMessages.ACTION_TYPE_FAILED_DISKS_DEACTIVATED.toString())); } /** Initialize Entities */ private void initVm(VMStatus vmStatus, Guid runOnVds, Guid diskImageId) { + initVm(vmStatus, runOnVds, diskImageId, true, null, false); + } + + private void initVm(VMStatus vmStatus, Guid runOnVds, Guid diskImageId, boolean isPlugged) { + initVm(vmStatus, runOnVds, diskImageId, isPlugged, null, false); + } + + private void initVm(VMStatus vmStatus, Guid runOnVds, Guid diskImageId1, boolean isPlugged1, + Guid diskImageId2, boolean isPlugged2) { VM vm = new VM(); vm.setStatus(vmStatus); vm.setRunOnVds(runOnVds); when(vmDao.get(any(Guid.class))).thenReturn(vm); - when(vmDao.getVmsListForDisk(diskImageId)).thenReturn(Collections.singletonList(vm)); - } + when(vmDao.getForDisk(diskImageId1)).thenReturn( + Collections.singletonMap(isPlugged1, Collections.singletonList(vm))); - private void setVmInPreview(boolean isInPreview) { - when(snapshotDao.exists(any(Guid.class), eq(SnapshotStatus.IN_PREVIEW))).thenReturn(isInPreview); + if (diskImageId2 != null) { + when(vmDao.getForDisk(diskImageId2)).thenReturn( + Collections.singletonMap(isPlugged2, Collections.singletonList(vm))); + } } private void initDiskImage(Guid diskImageId) { @@ -205,19 +225,9 @@ private DiskImage mockDiskImage(Guid diskImageId) { DiskImage diskImage = new DiskImage(); diskImage.setId(diskImageId); + diskImage.setImageId(diskImageId); return diskImage; - } - - private void mockSnapshotValidator() { - SnapshotsValidator validator = new SnapshotsValidator() { - @Override - protected SnapshotDao getSnapshotDao() { - return snapshotDao; - } - - }; - doReturn(validator).when(command).createSnapshotsValidator(); } /** Mock DAOs */ diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommandTest.java index e1f3607..7e4734b 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/lsm/LiveMigrateVmDisksCommandTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; @@ -18,10 +19,12 @@ import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.action.LiveMigrateDiskParameters; import org.ovirt.engine.core.common.action.LiveMigrateVmDisksParameters; import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.Snapshot; import org.ovirt.engine.core.common.businessentities.StorageDomain; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.StorageDomainType; @@ -32,6 +35,7 @@ import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.compat.Version; import org.ovirt.engine.core.dao.DiskImageDAO; +import org.ovirt.engine.core.dao.SnapshotDao; import org.ovirt.engine.core.dao.StorageDomainDAO; import org.ovirt.engine.core.dao.StoragePoolDAO; import org.ovirt.engine.core.dao.VmDAO; @@ -60,7 +64,13 @@ private VmDAO vmDao; @Mock + protected SnapshotDao snapshotDao; + + @Mock private VmValidator vmValidator; + + @Mock + private SnapshotsValidator snapshotsValidator; /** * The command under test @@ -182,6 +192,22 @@ .contains(VdcBllMessages.ACTION_TYPE_FAILED_VM_RUNNING_STATELESS.name())); } + @Test + public void canDoActionVmInPreview() { + createParameters(); + initDiskImage(diskImageId); + initVm(VMStatus.Up, null, diskImageId); + setVmInPreview(true); + + doReturn(new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_VM_IN_PREVIEW)).when(snapshotsValidator) + .vmNotInPreview(any(Guid.class)); + + assertFalse(command.canDoAction()); + assertTrue(command.getReturnValue() + .getCanDoActionMessages() + .contains(VdcBllMessages.ACTION_TYPE_FAILED_VM_IN_PREVIEW.toString())); + } + /** Initialize Entities */ private void initVm(VMStatus vmStatus, Guid runOnVds, Guid diskImageId) { @@ -226,6 +252,10 @@ when(command.getStoragePoolId()).thenReturn(storagePoolId); } + private void setVmInPreview(boolean isInPreview) { + when(snapshotDao.exists(any(Guid.class), eq(Snapshot.SnapshotStatus.IN_PREVIEW))).thenReturn(isInPreview); + } + /** Mock DAOs */ private void mockDaos() { @@ -254,6 +284,8 @@ private void mockValidators() { doReturn(vmValidator).when(command).createVmValidator(); + doReturn(snapshotsValidator).when(command).createSnapshotsValidator(); doReturn(ValidationResult.VALID).when(vmValidator).vmNotRunningStateless(); + doReturn(ValidationResult.VALID).when(snapshotsValidator).vmNotInPreview(any(Guid.class)); } } diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index 9d78564..2d182bc 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -105,6 +105,7 @@ ACTION_TYPE_FAILED_VM_IN_PREVIEW(ErrorType.CONFLICT), ACTION_TYPE_FAILED_DISKS_LOCKED(ErrorType.CONFLICT), ACTION_TYPE_FAILED_DISKS_ILLEGAL(ErrorType.INTERNAL_ERROR), + ACTION_TYPE_FAILED_DISKS_DEACTIVATED(ErrorType.INTERNAL_ERROR), ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST(ErrorType.CONFLICT), ACTION_TYPE_FAILED_VM_IS_LOCKED(ErrorType.CONFLICT), ACTION_TYPE_FAILED_VM_DURING_EXPORT(ErrorType.CONFLICT), diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index 214e79a..41deb01 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -137,6 +137,7 @@ ACTION_TYPE_FAILED_VM_IN_PREVIEW=Cannot ${action} ${type}. VM is previewing a Snapshot. ACTION_TYPE_FAILED_DISKS_LOCKED=Cannot ${action} ${type}: The following disks are locked: ${diskAliases}. Please try again in a few minutes. ACTION_TYPE_FAILED_DISKS_ILLEGAL=Cannot ${action} ${type}. The following attached disks are in ILLEGAL status: ${diskAliases} - please remove them and try again. +ACTION_TYPE_FAILED_DISKS_DEACTIVATED=Cannot ${action} ${type}. The following disks are deactivated: ${diskAliases}. Please activate them and try again. ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST=Cannot ${action} ${type}. The following disks already exist: ${diskAliases}. Please import as a clone. ACTION_TYPE_FAILED_VM_IS_LOCKED=Cannot ${action} ${type}: VM is locked. Please try again in a few minutes. ACTION_TYPE_FAILED_VM_DURING_EXPORT=Cannot ${action} ${type}: VM is being exported now. Please try again in a few minutes. diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java index edb4b67..d0942cb 100644 --- a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java +++ b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java @@ -346,6 +346,9 @@ @DefaultStringValue("Cannot ${action} ${type}. The following attached disks are in ILLEGAL status: ${diskAliases} - please remove them and try again.") String ACTION_TYPE_FAILED_DISKS_ILLEGAL(); + @DefaultStringValue("Cannot ${action} ${type}. The following disks are deactivated: ${diskAliases}. Please activate them and try again.") + String ACTION_TYPE_FAILED_DISKS_DEACTIVATED(); + @DefaultStringValue("Cannot ${action} ${type}. The following disks already exist: ${diskAliases}. Please import as a clone.") String ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST(); diff --git a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 06cba89..92a2041 100644 --- a/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -130,6 +130,7 @@ ACTION_TYPE_FAILED_VM_IN_PREVIEW=Cannot ${action} ${type}. VM is previewing a Snapshot. ACTION_TYPE_FAILED_DISKS_LOCKED=Cannot ${action} ${type}: The following disks are locked: ${diskAliases}. Please try again in a few minutes. ACTION_TYPE_FAILED_DISKS_ILLEGAL=Cannot ${action} ${type}. The following attached disks are in ILLEGAL status: ${diskAliases} - please remove them and try again. +ACTION_TYPE_FAILED_DISKS_DEACTIVATED=Cannot ${action} ${type}. The following disks are deactivated: ${diskAliases}. Please activate them and try again. ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST=Cannot ${action} ${type}. The following disks already exist: ${diskAliases}. Please import as a clone. ACTION_TYPE_FAILED_VM_IS_LOCKED=Cannot ${action} ${type}: VM is locked. Please try again in a few minutes. ACTION_TYPE_FAILED_VM_DURING_EXPORT=Cannot ${action} ${type}: VM is being exported now. Please try again in a few minutes. diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 939800c..3f83c1b 100644 --- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -134,6 +134,7 @@ ACTION_TYPE_FAILED_VM_IN_PREVIEW=Cannot ${action} ${type}. VM is previewing a Snapshot. ACTION_TYPE_FAILED_DISKS_LOCKED=Cannot ${action} ${type}: The following disks are locked: ${diskAliases}. Please try again in a few minutes. ACTION_TYPE_FAILED_DISKS_ILLEGAL=Cannot ${action} ${type}. The following attached disks are in ILLEGAL status: ${diskAliases} - please remove them and try again. +ACTION_TYPE_FAILED_DISKS_DEACTIVATED=Cannot ${action} ${type}. The following disks are deactivated: ${diskAliases}. Please activate them and try again. ACTION_TYPE_FAILED_IMPORT_DISKS_ALREADY_EXIST=Cannot ${action} ${type}. The following disks already exist: ${diskAliases}. Please import as a clone. ACTION_TYPE_FAILED_VM_IS_LOCKED=Cannot ${action} ${type}: VM is locked. Please try again in a few minutes. ACTION_TYPE_FAILED_VM_DURING_EXPORT=Cannot ${action} ${type}: VM is being exported now. Please try again in a few minutes. -- To view, visit http://gerrit.ovirt.org/19105 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie0fbfd95c0086cfe0d792b3071a2d804ff32c18e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <de...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
