Re: [Qemu-devel] iSCSI options for IQN with colons

[Markus Armbruster]

Pino Toscano <address@hidden> writes:

> On Tuesday 01 December 2015 10:27:28 Markus Armbruster wrote:
>> Beware, I know next to nothing about iSCSI.
>> 
>> Pino Toscano <address@hidden> writes:
>> 
>> > Hi,
>> >
>> > while testing the integration of QEMU with iSCSI, I was setting up an
>> > environment with both target and initiator IQNs with colons. Then I
>> > tried to connect to two different targets using two different initiator
>> > IQN, like the following:
>> >
>> >   $ qemu ... \
>> >       -iscsi 
>> > id=iqn.2015-11.com.bla:suffix1,initiator-name=iqn.2015-11.com.bla:suffix1-initiator
>> >  \
>> >       -iscsi 
>> > id=iqn.2015-11.com.bla:suffix2,initiator-name=iqn.2015-11.com.bla:suffix2-initiator
>> >  \
>> >       -drive 
>> > file=iscsi://server/iqn.2015-11.com.bla%3Asuffix1/0,format=raw,id=hd1,if=none
>> >  \
>> >       -drive 
>> > file=iscsi://server/iqn.2015-11.com.bla%3Asuffix2/0,format=raw,id=hd2,if=none
>> >  \
>> >       ...
>> >
>> > which didn't work at first:
>> >
>> >   qemu-system-x86_64: -iscsi 
>> > id=iqn.2015-11.com.bla:suffix1,initiator-name=iqn.2015-11.com.bla:suffix1-initiator:
>> >  Parameter 'id' expects an identifier
>> >
>> > which, according to id_wellformed in id.c, is true. Allowing colons in
>> > id=... like in the following patch
>> >
>> > diff --git a/util/id.c b/util/id.c
>> > index bcc64d8..25fca9d 100644
>> > --- a/util/id.c
>> > +++ b/util/id.c
>> > @@ -20,7 +20,7 @@ bool id_wellformed(const char *id)
>> >          return false;
>> >      }
>> >      for (i = 1; id[i]; i++) {
>> > -        if (!qemu_isalnum(id[i]) && !strchr("-._", id[i])) {
>> > +        if (!qemu_isalnum(id[i]) && !strchr("-._:", id[i])) {
>> >              return false;
>> >          }
>> >      }
>> >
>> > allowed me to work run QEMU with the attached disks.
>> >
>> > The question basically boils down to whether it is right to reject
>> > colons in id:
>> > - if so, then there should be a way to allow them only in id of -iscsi
>> >   (since colons can be part of IQNs)
>> > - if not, whether allowing them could cause regressions in option
>> >   parsing
>> 
>> Have you tried
>> 
>>     -iscsi id=iscsi.1,initiator-name=iqn.2015-11.com.bla:suffix1-initiator \
>>     -iscsi id=iscsi.2,initiator-name=iqn.2015-11.com.bla:suffix2-initiator \
>>     -drive
>> file=iscsi://server/iqn.2015-11.com.bla%3Asuffix1/0,format=raw,id=hd1,if=none
>> \
>>     -drive
>> file=iscsi://server/iqn.2015-11.com.bla%3Asuffix2/0,format=raw,id=hd2,if=none
>> \
>
> This won't work, as the various parse_* in iscsi.c (e.g.
> parse_initiator_name for the above cases) use the target IQN as
> identifier for the parameters.

Option parameter "id" is for naming things so that other things can
refer to them.  The actual name should not matter.  If it does, it's a
bug.

>From my largely iSCSI-ignorant point of view, it looks like -drive
file=iscsi *might* use the target IQN it gets from the URL to look up
the matching -iscsi option.  That would be inappropriate.

I'm copying iSCSI maintainers for an authoritative answer.