On 11/06/2014 09:35 AM, Turbo Fredriksson wrote:
2014-11-06 09:18 skrev Jerome Martin:
Note that you might want to have a To: and/or Reply-to: header in the
emails you send
Ok, thanx. This was the first time I used git format-patch and
send-email. I usually
do all my pull requests etc via GitHub...
Thanks for trying to do it the "right way" :-)
For the targetcli stack, github pull requests are fine though, as this
code is maintained on github/datera, not in kernel repos.
It does not "nullify" anything, it merely does not preserve the
initial formatting.
Isn't that 'nullifying'? Enforcing/creating a lowercased only from
something that shouldn't
be considered?
Again, according to RFC 3270, a lowercase IQN is equivalent to any
uppercase/mixed-case version of it. Thus lowercasing an IQN, according
to the RFC, is not changing/removing any information. The only thing
that gets affected is the cosmetic aspect.
Since I do not change the case in my application, I can no longer use
the lio-utils commands
(lio-node is the only one I've tested so far). It croaks when trying to
do anything, because
it can't find the lower cased IQN dir...
First of all, one of the reasons why I do not want to change code in
lio-utils is that it is EOL. As of targetcli 3.x series, lio-utils is
deprecated and no longer used, all configuration save/restore is done
natively. You should really not be using lio-utils, as it will not be
maintained to support future kernel-side features.
The better approach is to use rtslib, which has a far simpler and
superior API in any case. If you cannot use a python API for your
application, your have two options. Either use the new configuration
format in 3.x series and load/commit configuration snippets from the
shell, or invoke targetcli directly, as you did with lio-utils. There
might be some friction for the last two cases just now, but it is high
on my priority list to enable that, so I'll assist you if needed and
provide fixes and features needed to enable this.
Second, you'll notice that since the 3.x targetcli/rtslib series does
not use lio-utils anymore, it does not effectively lowercase the IQN
names. I am not sure just yet if this will last or not, but I will
probably enforce lowercase too soon. Apart from the historical reasons
behing lio-utils doing so (the problematic initiators case), this is a
convention that avoids potential issues at the configfs layer. The
configfs control layer _is_ case-sensitive somehow. Aka, you can create
both iqn.abc and iqn.ABC configfs nodes, which makes no sense are both
should be the same according to the RFC. So doing the lowercase
conversion in userspace is a convention to avoid ending up in that kind
of situation.
I have to defer to Nic the question of what happens when we have two
configfs nodes with similar RFC 3270 names but different letter cases,
as I have not looked into it myself.
RFC3720 states IQN is _not_ case sensitive.
And yet 'you' MAKES it case sensitive (by changing it)...
No. I disagree strongly.
Making it case-sensitive would mean treating iqn.abc and iqn.ABC
differently. In effect, what lio-utils does (which rtslib 3.x should and
will probably do soon) is ENFORCE case-insensitiveness, by normalizing
IQN value to a canonical representation, in order for iqn.abc and
iqn.ABC to always be treated the SAME way.
The decision to normalize to lower-case only was taken because of
non-RFC 3720
initiators that were spotted in the wild, causing issues: these were
choking up
when the IQN contained mixed case.
Please note the additional reason given above re. configfs.
That's bad news... I might have to reconsider my own app here..
If your app behaves differently when fed with iqn.abc and iqn.ABC, it
means that _your_ app is case-sensitive and non RFC compliant. So, yes,
you should reconsider. Also, I encourage you to use the same convention
lio-utils uses, for reasons explained above, and convert internaly to
lower-case. Depending on your application, you might want to expose the
original mixed-case value to your users and/or API though, this is up to
you. But really, if using lio-utils or rtslib it should not matter at
all, unless you are not storing the original user/API provided
mixed-case value and still want to expose it. In which case I am afraid
you'll need some persistence layer of your own. Again, if your
application chokes on iqn.abc because it expected iqn.ABC means it _is_
case sensitive, hence non-RFC compliant. All your IQN equality tests
should look like:
if IQN1.lower() == IQN2_lower()....
or
if IQN1.upper() == IQN2_lower()...
or even:
if IQN1.some_deterministic_camel_casing_method() ==
IQN2_some_deterministic_camel_casing_method()...
If you are testing IQN equality without normalizing the case, you ARE by
definition being case sensitive (and broken).
How about making this configurable? Say, setting a variable
LIO_USE_LOWER_CASE=0
(or whatever :) and if that's set, then don't lower case everything? If
you want to
mainain the current behaviour. I would argue to do it the other way
around (conform
to the RFC first, and if someone doesn't want/like that - because of
broken initiators,
then change the case to work with them).
Again, I fail to see how what lio-utils does does not confirm to the
RFC. It is expecting any specific letter case that is being
case-sensitive and non-RFC compliant. See above.
I think it was a windows initiator problem at the time, but am not
100% positive.
Why am I not surprised!? :)
Remove these, to conform to the RFC.
I fail to see how using an equivalent notation for those IQNs
(according to the RFC) is non conforming to the RFC.
Because if an (my) application conforms to the RFC (and allows mixed
case IQNs),
the lio-utils commands doesn't work (because of the mixed case)...
Hence, it suddenly
IS/becomes case sensitive...
Again, if your application fails to recognize that iqn.abc and iqn.ABC
are not the same as far as RFC is concerned, then it is _your_
application that is case-sensitive and broken. Sorry to drive that nail
so many time in this reply, I do not mean to bash you or anything, it
just comes up many times in this email...
The two important take-aways are these:
1) Do not create code using lio-utils, it is now legacy code and will
not be maintained for new features. Use rtslib/targetcli instead, and do
not hesitate to ask me to add features or better CLI scripting support
if you need it - I'll gladly do.
2) If you want to conform to the RFC and be case insensitive, it means
that you must _not_ expect anything regarding IQN case and consider
iqn.abc and iqn.ABC to be the same. Regarding the specific way this is
implemented in userspace currently. use the lowercase convention as that
ensures that we do not get name clashes at the configfs level (i.e. both
iqn.abc and iqn.ABC existing in configfs at the same time).
Kind Regards,
--
Jerome Martin
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
