Allon Mureinik has posted comments on this change.
Change subject: core: Fix IndexOutOfBoundsException in plug FC disk
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(5 inline comments)
The idea, in general, seems fine for 3.2.
Looking forward, I'd look into persisting the LUN type in the DB, and avoid
such heuristics as "if it doesn't have any connections, treat it as FCP".
Also, regarding the current implementation:
1. See some minor comments inline
2. Please add a test for this new functionality.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java
Line 71: * storage type, since FCP does not represent connections, if
other wise, we return the storage type of the first
Line 72: * connection
Line 73: *
Line 74: * @param lun
Line 75: * @return
Either document the @param and @return javadoc elements, or remove them
completely.
Line 76: */
Line 77: private StorageType getLunStorageType(LUNs lun) {
Line 78: lun.setLunConnections(new
ArrayList<storage_server_connections>(getDbFacade()
Line 79: .getStorageServerConnectionDao()
Line 72: * connection
Line 73: *
Line 74: * @param lun
Line 75: * @return
Line 76: */
I agree this name is a bit missleading.
How about having something like updateLUNConnectionsInfo() ?
Line 77: private StorageType getLunStorageType(LUNs lun) {
Line 78: lun.setLunConnections(new
ArrayList<storage_server_connections>(getDbFacade()
Line 79: .getStorageServerConnectionDao()
Line 80: .getAllForLun(lun.getLUN_id())));
....................................................
Commit Message
Line 5: CommitDate: 2012-12-04 19:12:03 +0200
Line 6:
Line 7: core: Fix IndexOutOfBoundsException in plug FC disk
Line 8:
Line 9: attach detach and plug disk throws an exception of index out of bounds
s/attach detach/Attach, detach/
s/ an exception of index out of bounds exception/an IndexOutOfBoundsException/
Line 10: exception.
Line 11: The reason for that is that the LUN FC which the disk is related to
does not contains any
Line 12: connections in it, compared to ISCSI lun which must have connections
Line 13: related to it.
Line 7: core: Fix IndexOutOfBoundsException in plug FC disk
Line 8:
Line 9: attach detach and plug disk throws an exception of index out of bounds
Line 10: exception.
Line 11: The reason for that is that the LUN FC which the disk is related to
does not contains any
s/LUN FC/FC LUN/
Line 12: connections in it, compared to ISCSI lun which must have connections
Line 13: related to it.
Line 14:
Line 15: The proposed fix is to use FCP storage type if the LUN does not contain
Line 8:
Line 9: attach detach and plug disk throws an exception of index out of bounds
Line 10: exception.
Line 11: The reason for that is that the LUN FC which the disk is related to
does not contains any
Line 12: connections in it, compared to ISCSI lun which must have connections
s/compared/as opposed to/
Line 13: related to it.
Line 14:
Line 15: The proposed fix is to use FCP storage type if the LUN does not contain
Line 16: any connections in it.
--
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: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Sharad Mishra <snmis...@linux.vnet.ibm.com>
Gerrit-Reviewer: Vered Volansky <vvola...@redhat.com>
Gerrit-Reviewer: liron aravot <liron.ara...@gmail.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
