On Wed, Mar 07, 2012 at 04:52:49PM +0200, Roi Dayan wrote:
On Wed, Mar 7, 2012 at 4:09 PM, Dan Kenigsberg
<danken(a)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(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?
>
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.
Ok. I've posted it in
http://gerrit.ovirt.org/2628
for you. Please review, ack, comment, there.
In the future, if you send a patch to the list, better make it the output of
git-format-patches. Otherwise, there's a lot of guesswork left for me.