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

Reply via email to