Allon Mureinik has posted comments on this change.

Change subject: webadmin: Fix disk alias recycling
......................................................................


Patch Set 1:

(2 comments)

See comments inline.
Also, please add a unit test to this new logic.

http://gerrit.ovirt.org/#/c/24061/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQuery.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetNextAvailableDiskAliasNameByVMIdQuery.java:

Line 21:         } else {
Line 22:             VM vm = 
getDbFacade().getVmDao().get(getParameters().getId(), getUserID(), 
getParameters().isFiltered());
Line 23:             if (vm != null) {
Line 24:                 updateDisksFromDb(vm);
Line 25:                 List<String> aliases = createDiskAliasesList(vm);
Use a Set, not a List
Line 26:                 int i = 0;
Line 27:                 do {
Line 28:                     i++;
Line 29:                     suggestedDiskName = 
ImagesHandler.getDefaultDiskAlias(vm.getName(), 
Integer.toString(vm.getDiskMapCount() + i));


Line 26:                 int i = 0;
Line 27:                 do {
Line 28:                     i++;
Line 29:                     suggestedDiskName = 
ImagesHandler.getDefaultDiskAlias(vm.getName(), 
Integer.toString(vm.getDiskMapCount() + i));
Line 30:                 } while(aliases.contains(suggestedDiskName));
I'd extract this to another method
Line 31:             }
Line 32:             getQueryReturnValue().setReturnValue(suggestedDiskName);
Line 33:         }
Line 34:     }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie852fbcf26c0742c610631a121b9bfd706c01218
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tal Nisan <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to