iSER patch for VDSM

Roi Dayan roi.dayan at gmail.com
Wed Mar 7 12:08:57 UTC 2012


Hi

I answered inline.

Thanks,
Roi

On Wed, Mar 7, 2012 at 1:33 PM, Dan Kenigsberg <danken at redhat.com> wrote:

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


The patch for getSessionInfo() is to read session info from
/sys/class/iscsi_session and /sys/class/iscsi_connection directories
if could not be found under /sys/devices/platform/host*/ which is how the
function with right now.
Actually from a mail from Mike Christie I saw this morning I understood
that relaying on the /sys/devices/platform/host*/ directory is not right.
Also he gave an example that all offload drivers have pci devs instead of
platform so they wont appear in /sys/devices/platform/.



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


Yes. this is from vdsm.conf under [irq] section:
iscsi_transports = iser,tcp

by default iscsi_transports is tcp only.



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

The patch doesn't touch HeaderDigest anymore. Changing iface to iser is
enough.

If we can tell from the start that we want iser target with do do discovery
with iser iface
but then all targets discovered will be set to iser iface.
Discovery is done over tcp so it's not that the result will be iser targets
only.

The patch modifies addIscsiNode() which is called after discovery.
If the user wanted iser, according to the config file, we change the iface
to iser.
If login fails we change iface back to "iface.name" which currently is
always default (tcp).



>
> Dan
>
>


-- 

*Roi *
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://fedorahosted.org/pipermail/vdsm-patches/attachments/20120307/d454d872/attachment.html>


More information about the vdsm-patches mailing list