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

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. [...]