10:29 a.m.
Hi,
The patch attached created on top of change I3cedeb6c: Implement storage
server connection aliasing
The patch adds a new config option "iscsi_transports" in section "irs"
that
can accept transport list.
E.g. "iser,tcp"
When adding a new iscsi node first vdsm will try iser and then tcp.
The patch was not tested since the build of change I3cedeb6c fails.
/usr/bin/pyflakes vdsm vdsm_cli vds_bootstrap vdsm_reg vdsm_hooks
vdsm/storage/storageServer.py:25: 'Thread' imported but unused
vdsm/storage/storageServer.py:25: 'RLock' imported but unused
vdsm/storage/storageServer.py:25: 'Event' imported but unused
vdsm/storage/storageServer.py:34: 'asyncmethod' imported but unused
vdsm/storage/storageServer.py:34: 'AsyncCallStub' imported but unused
make[2]: *** [check-local] Error 1
make[2]: Leaving directory `/tmp/vdsm'
make[1]: *** [check-am] Error 2
make[1]: Leaving directory `/tmp/vdsm'
make: *** [check-recursive] Error 1
Thanks,
Roi
--
*Roi *
12:51 p.m.
Hi,
I failed with gerrit currently.
For now I attached the patch to this email.
it is output of git format-patch.
Subject: [PATCH] Added iSER support enabled with a configuration option.
When
used VDSM will try to connect with iser and fallback to
default on failure.
Change-Id: I641745d26be06e172f052a9a4626ac26d2607930
Thanks,
Roi
On Wed, Mar 7, 2012 at 5:05 PM, Dan Kenigsberg <danken(a)redhat.com> wrote:
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.
--
*Roi *
8:41 a.m.
Hi,
I wanted to update about the problem we didn't see the luns was because the
disk already had lvm metadata on it
from the tests after cleaning it I saw the lun on discovery.
Thanks,
Roi
On Tue, Mar 6, 2012 at 4:03 PM, Roi Dayan <roi.dayan(a)gmail.com> wrote:
Hi,
I attached a new patch for iser using the iface option.
I also needed to change getSessionInfo() which read session and connection
information
from /sys/devices/platform/host*/ but with iser we don't have this
location (we are checking this).
so the patch also modifies the top of getSessionInfo() to point to the
right session and connection directories.
We still have a problem we didn't figure out yet.
the discovery and login works fine. falling from iser to tcp works fine.
but we dont see any luns for iser targets.
there are no errors in the vdsm.log file.
i see the output of devices in the log file which shows this after
connecting to tcp target and iser target:
{'devList': [{'GUID': '1965edbcc16c00d0f04b00506',
'capacity': '836411326464',
'devtype': 'iSCSI',
'fwrev': '3.14',
'logicalblocksize': '512',
'partitioned': False,
'pathlist': [{'connection': '192.168.20.123',
'initiatorname': 'iser',
'iqn': 'rhev-iser1',
'port': '3260',
'portal': '1'}],
'pathstatus': [{'lun': '1',
'physdev': 'sdd',
'state': 'active',
'type': 'iSCSI'}],
'physicalblocksize': '512',
'productID': 'MR9265-8i',
'pvUUID': 'HrI8Dk-5KcS-Mfnw-gcXu-CRst-EtfQ-8YC3MX',
'serial': 'SLSIMR9265-8i',
'vendorID': 'LSI',
'vgUUID': 'PAa3E1-LcIN-zJjG-s0r4-8ZZE-i1zB-mZYYjM'},
{'GUID': '1977bdbcc1670e70e04b00506',
'capacity': '955898658816',
'devtype': 'iSCSI',
'fwrev': '3.14',
'logicalblocksize': '512',
'partitioned': False,
'pathlist': [{'connection': '192.168.20.123',
'initiatorname': 'default',
'iqn': 'rhev-iscsi2',
'port': '3260',
'portal': '1'}],
'pathstatus': [{'lun': '1',
'physdev': 'sde',
'state': 'active',
'type': 'iSCSI'}],
'physicalblocksize': '512',
'productID': 'MR9265-8i',
'pvUUID': '',
'serial': 'SLSIMR9265-8i',
'vendorID': 'LSI',
'vgUUID': ''}]}
the output of devices seems ok but the gui doesn't show we have any luns
and no errors.
any idea where to look?
Thanks,
Roi
On Tue, Mar 6, 2012 at 12:14 PM, Roi Dayan <roid(a)mellanox.com> wrote:
> Hi,
>
> I changed the patch.
> I'll send it later today after I'll test it.
> In the new patch I'm not doing changes to header digest and I'm using the
> iface option.
>
> I add another binding with:
> iscsiadm -m node -p portal -T target -I iser -o new
>
> in a normal case I usually del the default binding with:
> iscsiadm -m node -p portal -T target -I default -o delete
>
> but for some reason I don’t see it after adding the target with iser
> interface.
> Maybe the reason is VDSM doesn't specify target portal group (tpgt) when
> adding also the default binding.
> e.g.
> Without tpgt:
> iscsiadm -m node -p ip:port -T target -I default -o new
> with tpgt:
> iscsiadm -m node -p ip:port,1 -T target -I iser -o new
>
> When falling back to tcp I delete the iser binding.
>
> Thanks,
> Roi
>
> -----Original Message-----
> From: Dan Kenigsberg [mailto:danken@redhat.com]
> Sent: Tuesday, March 06, 2012 8:58 AM
> To: VDSM Project Patches; Roi Dayan
> Cc: Saggi Mizrahi; Itzik Brown
> Subject: Re: iSER patch for VDSM
>
> On Mon, Mar 05, 2012 at 04:30:26PM +0000, Roi Dayan wrote:
> > Hi,
> >
> > With "the new code supports iser" do you mean the iface iser in
> iscsiadm?
> > I want to use, I don’t want to create a new one.
> > I did a fallback since I don’t have in the gui anywhere to choose
> between iser and iscsi.
> > Now I see it doesn't help me anyway since if I want to use the iser
> > iface in iscsiadm, I need to set this iface when doing discovery since
> it can't be changed later with op update.
> >
> > With a talk with Ayal Baron I was told I can currently submit the
> > patch to vdsm-patches without gerrit And to have a fixme comment about
> the fallback (which doesn't really help right now since I need discovery
> anyway).
> >
> > From you're answer I understand it is possible to choose iser iface
> from the gui?
> > How I do that?
> > I only have fields for ip,port,chap.
>
> I'm afraid you cannot. I'll try to explain Saggi's text, hopefully
> without introducing mistakes of my own: Vdsm's connectStorageServer
> accpets a parameter named initiatorName, which is abused and passed as-is
> to iscsiadm's -I argument.
> This can be abused in order to pass the special "iser" interface to
> iscsiadm.
>
> HOWEVER, I don't think that initiatorName has ever been used by ovirt
> Engine.
>
> Since I have very little knowledge of iscsiadm (and when it comes to iser
> even less), maybe you would agree to give me a more detailed explanation of
> your patch.
>
> 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?
>
> Regards,
> Dan.
> _______________________________________________
> vdsm-patches mailing list
> vdsm-patches(a)lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/vdsm-patches
>
--
*Roi *
--
*Roi *
6:08 a.m.
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/.
>
> thanks,
> Roi
>
> On Wed, Mar 7, 2012 at 12:23 AM, Saggi Mizrahi <smizrahi(a)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 *
4:23 p.m.
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.
----- Original Message -----
From: "Roi Dayan" <roi.dayan(a)gmail.com>
To: "VDSM Project Patches" <vdsm-patches(a)lists.fedorahosted.org>
Cc: "Roni Luxenberg" <rluxenbe(a)redhat.com>, "Itzik Brown"
<ItzikB(a)mellanox.com>
Sent: Tuesday, March 6, 2012 9:03:24 AM
Subject: Re: iSER patch for VDSM
Hi,
I attached a new patch for iser using the iface option.
I also needed to change getSessionInfo() which read session and
connection information
from /sys/devices/platform/host*/ but with iser we don't have this
location (we are checking this).
so the patch also modifies the top of getSessionInfo() to point to
the right session and connection directories.
We still have a problem we didn't figure out yet.
the discovery and login works fine. falling from iser to tcp works
fine.
but we dont see any luns for iser targets.
there are no errors in the vdsm.log file.
i see the output of devices in the log file which shows this after
connecting to tcp target and iser target:
{'devList': [{'GUID': '1965edbcc16c00d0f04b00506',
'capacity': '836411326464',
'devtype': 'iSCSI',
'fwrev': '3.14',
'logicalblocksize': '512',
'partitioned': False,
'pathlist': [{'connection': '192.168.20.123',
'initiatorname': 'iser',
'iqn': 'rhev-iser1',
'port': '3260',
'portal': '1'}],
'pathstatus': [{'lun': '1',
'physdev': 'sdd',
'state': 'active',
'type': 'iSCSI'}],
'physicalblocksize': '512',
'productID': 'MR9265-8i',
'pvUUID': 'HrI8Dk-5KcS-Mfnw-gcXu-CRst-EtfQ-8YC3MX',
'serial': 'SLSIMR9265-8i',
'vendorID': 'LSI',
'vgUUID': 'PAa3E1-LcIN-zJjG-s0r4-8ZZE-i1zB-mZYYjM'},
{'GUID': '1977bdbcc1670e70e04b00506',
'capacity': '955898658816',
'devtype': 'iSCSI',
'fwrev': '3.14',
'logicalblocksize': '512',
'partitioned': False,
'pathlist': [{'connection': '192.168.20.123',
'initiatorname': 'default',
'iqn': 'rhev-iscsi2',
'port': '3260',
'portal': '1'}],
'pathstatus': [{'lun': '1',
'physdev': 'sde',
'state': 'active',
'type': 'iSCSI'}],
'physicalblocksize': '512',
'productID': 'MR9265-8i',
'pvUUID': '',
'serial': 'SLSIMR9265-8i',
'vendorID': 'LSI',
'vgUUID': ''}]}
the output of devices seems ok but the gui doesn't show we have any
luns and no errors.
any idea where to look?
Thanks,
Roi
On Tue, Mar 6, 2012 at 12:14 PM, Roi Dayan < roid(a)mellanox.com >
wrote:
Hi,
I changed the patch.
I'll send it later today after I'll test it.
In the new patch I'm not doing changes to header digest and I'm using
the iface option.
I add another binding with:
iscsiadm -m node -p portal -T target -I iser -o new
in a normal case I usually del the default binding with:
iscsiadm -m node -p portal -T target -I default -o delete
but for some reason I don’t see it after adding the target with iser
interface.
Maybe the reason is VDSM doesn't specify target portal group (tpgt)
when adding also the default binding.
e.g.
Without tpgt:
iscsiadm -m node -p ip:port -T target -I default -o new
with tpgt:
iscsiadm -m node -p ip:port,1 -T target -I iser -o new
When falling back to tcp I delete the iser binding.
Thanks,
Roi
-----Original Message-----
From: Dan Kenigsberg [mailto: danken(a)redhat.com ]
Sent: Tuesday, March 06, 2012 8:58 AM
To: VDSM Project Patches; Roi Dayan
Cc: Saggi Mizrahi; Itzik Brown
Subject: Re: iSER patch for VDSM
On Mon, Mar 05, 2012 at 04:30:26PM +0000, Roi Dayan wrote:
> Hi,
>
> With "the new code supports iser" do you mean the iface iser in
> iscsiadm?
> I want to use, I don’t want to create a new one.
> I did a fallback since I don’t have in the gui anywhere to choose
> between iser and iscsi.
> Now I see it doesn't help me anyway since if I want to use the iser
> iface in iscsiadm, I need to set this iface when doing discovery
> since it can't be changed later with op update.
>
> With a talk with Ayal Baron I was told I can currently submit the
> patch to vdsm-patches without gerrit And to have a fixme comment
> about the fallback (which doesn't really help right now since I
> need discovery anyway).
>
> From you're answer I understand it is possible to choose iser iface
> from the gui?
> How I do that?
> I only have fields for ip,port,chap.
I'm afraid you cannot. I'll try to explain Saggi's text, hopefully
without introducing mistakes of my own: Vdsm's connectStorageServer
accpets a parameter named initiatorName, which is abused and passed
as-is to iscsiadm's -I argument.
This can be abused in order to pass the special "iser" interface to
iscsiadm.
HOWEVER, I don't think that initiatorName has ever been used by ovirt
Engine.
Since I have very little knowledge of iscsiadm (and when it comes to
iser even less), maybe you would agree to give me a more detailed
explanation of your patch.
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?
Regards,
Dan.
_______________________________________________
vdsm-patches mailing list
vdsm-patches(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches
--
Roi
_______________________________________________
vdsm-patches mailing list
vdsm-patches(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches
12:57 a.m.
On Mon, Mar 05, 2012 at 04:30:26PM +0000, Roi Dayan wrote:
Hi,
With "the new code supports iser" do you mean the iface iser in iscsiadm?
I want to use, I don’t want to create a new one.
I did a fallback since I don’t have in the gui anywhere to choose between iser and
iscsi.
Now I see it doesn't help me anyway since if I want to use the iser iface in
iscsiadm,
I need to set this iface when doing discovery since it can't be changed later with op
update.
With a talk with Ayal Baron I was told I can currently submit the patch to vdsm-patches
without gerrit
And to have a fixme comment about the fallback (which doesn't really help right now
since I need discovery anyway).
From you're answer I understand it is possible to choose iser iface from the gui?
How I do that?
I only have fields for ip,port,chap.
I'm afraid you cannot. I'll try to explain Saggi's text, hopefully without
introducing mistakes of my own: Vdsm's connectStorageServer accpets a parameter
named initiatorName, which is abused and passed as-is to iscsiadm's -I argument.
This can be abused in order to pass the special "iser" interface to iscsiadm.
HOWEVER, I don't think that initiatorName has ever been used by ovirt Engine.
Since I have very little knowledge of iscsiadm (and when it comes to iser even
less), maybe you would agree to give me a more detailed explanation of your
patch.
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?
Regards,
Dan.
10:30 a.m.
Hi,
With "the new code supports iser" do you mean the iface iser in iscsiadm?
I want to use, I don’t want to create a new one.
I did a fallback since I don’t have in the gui anywhere to choose between iser and
iscsi.
Now I see it doesn't help me anyway since if I want to use the iser iface in
iscsiadm,
I need to set this iface when doing discovery since it can't be changed later with op
update.
With a talk with Ayal Baron I was told I can currently submit the patch to vdsm-patches
without gerrit
And to have a fixme comment about the fallback (which doesn't really help right now
since I need discovery anyway).
From you're answer I understand it is possible to choose iser iface from the gui?
How I do that?
I only have fields for ip,port,chap.
Thanks,
Roi
-----Original Message-----
From: Saggi Mizrahi [mailto:smizrahi@redhat.com]
Sent: Monday, March 05, 2012 5:33 PM
To: VDSM Project Patches
Cc: Roi Dayan; Itzik Brown
Subject: Re: iSER patch for VDSM
Why isn't this up on gerrit?
Also, the new code supports iser.
Just pass 'iser' as the initiator name.
I know it's confusing but this is how the old interface was implemented.
The new interface calls it interfaceName which is what it should have been called in the
old API.
As I said, I don't like VDSM doing transport fallback, you should manually select the
interface you want to use.
Every host has two default interfaces 'default' which will use tcp and a default
nic and 'iser' which will use iser.
Currently you have no way of adding new iser interface 'I locked new interfaces to
only be TCP' but I can easily change that.
There is no external API for setting new iser interfaces but the new API for creating
interfaces will have discussed on the list with other ovirt projects.
----- Original Message -----
From: "Roi Dayan" <roi.dayan(a)gmail.com>
To: vdsm-patches(a)lists.fedorahosted.org
Cc: "Roi Dayan" <roid(a)mellanox.com>, "Itzik Brown"
<ItzikB(a)mellanox.com>
Sent: Monday, March 5, 2012 2:36:12 AM
Subject: Re: iSER patch for VDSM
Hi,
Attached the patch for the latest update.
Added fixme line for the transport check.
Attached for review.
It's not tested yet. We are installing fedora 17 for it.
Thanks,
Roi
On Thu, Mar 1, 2012 at 6:29 PM, Roi Dayan < roi.dayan(a)gmail.com >
wrote:
Hi,
The patch attached created on top of change I3cedeb6c: Implement
storage server connection aliasing
The patch adds a new config option "iscsi_transports" in section
"irs"
that can accept transport list.
E.g. "iser,tcp"
When adding a new iscsi node first vdsm will try iser and then tcp.
The patch was not tested since the build of change I3cedeb6c fails.
/usr/bin/pyflakes vdsm vdsm_cli vds_bootstrap vdsm_reg vdsm_hooks
vdsm/storage/storageServer.py:25: 'Thread' imported but unused
vdsm/storage/storageServer.py:25: 'RLock' imported but unused
vdsm/storage/storageServer.py:25: 'Event' imported but unused
vdsm/storage/storageServer.py:34: 'asyncmethod' imported but unused
vdsm/storage/storageServer.py:34: 'AsyncCallStub' imported but unused
make[2]: *** [check-local] Error 1
make[2]: Leaving directory `/tmp/vdsm'
make[1]: *** [check-am] Error 2
make[1]: Leaving directory `/tmp/vdsm'
make: *** [check-recursive] Error 1
Thanks,
Roi
--
Roi
--
Roi
_______________________________________________
vdsm-patches mailing list
vdsm-patches(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches
9:33 a.m.
Why isn't this up on gerrit?
Also, the new code supports iser.
Just pass 'iser' as the initiator name.
I know it's confusing but this is how the old interface was implemented.
The new interface calls it interfaceName which is what it should have been called in the
old API.
As I said, I don't like VDSM doing transport fallback, you should manually select the
interface you want to use.
Every host has two default interfaces 'default' which will use tcp and a default
nic and 'iser' which will use iser.
Currently you have no way of adding new iser interface 'I locked new interfaces to
only be TCP' but I can easily change that.
There is no external API for setting new iser interfaces but the new API for creating
interfaces will have discussed on the list with other ovirt projects.
----- Original Message -----
From: "Roi Dayan" <roi.dayan(a)gmail.com>
To: vdsm-patches(a)lists.fedorahosted.org
Cc: "Roi Dayan" <roid(a)mellanox.com>, "Itzik Brown"
<ItzikB(a)mellanox.com>
Sent: Monday, March 5, 2012 2:36:12 AM
Subject: Re: iSER patch for VDSM
Hi,
Attached the patch for the latest update.
Added fixme line for the transport check.
Attached for review.
It's not tested yet. We are installing fedora 17 for it.
Thanks,
Roi
On Thu, Mar 1, 2012 at 6:29 PM, Roi Dayan < roi.dayan(a)gmail.com >
wrote:
Hi,
The patch attached created on top of change I3cedeb6c: Implement
storage server connection aliasing
The patch adds a new config option "iscsi_transports" in section
"irs" that can accept transport list.
E.g. "iser,tcp"
When adding a new iscsi node first vdsm will try iser and then tcp.
The patch was not tested since the build of change I3cedeb6c fails.
/usr/bin/pyflakes vdsm vdsm_cli vds_bootstrap vdsm_reg vdsm_hooks
vdsm/storage/storageServer.py:25: 'Thread' imported but unused
vdsm/storage/storageServer.py:25: 'RLock' imported but unused
vdsm/storage/storageServer.py:25: 'Event' imported but unused
vdsm/storage/storageServer.py:34: 'asyncmethod' imported but unused
vdsm/storage/storageServer.py:34: 'AsyncCallStub' imported but unused
make[2]: *** [check-local] Error 1
make[2]: Leaving directory `/tmp/vdsm'
make[1]: *** [check-am] Error 2
make[1]: Leaving directory `/tmp/vdsm'
make: *** [check-recursive] Error 1
Thanks,
Roi
--
Roi
--
Roi
_______________________________________________
vdsm-patches mailing list
vdsm-patches(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches
9:05 a.m.
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.
8:52 a.m.
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.
> > > >
> > > > 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 *
4:14 a.m.
Hi,
I changed the patch.
I'll send it later today after I'll test it.
In the new patch I'm not doing changes to header digest and I'm using the iface
option.
I add another binding with:
iscsiadm -m node -p portal -T target -I iser -o new
in a normal case I usually del the default binding with:
iscsiadm -m node -p portal -T target -I default -o delete
but for some reason I don’t see it after adding the target with iser interface.
Maybe the reason is VDSM doesn't specify target portal group (tpgt) when adding also
the default binding.
e.g.
Without tpgt:
iscsiadm -m node -p ip:port -T target -I default -o new
with tpgt:
iscsiadm -m node -p ip:port,1 -T target -I iser -o new
When falling back to tcp I delete the iser binding.
Thanks,
Roi
-----Original Message-----
From: Dan Kenigsberg [mailto:danken@redhat.com]
Sent: Tuesday, March 06, 2012 8:58 AM
To: VDSM Project Patches; Roi Dayan
Cc: Saggi Mizrahi; Itzik Brown
Subject: Re: iSER patch for VDSM
On Mon, Mar 05, 2012 at 04:30:26PM +0000, Roi Dayan wrote:
Hi,
With "the new code supports iser" do you mean the iface iser in iscsiadm?
I want to use, I don’t want to create a new one.
I did a fallback since I don’t have in the gui anywhere to choose between iser and
iscsi.
Now I see it doesn't help me anyway since if I want to use the iser
iface in iscsiadm, I need to set this iface when doing discovery since it can't be
changed later with op update.
With a talk with Ayal Baron I was told I can currently submit the
patch to vdsm-patches without gerrit And to have a fixme comment about the fallback
(which doesn't really help right now since I need discovery anyway).
From you're answer I understand it is possible to choose iser iface from the gui?
How I do that?
I only have fields for ip,port,chap.
I'm afraid you cannot. I'll try to explain Saggi's text, hopefully without
introducing mistakes of my own: Vdsm's connectStorageServer accpets a parameter named
initiatorName, which is abused and passed as-is to iscsiadm's -I argument.
This can be abused in order to pass the special "iser" interface to iscsiadm.
HOWEVER, I don't think that initiatorName has ever been used by ovirt Engine.
Since I have very little knowledge of iscsiadm (and when it comes to iser even less),
maybe you would agree to give me a more detailed explanation of your patch.
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?
Regards,
Dan.
1:36 a.m.
Hi,
Attached the patch for the latest update.
Added fixme line for the transport check.
Attached for review.
It's not tested yet. We are installing fedora 17 for it.
Thanks,
Roi
On Thu, Mar 1, 2012 at 6:29 PM, Roi Dayan <roi.dayan(a)gmail.com> wrote:
Hi,
The patch attached created on top of change I3cedeb6c: Implement storage
server connection aliasing
The patch adds a new config option "iscsi_transports" in section
"irs"
that can accept transport list.
E.g. "iser,tcp"
When adding a new iscsi node first vdsm will try iser and then tcp.
The patch was not tested since the build of change I3cedeb6c fails.
/usr/bin/pyflakes vdsm vdsm_cli vds_bootstrap vdsm_reg vdsm_hooks
vdsm/storage/storageServer.py:25: 'Thread' imported but unused
vdsm/storage/storageServer.py:25: 'RLock' imported but unused
vdsm/storage/storageServer.py:25: 'Event' imported but unused
vdsm/storage/storageServer.py:34: 'asyncmethod' imported but unused
vdsm/storage/storageServer.py:34: 'AsyncCallStub' imported but unused
make[2]: *** [check-local] Error 1
make[2]: Leaving directory `/tmp/vdsm'
make[1]: *** [check-am] Error 2
make[1]: Leaving directory `/tmp/vdsm'
make: *** [check-recursive] Error 1
Thanks,
Roi
--
*Roi *
--
*Roi *
11:37 a.m.
The version that is currently up is not the latest batch, I will send a final tested
version today.
In any case I don't think I agree with using a list as part of the internal API.
This creates an ambiguity of the resulting connection.
----- Original Message -----
From: "Roi Dayan" <roi.dayan(a)gmail.com>
To: vdsm-patches(a)lists.fedorahosted.org
Cc: "Roi Dayan" <roid(a)mellanox.com>, "Itzik Brown"
<ItzikB(a)mellanox.com>
Sent: Thursday, March 1, 2012 11:29:29 AM
Subject: iSER patch for VDSM
Hi,
The patch attached created on top of change I3cedeb6c: Implement
storage server connection aliasing
The patch adds a new config option "iscsi_transports" in section
"irs" that can accept transport list.
E.g. "iser,tcp"
When adding a new iscsi node first vdsm will try iser and then tcp.
The patch was not tested since the build of change I3cedeb6c fails.
/usr/bin/pyflakes vdsm vdsm_cli vds_bootstrap vdsm_reg vdsm_hooks
vdsm/storage/storageServer.py:25: 'Thread' imported but unused
vdsm/storage/storageServer.py:25: 'RLock' imported but unused
vdsm/storage/storageServer.py:25: 'Event' imported but unused
vdsm/storage/storageServer.py:34: 'asyncmethod' imported but unused
vdsm/storage/storageServer.py:34: 'AsyncCallStub' imported but unused
make[2]: *** [check-local] Error 1
make[2]: Leaving directory `/tmp/vdsm'
make[1]: *** [check-am] Error 2
make[1]: Leaving directory `/tmp/vdsm'
make: *** [check-recursive] Error 1
Thanks,
Roi
--
Roi
_______________________________________________
vdsm-patches mailing list
vdsm-patches(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches
5:33 a.m.
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(a)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
1:43 a.m.
ok.
attached patch for getSessionInfo() only.
thanks,
Roi
On Wed, Mar 7, 2012 at 12:23 AM, Saggi Mizrahi <smizrahi(a)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.
----- Original Message -----
> From: "Roi Dayan" <roi.dayan(a)gmail.com>
> To: "VDSM Project Patches" <vdsm-patches(a)lists.fedorahosted.org>
> Cc: "Roni Luxenberg" <rluxenbe(a)redhat.com>, "Itzik Brown"
<
ItzikB(a)mellanox.com>
> Sent: Tuesday, March 6, 2012 9:03:24 AM
> Subject: Re: iSER patch for VDSM
>
>
>
> Hi,
>
>
> I attached a new patch for iser using the iface option.
> I also needed to change getSessionInfo() which read session and
> connection information
> from /sys/devices/platform/host*/ but with iser we don't have this
> location (we are checking this).
> so the patch also modifies the top of getSessionInfo() to point to
> the right session and connection directories.
>
>
> We still have a problem we didn't figure out yet.
> the discovery and login works fine. falling from iser to tcp works
> fine.
> but we dont see any luns for iser targets.
> there are no errors in the vdsm.log file.
>
>
> i see the output of devices in the log file which shows this after
> connecting to tcp target and iser target:
>
>
>
> {'devList': [{'GUID': '1965edbcc16c00d0f04b00506',
> 'capacity': '836411326464',
> 'devtype': 'iSCSI',
> 'fwrev': '3.14',
> 'logicalblocksize': '512',
> 'partitioned': False,
> 'pathlist': [{'connection': '192.168.20.123',
> 'initiatorname': 'iser',
> 'iqn': 'rhev-iser1',
> 'port': '3260',
> 'portal': '1'}],
> 'pathstatus': [{'lun': '1',
> 'physdev': 'sdd',
> 'state': 'active',
> 'type': 'iSCSI'}],
> 'physicalblocksize': '512',
> 'productID': 'MR9265-8i',
> 'pvUUID': 'HrI8Dk-5KcS-Mfnw-gcXu-CRst-EtfQ-8YC3MX',
> 'serial': 'SLSIMR9265-8i',
> 'vendorID': 'LSI',
> 'vgUUID': 'PAa3E1-LcIN-zJjG-s0r4-8ZZE-i1zB-mZYYjM'},
> {'GUID': '1977bdbcc1670e70e04b00506',
> 'capacity': '955898658816',
> 'devtype': 'iSCSI',
> 'fwrev': '3.14',
> 'logicalblocksize': '512',
> 'partitioned': False,
> 'pathlist': [{'connection': '192.168.20.123',
> 'initiatorname': 'default',
> 'iqn': 'rhev-iscsi2',
> 'port': '3260',
> 'portal': '1'}],
> 'pathstatus': [{'lun': '1',
> 'physdev': 'sde',
> 'state': 'active',
> 'type': 'iSCSI'}],
> 'physicalblocksize': '512',
> 'productID': 'MR9265-8i',
> 'pvUUID': '',
> 'serial': 'SLSIMR9265-8i',
> 'vendorID': 'LSI',
> 'vgUUID': ''}]}
>
>
> the output of devices seems ok but the gui doesn't show we have any
> luns and no errors.
> any idea where to look?
>
>
>
> Thanks,
> Roi
>
>
> On Tue, Mar 6, 2012 at 12:14 PM, Roi Dayan < roid(a)mellanox.com >
> wrote:
>
>
> Hi,
>
> I changed the patch.
> I'll send it later today after I'll test it.
> In the new patch I'm not doing changes to header digest and I'm using
> the iface option.
>
> I add another binding with:
> iscsiadm -m node -p portal -T target -I iser -o new
>
> in a normal case I usually del the default binding with:
> iscsiadm -m node -p portal -T target -I default -o delete
>
> but for some reason I don’t see it after adding the target with iser
> interface.
> Maybe the reason is VDSM doesn't specify target portal group (tpgt)
> when adding also the default binding.
> e.g.
> Without tpgt:
> iscsiadm -m node -p ip:port -T target -I default -o new
> with tpgt:
> iscsiadm -m node -p ip:port,1 -T target -I iser -o new
>
> When falling back to tcp I delete the iser binding.
>
> Thanks,
> Roi
>
> -----Original Message-----
> From: Dan Kenigsberg [mailto: danken(a)redhat.com ]
> Sent: Tuesday, March 06, 2012 8:58 AM
> To: VDSM Project Patches; Roi Dayan
> Cc: Saggi Mizrahi; Itzik Brown
> Subject: Re: iSER patch for VDSM
>
> On Mon, Mar 05, 2012 at 04:30:26PM +0000, Roi Dayan wrote:
> > Hi,
> >
> > With "the new code supports iser" do you mean the iface iser in
> > iscsiadm?
> > I want to use, I don’t want to create a new one.
> > I did a fallback since I don’t have in the gui anywhere to choose
> > between iser and iscsi.
> > Now I see it doesn't help me anyway since if I want to use the iser
> > iface in iscsiadm, I need to set this iface when doing discovery
> > since it can't be changed later with op update.
> >
> > With a talk with Ayal Baron I was told I can currently submit the
> > patch to vdsm-patches without gerrit And to have a fixme comment
> > about the fallback (which doesn't really help right now since I
> > need discovery anyway).
> >
> > From you're answer I understand it is possible to choose iser iface
> > from the gui?
> > How I do that?
> > I only have fields for ip,port,chap.
>
> I'm afraid you cannot. I'll try to explain Saggi's text, hopefully
> without introducing mistakes of my own: Vdsm's connectStorageServer
> accpets a parameter named initiatorName, which is abused and passed
> as-is to iscsiadm's -I argument.
> This can be abused in order to pass the special "iser" interface to
> iscsiadm.
>
> HOWEVER, I don't think that initiatorName has ever been used by ovirt
> Engine.
>
> Since I have very little knowledge of iscsiadm (and when it comes to
> iser even less), maybe you would agree to give me a more detailed
> explanation of your patch.
>
> 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?
>
> Regards,
> Dan.
> _______________________________________________
> vdsm-patches mailing list
> vdsm-patches(a)lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/vdsm-patches
>
>
>
>
> --
>
>
>
>
> Roi
>
> _______________________________________________
> vdsm-patches mailing list
> vdsm-patches(a)lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/vdsm-patches
>
_______________________________________________
vdsm-patches mailing list
vdsm-patches(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches
--
*Roi *
8:09 a.m.
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
8:03 a.m.
Hi,
I attached a new patch for iser using the iface option.
I also needed to change getSessionInfo() which read session and connection
information
from /sys/devices/platform/host*/ but with iser we don't have this location
(we are checking this).
so the patch also modifies the top of getSessionInfo() to point to the
right session and connection directories.
We still have a problem we didn't figure out yet.
the discovery and login works fine. falling from iser to tcp works fine.
but we dont see any luns for iser targets.
there are no errors in the vdsm.log file.
i see the output of devices in the log file which shows this after
connecting to tcp target and iser target:
{'devList': [{'GUID': '1965edbcc16c00d0f04b00506',
'capacity': '836411326464',
'devtype': 'iSCSI',
'fwrev': '3.14',
'logicalblocksize': '512',
'partitioned': False,
'pathlist': [{'connection': '192.168.20.123',
'initiatorname': 'iser',
'iqn': 'rhev-iser1',
'port': '3260',
'portal': '1'}],
'pathstatus': [{'lun': '1',
'physdev': 'sdd',
'state': 'active',
'type': 'iSCSI'}],
'physicalblocksize': '512',
'productID': 'MR9265-8i',
'pvUUID': 'HrI8Dk-5KcS-Mfnw-gcXu-CRst-EtfQ-8YC3MX',
'serial': 'SLSIMR9265-8i',
'vendorID': 'LSI',
'vgUUID': 'PAa3E1-LcIN-zJjG-s0r4-8ZZE-i1zB-mZYYjM'},
{'GUID': '1977bdbcc1670e70e04b00506',
'capacity': '955898658816',
'devtype': 'iSCSI',
'fwrev': '3.14',
'logicalblocksize': '512',
'partitioned': False,
'pathlist': [{'connection': '192.168.20.123',
'initiatorname': 'default',
'iqn': 'rhev-iscsi2',
'port': '3260',
'portal': '1'}],
'pathstatus': [{'lun': '1',
'physdev': 'sde',
'state': 'active',
'type': 'iSCSI'}],
'physicalblocksize': '512',
'productID': 'MR9265-8i',
'pvUUID': '',
'serial': 'SLSIMR9265-8i',
'vendorID': 'LSI',
'vgUUID': ''}]}
the output of devices seems ok but the gui doesn't show we have any luns
and no errors.
any idea where to look?
Thanks,
Roi
On Tue, Mar 6, 2012 at 12:14 PM, Roi Dayan <roid(a)mellanox.com> wrote:
Hi,
I changed the patch.
I'll send it later today after I'll test it.
In the new patch I'm not doing changes to header digest and I'm using the
iface option.
I add another binding with:
iscsiadm -m node -p portal -T target -I iser -o new
in a normal case I usually del the default binding with:
iscsiadm -m node -p portal -T target -I default -o delete
but for some reason I don’t see it after adding the target with iser
interface.
Maybe the reason is VDSM doesn't specify target portal group (tpgt) when
adding also the default binding.
e.g.
Without tpgt:
iscsiadm -m node -p ip:port -T target -I default -o new
with tpgt:
iscsiadm -m node -p ip:port,1 -T target -I iser -o new
When falling back to tcp I delete the iser binding.
Thanks,
Roi
-----Original Message-----
From: Dan Kenigsberg [mailto:danken@redhat.com]
Sent: Tuesday, March 06, 2012 8:58 AM
To: VDSM Project Patches; Roi Dayan
Cc: Saggi Mizrahi; Itzik Brown
Subject: Re: iSER patch for VDSM
On Mon, Mar 05, 2012 at 04:30:26PM +0000, Roi Dayan wrote:
> Hi,
>
> With "the new code supports iser" do you mean the iface iser in iscsiadm?
> I want to use, I don’t want to create a new one.
> I did a fallback since I don’t have in the gui anywhere to choose
between iser and iscsi.
> Now I see it doesn't help me anyway since if I want to use the iser
> iface in iscsiadm, I need to set this iface when doing discovery since
it can't be changed later with op update.
>
> With a talk with Ayal Baron I was told I can currently submit the
> patch to vdsm-patches without gerrit And to have a fixme comment about
the fallback (which doesn't really help right now since I need discovery
anyway).
>
> From you're answer I understand it is possible to choose iser iface from
the gui?
> How I do that?
> I only have fields for ip,port,chap.
I'm afraid you cannot. I'll try to explain Saggi's text, hopefully without
introducing mistakes of my own: Vdsm's connectStorageServer accpets a
parameter named initiatorName, which is abused and passed as-is to
iscsiadm's -I argument.
This can be abused in order to pass the special "iser" interface to
iscsiadm.
HOWEVER, I don't think that initiatorName has ever been used by ovirt
Engine.
Since I have very little knowledge of iscsiadm (and when it comes to iser
even less), maybe you would agree to give me a more detailed explanation of
your patch.
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?
Regards,
Dan.
_______________________________________________
vdsm-patches mailing list
vdsm-patches(a)lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches
--
*Roi *
4470
days inactive
4476
days old
vdsm-patches@lists.fedorahosted.org
16 comments
4 participants
participants (4)
-
Dan Kenigsberg -
Roi Dayan -
Roi Dayan -
smizrahi＠redhat.com