Daniel Erez has uploaded a new change for review. Change subject: core: block preview of active vm snapshot ......................................................................
core: block preview of active vm snapshot * TryBackToAllSnapshotsOfVmCommand -> canDo: - Active VM snapshot validation. * DiskSnapshotsValidator: - Added canDiskSnapshotsBePreviewed validation. - Allow custom preview of active VM configuration only when selecting a subset of snapshot disks. Change-Id: Iee5d5e6716cec69f36e2e98fa2c1578c53dc2384 Bug-Url: https://bugzilla.redhat.com/1096884 Signed-off-by: Daniel Erez <de...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java A backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskSnapshotsValidator.java A backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskSnapshotsValidatorTest.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 8 files changed, 155 insertions(+), 17 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/02/28302/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java index c60e28f..0678ffe 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java @@ -13,6 +13,7 @@ import org.ovirt.engine.core.bll.snapshots.SnapshotsValidator; import org.ovirt.engine.core.bll.storage.StoragePoolValidator; import org.ovirt.engine.core.bll.validator.DiskImagesValidator; +import org.ovirt.engine.core.bll.validator.DiskSnapshotsValidator; import org.ovirt.engine.core.bll.validator.MultipleStorageDomainsValidator; import org.ovirt.engine.core.bll.validator.VmValidator; import org.ovirt.engine.core.common.AuditLogType; @@ -35,8 +36,6 @@ import org.ovirt.engine.core.compat.Guid; import org.ovirt.engine.core.dal.dbbroker.DbFacade; import org.ovirt.engine.core.dao.SnapshotDao; -import org.ovirt.engine.core.utils.linq.Function; -import org.ovirt.engine.core.utils.linq.LinqUtils; import org.ovirt.engine.core.utils.transaction.TransactionMethod; import org.ovirt.engine.core.utils.transaction.TransactionSupport; @@ -281,15 +280,9 @@ } } - List<Guid> snapshotIds = LinqUtils.foreach(diskImages, new Function<DiskImage, Guid>() { - @Override - public Guid eval(DiskImage disk) { - return disk.getVmSnapshotId(); - } - }); - // Allow active VM custom preview (i.e. block only when no images are specified). - if (snapshotIds.contains(getParameters().getDstSnapshotId()) && getParameters().getDisks() == null) { - return failCanDoAction(VdcBllMessages.CANNOT_PREIEW_CURRENT_IMAGE); + DiskSnapshotsValidator diskSnapshotsValidator = new DiskSnapshotsValidator(getParameters().getDisks()); + if (!validate(diskSnapshotsValidator.canDiskSnapshotsBePreviewed(getParameters().getDstSnapshotId()))) { + return false; } return true; diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskSnapshotsValidator.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskSnapshotsValidator.java new file mode 100644 index 0000000..7d9a564 --- /dev/null +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskSnapshotsValidator.java @@ -0,0 +1,53 @@ +package org.ovirt.engine.core.bll.validator; + +import java.util.List; + +import org.ovirt.engine.core.bll.ValidationResult; +import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.Snapshot; +import org.ovirt.engine.core.common.errors.VdcBllMessages; +import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dal.dbbroker.DbFacade; +import org.ovirt.engine.core.dao.DiskImageDAO; +import org.ovirt.engine.core.dao.SnapshotDao; + +public class DiskSnapshotsValidator { + + private List<DiskImage> images; + + public DiskSnapshotsValidator(List<DiskImage> images) { + this.images = images; + } + + /** + * Validated whether the disk snapshots can be preview (according to the specified snapshot ID). + * Previewing an Active VM snapshot is applicable only when custom selecting a subset of disks + * (I.e. regular preview of Active VM snapshot isn't allowed). + * + * @return A {@link ValidationResult} with the validation information. + */ + public ValidationResult canDiskSnapshotsBePreviewed(Guid dstSnapshotId) { + Snapshot dstSnapshot = getSnapshotDao().get(dstSnapshotId); + if (dstSnapshot.getType() == Snapshot.SnapshotType.ACTIVE) { + if (images != null && !images.isEmpty()) { + for (DiskImage diskImage : images) { + if (getDiskImageDao().get(diskImage.getImageId()) == null) { + return ValidationResult.VALID; + } + } + } + + return new ValidationResult(VdcBllMessages.CANNOT_PREVIEW_ACTIVE_SNAPSHOT); + } + + return ValidationResult.VALID; + } + + protected SnapshotDao getSnapshotDao() { + return DbFacade.getInstance().getSnapshotDao(); + } + + protected DiskImageDAO getDiskImageDao() { + return DbFacade.getInstance().getDiskImageDao(); + } +} diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskSnapshotsValidatorTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskSnapshotsValidatorTest.java new file mode 100644 index 0000000..8c80ef8 --- /dev/null +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/DiskSnapshotsValidatorTest.java @@ -0,0 +1,92 @@ +package org.ovirt.engine.core.bll.validator; + +import static org.junit.Assert.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; +import static org.ovirt.engine.core.bll.validator.ValidationResultMatchers.failsWith; +import static org.ovirt.engine.core.bll.validator.ValidationResultMatchers.isValid; + +import java.util.ArrayList; +import java.util.Arrays; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; +import org.ovirt.engine.core.common.businessentities.DiskImage; +import org.ovirt.engine.core.common.businessentities.ImageStatus; +import org.ovirt.engine.core.common.businessentities.Snapshot; +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; + +@RunWith(MockitoJUnitRunner.class) +public class DiskSnapshotsValidatorTest { + private DiskImage disk1; + private DiskImage disk2; + private DiskSnapshotsValidator validator; + + @Mock + private DiskImageDAO diskImageDao; + + @Mock + private SnapshotDao snapshotDao; + + @Before + public void setUp() { + disk1 = createDisk(); + disk1.setDiskAlias("disk1"); + disk2 = createDisk(); + disk2.setDiskAlias("disk2"); + validator = spy(new DiskSnapshotsValidator(Arrays.asList(disk1, disk2))); + + doReturn(diskImageDao).when(validator).getDiskImageDao(); + doReturn(snapshotDao).when(validator).getSnapshotDao(); + } + + private static DiskImage createDisk() { + DiskImage disk = new DiskImage(); + disk.setImageId(Guid.newGuid()); + disk.setActive(true); + disk.setImageStatus(ImageStatus.OK); + ArrayList<Guid> storageDomainIds = new ArrayList<>(); + storageDomainIds.add(Guid.newGuid()); + disk.setStorageIds(storageDomainIds); + return disk; + } + + @Test + public void diskSnapshotsCanBePreviewed() { + Snapshot activeSnapshot = getActiveSnapshot(); + when(snapshotDao.get(any(Guid.class))).thenReturn(activeSnapshot); + + when(diskImageDao.get(disk1.getImageId())).thenReturn(null); + when(diskImageDao.get(disk2.getImageId())).thenReturn(null); + + assertThat(validator.canDiskSnapshotsBePreviewed(activeSnapshot.getId()), isValid()); + } + + @Test + public void diskSnapshotsCannotBePreviewed() { + Snapshot activeSnapshot = getActiveSnapshot(); + when(snapshotDao.get(any(Guid.class))).thenReturn(activeSnapshot); + + when(diskImageDao.get(disk1.getImageId())).thenReturn(disk1); + when(diskImageDao.get(disk2.getImageId())).thenReturn(disk2); + + assertThat(validator.canDiskSnapshotsBePreviewed(activeSnapshot.getId()), + failsWith(VdcBllMessages.CANNOT_PREVIEW_ACTIVE_SNAPSHOT)); + } + + private Snapshot getActiveSnapshot() { + Snapshot snapshot = new Snapshot(); + snapshot.setId(Guid.newGuid()); + snapshot.setType(Snapshot.SnapshotType.ACTIVE); + + return snapshot; + } +} 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 5ad73aa..fc42dc8 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 @@ -360,7 +360,7 @@ VM_PINNING_PCPU_DOES_NOT_EXIST(ErrorType.BAD_PARAMETERS), VM_PINNING_DUPLICATE_DEFINITION(ErrorType.BAD_PARAMETERS), VM_PINNING_PINNED_TO_NO_CPU(ErrorType.BAD_PARAMETERS), - CANNOT_PREIEW_CURRENT_IMAGE(ErrorType.BAD_PARAMETERS), + CANNOT_PREVIEW_ACTIVE_SNAPSHOT(ErrorType.BAD_PARAMETERS), VM_CANNOT_SUSPENDE_HAS_RUNNING_TASKS(ErrorType.CONFLICT), VM_CANNOT_REMOVE_HAS_RUNNING_TASKS(ErrorType.CONFLICT), VM_CANNOT_RUN_FROM_NETWORK_WITHOUT_NETWORK(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 e22a39a..c53cf6e 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -583,7 +583,7 @@ ACTION_TYPE_FAILED_EXTERNAL_NETWORK_CANNOT_HAVE_MTU=Cannot ${action} ${type}. External network cannot have MTU set. ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_REQUIRED=Cannot ${action} ${type}. The management network '${NetworkName}' must be required, please change the network to be required and try again. ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED=Cannot ${action} ${type}. The address of the network '${ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED_LIST}' cannot be modified without reinstalling the host, since this address was used to create the host's certification. -CANNOT_PREIEW_CURRENT_IMAGE=The currently used VM Snapshot Image cannot be used in Preview command. +CANNOT_PREVIEW_ACTIVE_SNAPSHOT=Cannot preview Active VM snapshot. CONFIG_UNKNOWN_KEY=Illegal configuration entry.\n\ -Please check configuration entry name. TAGS_SPECIFIED_TAG_CANNOT_BE_THE_PARENT_OF_ITSELF=Operation canceled, recursive Tag hierarchy cannot be defined. 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 347b882..61e7a88 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 @@ -1597,8 +1597,8 @@ @DefaultStringValue("Cannot ${action} ${type}. The address of the network '${ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED_LIST}' cannot be modified without reinstalling the host, since this address was used to create the host's certification.") String ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED(); - @DefaultStringValue("The currently used VM Snapshot Image cannot be used in Preview command.") - String CANNOT_PREIEW_CURRENT_IMAGE(); + @DefaultStringValue("Cannot preview Active VM snapshot.") + String CANNOT_PREVIEW_ACTIVE_SNAPSHOT(); @DefaultStringValue("Illegal configuration entry.\n-Please check configuration entry name.") String CONFIG_UNKNOWN_KEY(); 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 d4f4cc3..0e8dcde 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 @@ -563,7 +563,7 @@ ACTION_TYPE_FAILED_EXTERNAL_NETWORK_CANNOT_HAVE_MTU=Cannot ${action} ${type}. External network cannot have MTU set. ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_REQUIRED=Cannot ${action} ${type}. The management network '${NetworkName}' must be required, please change the network to be required and try again. ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED=Cannot ${action} ${type}. The address of the network '${ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED_LIST}' cannot be modified without reinstalling the host, since this address was used to create the host's certification. -CANNOT_PREIEW_CURRENT_IMAGE=The currently used VM Snapshot Image cannot be used in Preview command. +CANNOT_PREVIEW_ACTIVE_SNAPSHOT=Cannot preview Active VM snapshot. CONFIG_UNKNOWN_KEY=Illegal configuration entry.\n\ -Please check configuration entry name. TAGS_SPECIFIED_TAG_CANNOT_BE_THE_PARENT_OF_ITSELF=Operation canceled, recursive Tag hierarchy cannot be defined. 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 3187683..c0d65ce 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 @@ -587,7 +587,7 @@ ACTION_TYPE_FAILED_EXTERNAL_NETWORK_CANNOT_HAVE_MTU=Cannot ${action} ${type}. External network cannot have MTU set. ACTION_TYPE_FAILED_MANAGEMENT_NETWORK_REQUIRED=Cannot ${action} ${type}. The management network '${NetworkName}' must be required, please change the network to be required and try again. ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED=Cannot ${action} ${type}. The address of the network '${ACTION_TYPE_FAILED_NETWORK_ADDRESS_CANNOT_BE_CHANGED_LIST}' cannot be modified without reinstalling the host, since this address was used to create the host's certification. -CANNOT_PREIEW_CURRENT_IMAGE=The currently used VM Snapshot Image cannot be used in Preview command. +CANNOT_PREVIEW_ACTIVE_SNAPSHOT=Cannot preview Active VM snapshot. CONFIG_UNKNOWN_KEY=Illegal configuration entry.\n\ -Please check configuration entry name. TAGS_SPECIFIED_TAG_CANNOT_BE_THE_PARENT_OF_ITSELF=Operation canceled, recursive Tag hierarchy cannot be defined. -- To view, visit http://gerrit.ovirt.org/28302 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iee5d5e6716cec69f36e2e98fa2c1578c53dc2384 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Daniel Erez <de...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
