Maor Lipchuk has uploaded a new change for review.

Change subject: core: Failed move disk should not set illegal disk.
......................................................................

core: Failed move disk should not set illegal disk.

When disk failed to be moved to another storage domain,
the engine set the disk status to Illegal with the destination storage
domain.

The proposed fix, set the disk state to OK, with the original storage
domain, if the move task fails.
By doing that, the user can use the VM disks even if the move operation
failed.

As part of the fix, some of the code which updated the destination
storage domain in the disk, moved from the execute part to the
EndWithSuccess section.

Change-Id: I22c83bdd2e7182be49156c20992c494bcb68915a
Signed-off-by: Maor Lipchuk <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyImageGroupCommand.java
1 file changed, 25 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/03/9103/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyImageGroupCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyImageGroupCommand.java
index c53c2a1..15425b5 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyImageGroupCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyImageGroupCommand.java
@@ -109,20 +109,11 @@
                             sourceDomainId,
                             getParameters().getStorageDomainId()));
 
-            // change storage domain in db only if object moved
-            if (getParameters().getOperation() == ImageOperation.Move
-                    || getParameters().getParentCommand() == 
VdcActionType.ImportVm
-                    || getParameters().getParentCommand() == 
VdcActionType.ImportVmTemplate) {
-                List<DiskImage> snapshots = getDiskImageDao()
-                        
.getAllSnapshotsForImageGroup(getParameters().getDestImageGroupId());
-                setSnapshotForShareableDisk(snapshots);
-                for (DiskImage snapshot : snapshots) {
-                    getImageStorageDomainMapDao().remove
-                            (new 
image_storage_domain_map_id(snapshot.getImageId(), 
snapshot.getstorage_ids().get(0)));
-                    getImageStorageDomainMapDao().save
-                            (new 
image_storage_domain_map(snapshot.getImageId(), 
getParameters().getStorageDomainId()));
-                }
-            } else if (getParameters().getAddImageDomainMapping()) {
+            // Add storage domain in db only if there is new entity in DB.
+            if (getParameters().getOperation() != ImageOperation.Move
+                    && getParameters().getParentCommand() != 
VdcActionType.ImportVm
+                    && getParameters().getParentCommand() != 
VdcActionType.ImportVmTemplate
+                    && getParameters().getAddImageDomainMapping()) {
                 getImageStorageDomainMapDao().save
                         (new 
image_storage_domain_map(getParameters().getImageId(),
                                 getParameters().getStorageDomainId()));
@@ -158,6 +149,24 @@
     }
 
     @Override
+    protected void endSuccessfully() {
+        if (getParameters().getOperation() == ImageOperation.Move ||
+                getParameters().getParentCommand() == VdcActionType.ImportVm ||
+                getParameters().getParentCommand() == 
VdcActionType.ImportVmTemplate) {
+            List<DiskImage> snapshots = getDiskImageDao()
+                    
.getAllSnapshotsForImageGroup(getParameters().getDestImageGroupId());
+            setSnapshotForShareableDisk(snapshots);
+            for (DiskImage snapshot : snapshots) {
+                getImageStorageDomainMapDao().remove
+                        (new 
image_storage_domain_map_id(snapshot.getImageId(), 
snapshot.getstorage_ids().get(0)));
+                getImageStorageDomainMapDao().save
+                        (new image_storage_domain_map(snapshot.getImageId(), 
getParameters().getStorageDomainId()));
+            }
+        }
+        super.endSuccessfully();
+    }
+
+    @Override
     protected void endWithFailure() {
         if (getMoveOrCopyImageOperation() == ImageOperation.Copy) {
             unLockImage();
@@ -168,10 +177,8 @@
                                 getParameters().getStorageDomainId()));
             }
             revertTasks();
-        }
-
-        else {
-            markImageAsIllegal();
+        } else {
+            unLockImage();
         }
 
         setSucceeded(true);


--
To view, visit http://gerrit.ovirt.org/9103
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I22c83bdd2e7182be49156c20992c494bcb68915a
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to