[libvirt] [PATCH 2/2] qemu: Escape commas for qemuBuildSCSIiSCSIHostdevDrvStr

[John Ferlan]

On 06/22/2018 09:06 AM, Anya Harter wrote:
[...]

>>
>> Something is not quite right about seeing "example,foo.org" as a host
>> name. I don't see that as a valid host name regardless of comma
>> escaping. Do we escape commas in other "<host name=%s...>" string
>> fields? I don't see it being done for qemuBuildChrChardevStr (search on
>> TCP or "host.*name" in qemu_command.c)>
>> Now the one thing that perhaps *could* be escaped is the <source> name
>> attribute. Currently it's set to "iqn.1992-01.com.example", but the spec
>> defines a structure of that name field to have a field that is a "String
>> defined by the naming authority" (shorter version, see
>> https://en.wikipedia.org/wiki/ISCSI)
>>
>> So if the name is : 'iqn.1992-01.com.example:storage/2', then the
>> "storage/2" piece is the string defined by the naming authority. Not
>> that I know whether it would work, but that's where I suppose a comma
>> could go.
> 
> If you could give me a string with a comma in it, I would be happy to
> put it into the test instead of in the host name field. However, if no
> one can think of a realistic case where comma escaping would be
> necessary, we can just remove the test altogether. I only put it in as a
> test that implementation was correct and that the commas were actually
> being escaped correctly.
> 

Sorry for the delay - I stuck my head into the checkpoint/backup
quicksand and it's been "difficult" to remove it ;-)

So going back to your analysis from the previous series:

https://www.redhat.com/archives/libvir-list/2018-June/msg01467.html

Now I see where/why you got here. The myopia I have is that host->name
shouldn't have a comma in it as it's an invalid address format.

So, rather than:

  <host name='example,foo.org' port='3260'/>

if I instead use:

  <source protocol='iscsi' name='iqn.1992-01.com.example:my,storage/1'>

then with a couple of adjustments to input:

-    <controller type='scsi' index='0'/>
+    <controller type='scsi' index='0' model='virtio-scsi'/>

...

-      <source protocol='iscsi' name='iqn.1992-01.com.example'>
-        <host name='example,foo.org' port='3260'/>
+        <source protocol='iscsi' name='iqn.1992-01.com.example:my,storage/1'>
+        <host name='example.foo.org' port='3260'/>

qemuxml2argvtest.c:

-            QEMU_CAPS_SCSI_LSI,

and output:

--device lsi,id=scsi0,bus=pci.0,addr=0x3 \
+-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \

...

--drive file=iscsi://example,,foo.org:3260/iqn.1992-01.com.example/0,if=none,\
-format=raw,id=drive-hostdev0 \
--device scsi-generic,bus=scsi0.0,scsi-id=4,drive=drive-hostdev0,id=hostdev0 \
+-drive file=iscsi://example.foo.org:3260/iqn.1992-01.com.example%3Amy,,\
+storage/1,if=none,format=raw,id=drive-hostdev0 \
+-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=0,lun=4,\
+drive=drive-hostdev0,id=hostdev0 \

Then, I think we cover this oddball case.

I can make those changes for you before pushing as long as you
agree that's fine. In the long run, it's similar, it's just avoiding
the invalid hostname/uri string.

John

and yes, once this is pushed, that item comes off the wiki list.

[...]