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 M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskSnapshotsValidator.java M 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, 88 insertions(+), 17 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/17/27817/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 0c7da9a..149d313 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; @@ -284,15 +283,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 index 830ce13..575e22e 100644 --- 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 @@ -4,8 +4,12 @@ import org.ovirt.engine.core.bll.ImagesHandler; 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; import java.util.ArrayList; import java.util.List; @@ -78,4 +82,36 @@ return ValidationResult.VALID; } + + /** + * 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 org.ovirt.engine.core.bll.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 index 0ddd891..e3f91d2 100644 --- 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 @@ -8,9 +8,11 @@ import org.ovirt.engine.core.bll.ValidationResult; 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; import java.util.ArrayList; import java.util.Arrays; @@ -18,7 +20,10 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.junit.matchers.JUnitMatchers.both; +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; @@ -31,6 +36,9 @@ @Mock private DiskImageDAO diskImageDao; + @Mock + private SnapshotDao snapshotDao; + @Before public void setUp() { disk1 = createDisk(); @@ -38,6 +46,9 @@ 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() { @@ -97,4 +108,35 @@ assertThat(validator.imagesAreSnapshots(), failsWith(VdcBllMessages.ACTION_TYPE_FAILED_DISK_SNAPSHOTS_ACTIVE)); } + + @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 8f25e23..25947e5 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 @@ -367,7 +367,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 ff16a20..f70daa0 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -597,7 +597,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 8fa0e81..15e8a5a 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 @@ -1639,8 +1639,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 6735ee2..d653e41 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 @@ -575,7 +575,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 11672ae..fd11289 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 @@ -602,7 +602,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/27817 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iee5d5e6716cec69f36e2e98fa2c1578c53dc2384 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
