iSER patch for VDSM

Roi Dayan roi.dayan at gmail.com
Wed Mar 7 14:52:49 UTC 2012


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

> On Wed, Mar 07, 2012 at 02:08:57PM +0200, Roi Dayan wrote:
> > 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/.
>
> So, you say that a more intrusive patch is needed? Or your current version
> is an
> acceptable step forward?
>


yes. we can just skip checking the platform directory since even if a
platform exists for a session
it will exists in /sys/class/iscsi_session/ anyway.

I also saw now that the function iterateIscsiSessions() also checking
platform to know which sessions
exists.
I edited it to work with /sys/class/iscsi_session/ instead.

I attached an updated patch with the above.
its actually cleaner after removing the platform directory glob stuff.



>
> > > > >
> > > > > 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 this is from your old, obsoleted patch, right?
> We are going to have a slightly different patch (this time to hsm.py)
> I think a clearer name would be be
>
> iscsi_default_ifaces = default,iser
>

ok. I'll change that.


>
> > >
> > > 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.
>
> Which patch? Is there anything beyond the getSessionInfo patch?
>
> >
> > 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.
>
> I still do not understand if it is possible/beneficial to use
>  iscsiadm -m discoverydb -t st -p ip:port -I iser -I default --discover
> in your use case.
>


Doing this discovery will just create a target with 2 iface
configurations. one for tcp and one for iser.
I still need to do a login over iser and see if it fails to know if the
target is not iser.
So if i want to do this during the discovery process I need per target to
do a login
and if it fails to set it default iface and if not to do logout and set it
iser iface.



>
> >
> > 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).
>
> that's the old patch, right? when you can, please provide a patch with
> logic
> moved to hsm.py.
>


I'm not sure where you want me to set it on hsm.py.
Should it be in connectStorageServer() ? it's marked deprecated.

If I want after discovery to check if target should be iser or default (
..discoverSendTargets().. )
I'll have to do a login and logout per target as I described above.




>
> Regards,
> Dan
>
>
>
>


-- 

*Roi *
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://fedorahosted.org/pipermail/vdsm-patches/attachments/20120307/e30030e9/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vdsm-getsessioninfo-1331130748.patch
Type: application/octet-stream
Size: 2034 bytes
Desc: not available
URL: <https://fedorahosted.org/pipermail/vdsm-patches/attachments/20120307/e30030e9/attachment-0001.obj>


More information about the vdsm-patches mailing list