Liron Ar has uploaded a new change for review. Change subject: core: snapshot disks should be part of the vm disk map only. ......................................................................
core: snapshot disks should be part of the vm disk map only. *Currently the vm populated DiskList should contain only it's actual image disks. The vm attached snapshot disks should be treated as LUN/shareable disks. Regardless to this change, it needs to be inspected if we want to perform the filtering of the disks when loading it from the db or by using ImagesHandler.filterImageDisks, as currently in many flows the filtering is done although VM.getDiskList() contains only the needed disks - therefore in this change I fixed only this part. *When filtering the vm disks/devices - LUN disks should be filtered out as well. Change-Id: I5d18e5d3e321b871f4c7c283c8950fa7645eed5c Signed-off-by: Liron Aravot <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java 3 files changed, 15 insertions(+), 13 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/97/20897/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java index 4c1b3fa..e30c229 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmTemplateCommand.java @@ -132,7 +132,7 @@ protected void updateVmDisks() { VmHandler.updateDisksFromDb(getVm()); - VmHandler.filterDisksForVM(getVm(), false, false, true); + VmHandler.filterImageDisksForVM(getVm(), false, false, true); mImages.addAll(getVm().getDiskList()); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java index 6ae4958..bf1dc76 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java @@ -254,7 +254,7 @@ public static void updateDisksForVm(VM vm, Collection<? extends Disk> disks) { for (Disk disk : disks) { - if (disk.isAllowSnapshot()) { + if (disk.isAllowSnapshot() && !disk.isDiskSnapshot()) { DiskImage image = (DiskImage) disk; vm.getDiskMap().put(image.getId(), image); vm.getDiskList().add(image); @@ -264,16 +264,18 @@ } } - public static void filterDisksForVM(VM vm, boolean allowOnlyNotShareableDisks, - boolean allowOnlySnapableDisks, boolean allowOnlyActiveDisks) { - List<DiskImage> filteredDisks = ImagesHandler.filterImageDisks(vm.getDiskList(), + /** + * Filters the vm image disks/disk devices according to the given parameters + * note: all the given parameters are relevant for image disks, luns will be filtered. + */ + public static void filterImageDisksForVM(VM vm, boolean allowOnlyNotShareableDisks, + boolean allowOnlySnapableDisks, boolean allowOnlyActiveDisks) { + List<DiskImage> filteredDisks = ImagesHandler.filterImageDisks(vm.getDiskMap().values(), allowOnlyNotShareableDisks, allowOnlySnapableDisks, allowOnlyActiveDisks); - Collection<DiskImage> vmDisksToRemove = CollectionUtils.subtract(vm.getDiskList(), filteredDisks); - // done so that lun disks would be included as well - Collection<Disk> vmDisksAfterFilter = CollectionUtils.disjunction(vm.getDiskMap().values(), vmDisksToRemove); + Collection<? extends Disk> vmDisksToRemove = CollectionUtils.subtract(vm.getDiskMap().values(), filteredDisks); vm.clearDisks(); - updateDisksForVm(vm, vmDisksAfterFilter); - for (DiskImage diskToRemove : vmDisksToRemove) { + updateDisksForVm(vm, filteredDisks); + for (Disk diskToRemove : vmDisksToRemove) { vm.getManagedVmDeviceMap().remove(diskToRemove.getId()); } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java index d6f0e75..7049f97 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/VmHandlerTest.java @@ -108,7 +108,7 @@ disks.add(snapshotDisk1); disks.add(snapshotDisk2); populateVmWithDisks(disks, vm); - VmHandler.filterDisksForVM(vm, false, false, true); + VmHandler.filterImageDisksForVM(vm, false, false, true); assertTrue(vm.getDiskList().isEmpty()); assertTrue(vm.getManagedVmDeviceMap().isEmpty()); } @@ -121,11 +121,11 @@ VM vm = new VM(); vm.setId(Guid.newGuid()); populateVmWithDisks(Arrays.asList(snapshotDisk, regularDisk, lunDisk), vm); - VmHandler.filterDisksForVM(vm, false, false, true); + VmHandler.filterImageDisksForVM(vm, false, false, true); assertFalse(vm.getDiskList().contains(snapshotDisk)); assertTrue(vm.getDiskList().contains(regularDisk)); assertTrue(vm.getManagedVmDeviceMap().containsKey(regularDisk.getId())); - assertTrue(vm.getManagedVmDeviceMap().containsKey(lunDisk.getId())); + assertFalse(vm.getManagedVmDeviceMap().containsKey(lunDisk.getId())); assertFalse(vm.getManagedVmDeviceMap().containsKey(snapshotDisk.getId())); } -- To view, visit http://gerrit.ovirt.org/20897 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5d18e5d3e321b871f4c7c283c8950fa7645eed5c Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.3 Gerrit-Owner: Liron Ar <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
