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 <smizrahi(a)redhat.com> 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