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(a)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?
> > >
> > > 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
>
> 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.
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.
Regards,
Dan