Liron Ar has uploaded a new change for review.

Change subject: core : lock correct volume when creating a template
......................................................................

core : lock correct volume when creating a template

When creating a template out of a vm, CreateImageTemplateCommand is
being called for each of the needed vm disks. In that command there all the
disk snapshots are being loaded in order to lock the active disk
snapshot.

That logic caused for 2 bugs in that command:
1. The lock is done always on a wrong disk snapshot, as
ImagesHandler.getAllImageSnapshots() will always return the active
images as first on the list, and ImagesHandler.getActiveVolume() returns
the last on the list, the lock won't be performed on the active volume.

2. The cloned disk properties were copied from that snapshots and from
the correct snapshots, leading to wrong persisted information.

The fix is to just remove that load and set, as currently a template is
created for the active disks only we are supposed to always have the
active one already loaded.

If and when creating a template from a snapshot will be supported, the
locking could be re-inspected, right now there's no need for further db
loads.

Change-Id: I7f27535de8ec6e007035ffa7a2e5fad80460701a
Signed-off-by: Liron Aravot <lara...@redhat.com>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateImageTemplateCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
M 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImportVmCommandTest.java
3 files changed, 8 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/77/20477/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateImageTemplateCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateImageTemplateCommand.java
index a691bd8..9275879 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateImageTemplateCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateImageTemplateCommand.java
@@ -46,9 +46,6 @@
         // Create new image group id and image id:
         Guid destinationImageGroupID = Guid.newGuid();
         setDestinationImageId(Guid.newGuid());
-        getDiskImage().getSnapshots().addAll(
-                
ImagesHandler.getAllImageSnapshots(getDiskImage().getImageId(), 
getDiskImage().getImageTemplateId()));
-        
setDiskImage(ImagesHandler.getActiveVolumeDisk(getDiskImage().getSnapshots()));
         DiskImage newImage = cloneDiskImage(getDestinationImageId());
         fillVolumeInformation(newImage);
 
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
index ff43f08..a5688a3 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ImportVmCommand.java
@@ -299,11 +299,15 @@
             for (Map.Entry<Guid, List<DiskImage>> entry : images.entrySet()) {
                 Guid id = entry.getKey();
                 List<DiskImage> diskList = entry.getValue();
-                getVm().getDiskMap().put(id, 
ImagesHandler.getActiveVolumeDisk(diskList));
+                getVm().getDiskMap().put(id, getActiveVolumeDisk(diskList));
             }
         }
 
         return true;
+    }
+
+    protected DiskImage getActiveVolumeDisk(List<DiskImage> diskList) {
+        return diskList.get(diskList.size() - 1);
     }
 
     /**
@@ -775,7 +779,7 @@
             Guid snapshotId = Guid.newGuid();
             int aliasCounter = 0;
             for (List<DiskImage> diskList : images.values()) {
-                DiskImage disk = ImagesHandler.getActiveVolumeDisk(diskList);
+                DiskImage disk = getActiveVolumeDisk(diskList);
                 disk.setParentId(VmTemplateHandler.BlankVmTemplateId);
                 disk.setImageTemplateId(VmTemplateHandler.BlankVmTemplateId);
                 disk.setVmSnapshotId(snapshotId);
@@ -819,7 +823,7 @@
 
             int aliasCounter = 0;
             for (List<DiskImage> diskList : images.values()) {
-                DiskImage disk = ImagesHandler.getActiveVolumeDisk(diskList);
+                DiskImage disk = getActiveVolumeDisk(diskList);
                 newDiskIdForDisk.put(disk.getId(), disk);
                 snapshotId = disk.getVmSnapshotId();
                 disk.setActive(true);
diff --git 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImportVmCommandTest.java
 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImportVmCommandTest.java
index 7a9d094..29dd4bc 100644
--- 
a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImportVmCommandTest.java
+++ 
b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/ImportVmCommandTest.java
@@ -214,7 +214,7 @@
         diskImage2.setStorageIds(new ArrayList<Guid>());
         diskList.add(diskImage);
         diskList.add(diskImage2);
-        DiskImage disk = ImagesHandler.getActiveVolumeDisk(diskList);
+        DiskImage disk = command.getActiveVolumeDisk(diskList);
         Map<Guid, VmDevice> managedDevices = new HashMap<>();
         managedDevices.put(disk.getId(), new VmDevice());
         Guid beforeOldDiskId = disk.getId();


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7f27535de8ec6e007035ffa7a2e5fad80460701a
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <lara...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to