Maor Lipchuk has posted comments on this change.

Change subject: core: Fix IndexOutOfBoundsException in plug FC disk
......................................................................


Patch Set 7: (1 inline comment)

> In addition if you are assuming that the connection list is empty, you should 
> look that all the relevant code (detach?) supports an empty list too.
 A pre integration ticket has been opened to address this scenario 
https://engineering.redhat.com/trac/preint/ticket/823

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java
Line 77:      */
Line 78:     protected StorageType getLUNStorageType(LUNs lun) {
Line 79:         return lun.getLunConnections().isEmpty() ? StorageType.FCP : 
lun.getLunConnections().get(0).getstorage_type();
Line 80:     }
Line 81: 
I'm aware of that, as I replied earlier this is a fix for the NPE.

> lun.getLunConnections().get(0).getstorage_type() assume that all LUN 
> connections are of the same type. This is false. The same LUN can be exposed 
> by different transports. 
 Again, You are describing an RFE in the engine, I will address to it in 
another patch
> The empty connection list = FCP inference is False. A local LUN has no 
> connections too. Before plugging the LUN the host should be connected to the 
> storage server using the proper type.
 See earlier patch which I replied you on that comment, I asked you and you 
didn't replied. 
When do we use local lun in the engine? I didn't see that we do? This can be 
addressed in another patch
> if there are no connections there is no need for an explicit connection 
> Assuming FCP is wrong and unnecessary at all.
 Again, this will be addressed in another patch.
Line 82:     /**
Line 83:      * Sets the LUN connection list from the DB.
Line 84:      *
Line 85:      * @param lun


--
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: 7
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: Eduardo <ewars...@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

Reply via email to