Tal Nisan has uploaded a new change for review. Change subject: webadmin: Fix disk alias recycling ......................................................................
webadmin: Fix disk alias recycling When creating a new disk via webadmin, the suggested disk alias created the suggested alias from the VM name concated with the amount of the disk on the VM + 1, in case all disk were created by that convention, deleting a disk which is not the last one and adding a new disk resulted in a suggested disk alias which matches the last disk alias exactly Change-Id: Ie852fbcf26c0742c610631a121b9bfd706c01218 Signed-off-by: Tal Nisan <[email protected]> Bug-url: https://bugzilla.redhat.com/1122932 --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQuery.java M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQueryTest.java 2 files changed, 63 insertions(+), 12 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/83/30783/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQuery.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQuery.java index ed05073..5961744 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQuery.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQuery.java @@ -1,5 +1,9 @@ package org.ovirt.engine.core.bll; +import java.util.HashSet; +import java.util.Set; + +import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.queries.IdQueryParameters; @@ -18,8 +22,7 @@ VM vm = getDbFacade().getVmDao().get(getParameters().getId(), getUserID(), getParameters().isFiltered()); if (vm != null) { updateDisksFromDb(vm); - suggestedDiskName = - ImagesHandler.getDefaultDiskAlias(vm.getName(), Integer.toString(vm.getDiskMapCount() + 1)); + suggestedDiskName = getSuggestedDiskName(vm); } getQueryReturnValue().setReturnValue(suggestedDiskName); } @@ -28,4 +31,23 @@ protected void updateDisksFromDb(VM vm) { VmHandler.updateDisksFromDb(vm); } + + private String getSuggestedDiskName(VM vm) { + Set<String> aliases = createDiskAliasesList(vm); + String suggestedDiskName; + int i = 0; + do { + i++; + suggestedDiskName = ImagesHandler.getDefaultDiskAlias(vm.getName(), Integer.toString(vm.getDiskMapCount() + i)); + } while (aliases.contains(suggestedDiskName)); + return suggestedDiskName; + } + + private Set<String> createDiskAliasesList(VM vm) { + Set<String> diskAliases = new HashSet<>(vm.getDiskMap().size()); + for (Disk disk : vm.getDiskMap().values()) { + diskAliases.add(disk.getDiskAlias()); + } + return diskAliases; + } } diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQueryTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQueryTest.java index bd42b4f..73930fe 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQueryTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQueryTest.java @@ -35,10 +35,10 @@ } @Test - public void testExecuteQuery() throws Exception { + public void testExecuteQueryVmWithNoDisks() throws Exception { mockDAOForQuery(); vm = mockVmAndReturnFromDAO(); - String diskAliasName = VM_NAME + "_Disk1"; + String diskAliasName = ImagesHandler.getDefaultDiskAlias(vm.getName(), "1"); // Execute query. getQuery().executeQueryCommand(); @@ -56,18 +56,47 @@ } @Test - public void testExecuteQueryWithOneImage() throws Exception { + public void testExecuteQueryVmWithMultipleDisks() throws Exception { mockDAOForQuery(); vm = mockVmAndReturnFromDAO(); - Map<Guid, Disk> diskMap = vm.getDiskMap(); - DiskImage diskImage = new DiskImage(); - diskImage.setImageId(Guid.newGuid()); - diskMap.put(diskImage.getId(), diskImage); - String diskAliasName = VM_NAME + "_Disk2"; + populateVmDiskMap(vm, 5); + String expectedDiskAlias = ImagesHandler.getDefaultDiskAlias(vm.getName(), "6"); - // Execute query. getQuery().executeQueryCommand(); - assertEquals(diskAliasName, getQuery().getQueryReturnValue().getReturnValue()); + assertEquals(expectedDiskAlias, getQuery().getQueryReturnValue().getReturnValue()); + } + + /** + * When removing a disk from VM with n disks the default disk alias will be VM_DISK{n} yet this alias is already + * taken since we created n disks to begin with, this test asserts that the suggested alias returned by this query + * will be VM_DISK{n+1} + */ + @Test + public void testExecuteQueryNotOverlappingExisting() throws Exception { + mockDAOForQuery(); + vm = mockVmAndReturnFromDAO(); + populateVmDiskMap(vm, 5); + + vm.getDiskMap().remove(vm.getDiskMap().entrySet().iterator().next()); + + String expectedDiskAlias = ImagesHandler.getDefaultDiskAlias(vm.getName(), "6"); + + getQuery().executeQueryCommand(); + assertEquals(expectedDiskAlias, getQuery().getQueryReturnValue().getReturnValue()); + } + + /** + * Populates the VM disk map with the amount of disks specified, each with a default disk alias + */ + private void populateVmDiskMap(VM vm, int numOfDisks) { + Map<Guid, Disk> diskMap = vm.getDiskMap(); + + for (Integer i = 0; i < numOfDisks; i++) { + DiskImage diskImage = new DiskImage(); + diskImage.setId(Guid.newGuid()); + diskImage.setDiskAlias(ImagesHandler.getDefaultDiskAlias(vm.getName(), i.toString())); + diskMap.put(diskImage.getId(), diskImage); + } } /** -- To view, visit http://gerrit.ovirt.org/30783 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie852fbcf26c0742c610631a121b9bfd706c01218 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.4 Gerrit-Owner: Tal Nisan <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
