On Wed, Mar 07, 2012 at 09:43:27AM +0200, Roi Dayan wrote: > ok. > > attached patch for getSessionInfo() only.
Roi, It's nice that you've reached agreement with Saggi, but would you please give the laymen a short description of this patch? It seems unrelated to iser. > > thanks, > Roi > > On Wed, Mar 7, 2012 at 12:23 AM, Saggi Mizrahi <[email protected]> wrote: > > > Hi, first of all I'm truly sorry about sending you all back and forth and > > you have been most patient. > > > > Secondly I would like you to split your patch to 2 parts. > > The getSessionInfo() changes are good and I would have ACKed them if they > > where posted on their own. > > > > Again, discovery will use the interface passed in iface. There is no need > > to do all the looping and there is no need for the config values. Accepting > > your patch means that the transport in the interface given in the parameter > > is ignored. > > > > I do know that there is no way to currently select iser in the UI and > > backend. > > This, although unfortunate, does not mean we need to start hacking things > > in to VDSM. > > My problem with accepting this change is that it is now supported by VDSMs > > interface. > > This will make things hard for us to support in the future. > > > > I completely sympathize with your strong will to not start poking in the > > Engine and UI. > > > > In any case, there is a way I can meet you half way. I would rather it'd > > be fixed properly but I do get that time constraints might force us to fix > > this in a less than optimal manner. > > > > The fix as I see it: > > In HSM when there is no initiator name passed, hsm will not only try > > iface('default') but also iface('iser'). > > Doing it this way will mean that the fallback is only supported by the old > > API and is implemented at the API level and not the iscsi abstraction layer. > > I would have done it myself and sent the patch if I had a set up to test > > it in. > > > > Note that you will need the OK of someone in the ovirt management to > > approve this API semantic change and it's supportability. I'm just an > > annoying developer, I don't have veto powers. If this is enabled via a config item, it would be easily acceptible. But please note my unanswered questions: > > > A casual read of the man page makes we wonder: could a simple > > > > > > iscsiadm -m discoverydb -t st -p ip:port -I iser --discover > > > > > > do the trick for discovery? > > > Could HeaderDigest mangling be avoided? > > > > > > BTW, Saggi, does it make sense to have our API accomodate > > > discovery/login over multiple interfaces? iscsi.discoverSendTargets() could accept a *list* of interfaces, so that both -I iser and -I default are used. Dan _______________________________________________ vdsm-patches mailing list [email protected] https://fedorahosted.org/mailman/listinfo/vdsm-patches
