iSER patch for VDSM

Dan Kenigsberg danken at redhat.com
Wed Mar 7 11:33:20 UTC 2012


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 at 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



More information about the vdsm-patches mailing list