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