Liron Aravot has posted comments on this change.
Change subject: core: Fix IndexOutOfBoundsException in plug FC disk
......................................................................
Patch Set 5: (5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java
Line 67: getVm().getId(), disk, vmDevice));
Line 68: }
Line 69:
Line 70: /**
Line 71: * If the LUN has no connections we assume that it is FCP storage
type, since FCP does not represent connections,
/s/does/doesn't /s/represent/have
Line 72: * otherwise, we return the storage type of the first connection
Line 73: *
Line 74: * @param lun
Line 75: * - The lun we set the connection at.
Line 74: * @param lun
Line 75: * - The lun we set the connection at.
Line 76: * @return The storage type of the lun (ISCSI or FCP).
Line 77: */
Line 78: protected StorageType getLUNStorageType(LUNs lun) {
as this method is now protected and not private (as opposed to first patchset)
- I think that if the connection list is null (meaning - load hasn't been done
before), we can perform the load here...to avoid future NPEs - you will have
the NPE here.
Line 79: return lun.getLunConnections().isEmpty() ? StorageType.FCP :
lun.getLunConnections().get(0).getstorage_type();
Line 80: }
Line 81:
Line 82: /**
Line 81:
Line 82: /**
Line 83: * Sets the LUN connection list from the DB.
Line 84: *
Line 85: * @param lun
please remove new line
Line 86: * - The lun we set the connection at.
Line 87: */
Line 88: private void updateLUNConnectionsInfo(LUNs lun) {
Line 89: lun.setLunConnections(new
ArrayList<storage_server_connections>(getDbFacade()
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/HotPlugDiskToVmCommandTest.java
Line 87: public void getFCStorageTypeForLun() throws Exception {
Line 88: LUNs lun = new LUNs();
Line 89: ArrayList<storage_server_connections> connections = new
ArrayList<storage_server_connections>();
Line 90: lun.setLunConnections(connections);
Line 91: StorageType storageType = command.getLUNStorageType(lun);
please switch between the assert values, expected value should be second..
/s/disk/storage type /s/had/have
Line 92: assertEquals("Lun disk should be of FC storage type since it
does not had connections",
Line 93: storageType,
Line 94: storageType.FCP);
Line 95: }
Line 93: storageType,
Line 94: storageType.FCP);
Line 95: }
Line 96:
Line 97: @Test
please switch between the assert values, expected value should be second..
Line 98: public void getISCSIStorageTypeForLun() throws Exception {
Line 99: LUNs lun = new LUNs();
Line 100: ArrayList<storage_server_connections> connections = new
ArrayList<storage_server_connections>();
Line 101: connections.add(new storage_server_connections("Some LUN
connection",
--
To view, visit http://gerrit.ovirt.org/9724
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6414445b9a74be299205ff7fc9a21d1388a29687
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Sharad Mishra <[email protected]>
Gerrit-Reviewer: Vered Volansky <[email protected]>
Gerrit-Reviewer: liron aravot <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
