Maor Lipchuk has uploaded a new change for review. Change subject: core: Add snapshot validation for empty guid. ......................................................................
core: Add snapshot validation for empty guid. Currently when virt-v2v is used to convert a vm from a foreign hypervisor (Xen, VMware and etc), it sets the vm_snapshot_id of all disks of that vm to empty guid. This later causes issues doing different tasks on snapshots. The proposed fix is to add a validation whether the snapshot id is empty and provides an appropriate message. Change-Id: I19bc2982adecca198f8dc38ca011f4528dd58db9 Signed-off-by: Maor Lipchuk <mlipc...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommandTest.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, 27 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/01/15701/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java index 43b4b21..b3f6f99 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RestoreAllSnapshotsCommand.java @@ -299,6 +299,10 @@ @Override protected boolean canDoAction() { + if (Guid.Empty.equals(getParameters().getDstSnapshotId())) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID); + } + SnapshotsValidator snapshotValidator = createSnapshotValidator(); VmValidator vmValidator = new VmValidator(getVm()); if (!validate(snapshotValidator.snapshotExists(getVmId(), getParameters().getDstSnapshotId())) || 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 f31bcb7..8393ca5 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 @@ -199,6 +199,10 @@ @Override protected boolean canDoAction() { + if (Guid.Empty.equals(getParameters().getDstSnapshotId())) { + return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID); + } + Snapshot snapshot = getSnapshotDao().get(getParameters().getDstSnapshotId()); SnapshotsValidator snapshotsValidator = new SnapshotsValidator(); VmValidator vmValidator = new VmValidator(getVm()); @@ -206,7 +210,6 @@ || !validate(snapshotsValidator.snapshotExists(getVmId(), getParameters().getDstSnapshotId())) || !validate(snapshotsValidator.vmNotDuringSnapshot(getVmId())) || !validate(snapshotsValidator.vmNotInPreview(getVmId())) - || !validate(snapshotsValidator.snapshotExists(snapshot)) || !validate(snapshotsValidator.snapshotNotBroken(snapshot))) { return false; } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommandTest.java index 84163e0..5498ea3 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommandTest.java @@ -32,10 +32,11 @@ private VM vm; private Snapshot snapshot; + Guid vmId; @Before public void setUp() { - Guid vmId = Guid.NewGuid(); + vmId = Guid.NewGuid(); vm = new VM(); vm.setId(vmId); when(vmDao.get(vmId)).thenReturn(vm); @@ -59,4 +60,14 @@ vm.setStatus(VMStatus.Up); CanDoActionTestUtils.runAndAssertCanDoActionFailure(cmd, VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_NOT_DOWN); } + + @Test + public void testCanDoActionWithEmptySnapshotGuid() { + TryBackToAllSnapshotsOfVmParameters params = new TryBackToAllSnapshotsOfVmParameters(vmId, Guid.Empty); + cmd = spy(new TryBackToAllSnapshotsOfVmCommand<TryBackToAllSnapshotsOfVmParameters>(params)); + doNothing().when(cmd).updateVmDisksFromDb(); + doReturn(snapshotDao).when(cmd).getSnapshotDao(); + CanDoActionTestUtils.runAndAssertCanDoActionFailure(cmd, + VdcBllMessages.ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID); + } } 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 4617e4d..c3b4f0f 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 @@ -95,6 +95,7 @@ ACTION_TYPE_FAILED_VM_HAS_NO_DISKS(ErrorType.CONFLICT), ACTION_TYPE_FAILED_VM_IMAGE_DOES_NOT_EXIST(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_VM_SNAPSHOT_DOES_NOT_EXIST(ErrorType.BAD_PARAMETERS), + ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID(ErrorType.DATA_CORRUPTION), ACTION_TYPE_FAILED_VM_SNAPSHOT_HAS_NO_CONFIGURATION(ErrorType.BAD_PARAMETERS), ACTION_TYPE_FAILED_VM_SNAPSHOT_IS_BROKEN(ErrorType.DATA_CORRUPTION), ACTION_TYPE_FAILED_IMAGE_REPOSITORY_NOT_FOUND(ErrorType.BAD_PARAMETERS), 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 fe4344b..2066599 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -13,6 +13,7 @@ VMT_CANNOT_REMOVE_DOMAINS_LIST_MISMATCH=Cannot delete Template. The Template does not exist on the following Storage Domains: ${domainsList}.\nEither verify that Template exists on all Storage Domains listed on the domains list,\nor do not send domains list in order to delete all instances of the Template from the system. ACTION_TYPE_FAILED_VM_IMAGE_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Image does not exist. ACTION_TYPE_FAILED_VM_SNAPSHOT_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Snapshot does not exist. +ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID=Cannot ${action} ${type}. The snapshot configuration has been corrupted (snapshot id is empty). Please contact the system administrator. ACTION_TYPE_FAILED_VM_SNAPSHOT_HAS_NO_CONFIGURATION=Cannot ${action} ${type}. The snapshot ${SnapshotName} of VM ${VmName} has no configuration available. Please choose a snapshot with configuration available. ACTION_TYPE_FAILED_VM_SNAPSHOT_IS_BROKEN=Cannot ${action} ${type}. The snapshot is broken, and no further work can be done on it. Please remove this snapshot from the VM. IMAGE_REPOSITORY_NOT_FOUND=Storage Domain cannot be accessed.\n\ 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 cf837f7..1f595f6 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 @@ -49,6 +49,9 @@ @DefaultStringValue("Cannot ${action} ${type}. VM's Snapshot does not exist.") String ACTION_TYPE_FAILED_VM_SNAPSHOT_DOES_NOT_EXIST(); + @DefaultStringValue("Cannot ${action} ${type}. The snapshot configuration has been corrupted (snapshot id is empty). Please contact the system administrator.") + String ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID(); + @DefaultStringValue("Cannot ${action} ${type}. The snapshot ${SnapshotName} of VM ${VmName} has no configuration available. Please choose a snapshot with configuration available.") String ACTION_TYPE_FAILED_VM_SNAPSHOT_HAS_NO_CONFIGURATION(); 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 c958c03..6839418 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 @@ -13,6 +13,7 @@ VMT_CANNOT_REMOVE_DOMAINS_LIST_MISMATCH=Cannot delete Template. The Template does not exist on the following Storage Domains: ${domainsList}.\nEither verify that Template exists on all Storage Domains listed on the domains list,\nor do not send domains list in order to delete all instances of the Template from the system. ACTION_TYPE_FAILED_VM_IMAGE_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Image does not exist. ACTION_TYPE_FAILED_VM_SNAPSHOT_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Snapshot does not exist. +ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID=Cannot ${action} ${type}. The snapshot configuration has been corrupted (snapshot id is empty). Please contact the system administrator. ACTION_TYPE_FAILED_VM_SNAPSHOT_HAS_NO_CONFIGURATION=Cannot ${action} ${type}. The snapshot ${SnapshotName} of VM ${VmName} has no configuration available. Please choose a snapshot with configuration available. ACTION_TYPE_FAILED_VM_SNAPSHOT_IS_BROKEN=Cannot ${action} ${type}. The snapshot is broken, and no further work can be done on it. Please remove this snapshot from the VM. IMAGE_REPOSITORY_NOT_FOUND=Storage Domain cannot be accessed.\n\ 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 dc68e5b..f0134e5 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 @@ -13,6 +13,7 @@ VMT_CANNOT_REMOVE_DOMAINS_LIST_MISMATCH=Cannot delete Template. The Template does not exist on the following Storage Domains: ${domainsList}.\nEither verify that Template exists on all Storage Domains listed on the domains list,\nor do not send domains list in order to delete all instances of the Template from the system. ACTION_TYPE_FAILED_VM_IMAGE_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Image does not exist. ACTION_TYPE_FAILED_VM_SNAPSHOT_DOES_NOT_EXIST=Cannot ${action} ${type}. VM's Snapshot does not exist. +ACTION_TYPE_FAILED_CORRUPTED_VM_SNAPSHOT_ID=Cannot ${action} ${type}. The snapshot configuration has been corrupted (snapshot id is empty). Please contact the system administrator. ACTION_TYPE_FAILED_VM_SNAPSHOT_HAS_NO_CONFIGURATION=Cannot ${action} ${type}. The snapshot ${SnapshotName} of VM ${VmName} has no configuration available. Please choose a snapshot with configuration available. ACTION_TYPE_FAILED_VM_SNAPSHOT_IS_BROKEN=Cannot ${action} ${type}. The snapshot is broken, and no further work can be done on it. Please remove this snapshot from the VM. IMAGE_REPOSITORY_NOT_FOUND=Storage Domain cannot be accessed.\n\ -- To view, visit http://gerrit.ovirt.org/15701 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I19bc2982adecca198f8dc38ca011f4528dd58db9 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches