8:16 a.m.
In order to use the virQEMUBuildBufferEscapeComma function to escape the
commas, the function first had to be converted to use a buffer.
Added a test case for comma escaping in name-escape.xml.
This should complete the BiteSizedTask entry at
https://wiki.libvirt.org/page/BiteSizedTasks#qemu:_Use_comma_escaping_for....
Anya Harter (2):
qemu: use virBuffer in qemuBuildSCSIiSCSIHostdevDrvStr
qemu: Escape commas for qemuBuildSCSIiSCSIHostdevDrvStr
src/qemu/qemu_command.c | 19 +++++++++++++------
tests/qemuxml2argvdata/name-escape.args | 6 +++++-
tests/qemuxml2argvdata/name-escape.xml | 7 +++++++
tests/qemuxml2argvtest.c | 5 ++++-
4 files changed, 29 insertions(+), 8 deletions(-)
--
2.17.1
8:16 a.m.
New subject: [libvirt] [PATCH 1/2] qemu: use virBuffer in qemuBuildSCSIiSCSIHostdevDrvStr
Instead of source to enable use of virBuffer functions in
string construction.
Signed-off-by: Anya Harter <aharter(a)redhat.com>
---
src/qemu/qemu_command.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e05b106a5e..a99240992a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4657,7 +4657,8 @@ static char *
qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
virQEMUCapsPtr qemuCaps)
{
- char *source = NULL;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
char *netsource = NULL;
virJSONValuePtr srcprops = NULL;
virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
@@ -4670,21 +4671,25 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
goto cleanup;
if (!(netsource = virQEMUBuildDriveCommandlineFromJSON(srcprops)))
goto cleanup;
- if (virAsprintf(&source, "%s,if=none,format=raw", netsource) <
0)
- goto cleanup;
+ virBufferAsprintf(&buf, "%s,if=none,format=raw", netsource);
} else {
/* Rather than pull what we think we want - use the network disk code */
if (!(netsource = qemuBuildNetworkDriveStr(iscsisrc->src, srcPriv ?
srcPriv->secinfo : NULL)))
goto cleanup;
- if (virAsprintf(&source, "file=%s,if=none,format=raw", netsource)
< 0)
- goto cleanup;
+ virBufferAsprintf(&buf, "file=%s,if=none,format=raw", netsource);
}
+ if (virBufferCheckError(&buf) < 0)
+ goto cleanup;
+
+ return virBufferContentAndReset(&buf);
+
cleanup:
VIR_FREE(netsource);
virJSONValueFree(srcprops);
- return source;
+ virBufferFreeAndReset(&buf);
+ return NULL;
}
char *
--
2.17.1
8:17 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Escape commas for qemuBuildSCSIiSCSIHostdevDrvStr
Add comma escaping for netsource
Signed-off-by: Anya Harter <aharter(a)redhat.com>
---
src/qemu/qemu_command.c | 4 +++-
tests/qemuxml2argvdata/name-escape.args | 6 +++++-
tests/qemuxml2argvdata/name-escape.xml | 7 +++++++
tests/qemuxml2argvtest.c | 5 ++++-
4 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a99240992a..96c6c08c2d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4677,7 +4677,9 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
if (!(netsource = qemuBuildNetworkDriveStr(iscsisrc->src, srcPriv ?
srcPriv->secinfo : NULL)))
goto cleanup;
- virBufferAsprintf(&buf, "file=%s,if=none,format=raw", netsource);
+ virBufferAddLit(&buf, "file=");
+ virQEMUBuildBufferEscapeComma(&buf, netsource);
+ virBufferAddLit(&buf, ",if=none,format=raw");
}
if (virBufferCheckError(&buf) < 0)
diff --git a/tests/qemuxml2argvdata/name-escape.args
b/tests/qemuxml2argvdata/name-escape.args
index aef7c238ca..1cbb1efd66 100644
--- a/tests/qemuxml2argvdata/name-escape.args
+++ b/tests/qemuxml2argvdata/name-escape.args
@@ -22,6 +22,7 @@ bar=2/monitor.sock,server,nowait \
-no-shutdown \
-no-acpi \
-boot c \
+-device lsi,id=scsi0,bus=pci.0,addr=0x3 \
-device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-usb \
-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\
@@ -41,4 +42,7 @@ cert3=cert3,db=/etc/pki/nssdb,,foo,id=smartcard0,bus=ccid0.0 \
-spice unix,addr=/tmp/lib/domain--1-foo=1,,bar=2/spice.sock,gl=on,\
rendernode=/dev/dri/foo,,bar \
-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \
--device virtio-balloon-pci,id=balloon0,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 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/name-escape.xml
b/tests/qemuxml2argvdata/name-escape.xml
index 70a1ce09d3..24dd248842 100644
--- a/tests/qemuxml2argvdata/name-escape.xml
+++ b/tests/qemuxml2argvdata/name-escape.xml
@@ -27,6 +27,13 @@
</iotune>
<address type='drive' controller='0' bus='0'
target='0' unit='0'/>
</disk>
+ <controller type='scsi' index='0'/>
+ <hostdev mode='subsystem' type='scsi' managed='yes'>
+ <source protocol='iscsi' name='iqn.1992-01.com.example'>
+ <host name='example,foo.org' port='3260'/>
+ </source>
+ <address type='drive' controller='0' bus='0'
target='0' unit='4'/>
+ </hostdev>
<graphics type='vnc'>
<listen type='socket'/>
</graphics>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 582a9de7bb..2e891a0bea 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2772,7 +2772,10 @@ mymain(void)
QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_DEVICE_ISA_SERIAL,
QEMU_CAPS_CHARDEV_FILE_APPEND,
- QEMU_CAPS_CCID_EMULATED);
+ QEMU_CAPS_CCID_EMULATED,
+ QEMU_CAPS_VIRTIO_SCSI,
+ QEMU_CAPS_SCSI_LSI,
+ QEMU_CAPS_DEVICE_SCSI_GENERIC);
DO_TEST("debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS);
DO_TEST("master-key", QEMU_CAPS_OBJECT_SECRET);
--
2.17.1
5:55 p.m.
New subject: [libvirt] [PATCH 2/2] qemu: Escape commas for qemuBuildSCSIiSCSIHostdevDrvStr
On 06/20/2018 09:17 AM, Anya Harter wrote:
Add comma escaping for netsource
Signed-off-by: Anya Harter <aharter(a)redhat.com>
---
src/qemu/qemu_command.c | 4 +++-
tests/qemuxml2argvdata/name-escape.args | 6 +++++-
tests/qemuxml2argvdata/name-escape.xml | 7 +++++++
tests/qemuxml2argvtest.c | 5 ++++-
4 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a99240992a..96c6c08c2d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4677,7 +4677,9 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
if (!(netsource = qemuBuildNetworkDriveStr(iscsisrc->src, srcPriv ?
srcPriv->secinfo : NULL)))
goto cleanup;
- virBufferAsprintf(&buf, "file=%s,if=none,format=raw", netsource);
+ virBufferAddLit(&buf, "file=");
+ virQEMUBuildBufferEscapeComma(&buf, netsource);
+ virBufferAddLit(&buf, ",if=none,format=raw");
}
if (virBufferCheckError(&buf) < 0)
diff --git a/tests/qemuxml2argvdata/name-escape.args
b/tests/qemuxml2argvdata/name-escape.args
index aef7c238ca..1cbb1efd66 100644
--- a/tests/qemuxml2argvdata/name-escape.args
+++ b/tests/qemuxml2argvdata/name-escape.args
@@ -22,6 +22,7 @@ bar=2/monitor.sock,server,nowait \
-no-shutdown \
-no-acpi \
-boot c \
+-device lsi,id=scsi0,bus=pci.0,addr=0x3 \
[1]
-device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-usb \
-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\
@@ -41,4 +42,7 @@ cert3=cert3,db=/etc/pki/nssdb,,foo,id=smartcard0,bus=ccid0.0 \
-spice unix,addr=/tmp/lib/domain--1-foo=1,,bar=2/spice.sock,gl=on,\
rendernode=/dev/dri/foo,,bar \
-device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \
--device virtio-balloon-pci,id=balloon0,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 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/name-escape.xml
b/tests/qemuxml2argvdata/name-escape.xml
index 70a1ce09d3..24dd248842 100644
--- a/tests/qemuxml2argvdata/name-escape.xml
+++ b/tests/qemuxml2argvdata/name-escape.xml
@@ -27,6 +27,13 @@
</iotune>
<address type='drive' controller='0' bus='0'
target='0' unit='0'/>
</disk>
+ <controller type='scsi' index='0'/>
[1] If you change this to:
<controller type='scsi' index='0' model='virtio-scsi'/>
Then the lsi doesn't show up...
+ <hostdev mode='subsystem' type='scsi'
managed='yes'>
+ <source protocol='iscsi' name='iqn.1992-01.com.example'>
+ <host name='example,foo.org' port='3260'/>
+ </source>
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.
+ <address type='drive' controller='0'
bus='0' target='0' unit='4'/>
+ </hostdev>
<graphics type='vnc'>
<listen type='socket'/>
</graphics>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 582a9de7bb..2e891a0bea 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2772,7 +2772,10 @@ mymain(void)
QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_DEVICE_ISA_SERIAL,
QEMU_CAPS_CHARDEV_FILE_APPEND,
- QEMU_CAPS_CCID_EMULATED);
+ QEMU_CAPS_CCID_EMULATED,
+ QEMU_CAPS_VIRTIO_SCSI,
+ QEMU_CAPS_SCSI_LSI,
[1] and the LSI wouldn't be needed here.
John
first patch looks fine, but I'm not so convinced on this one only
because of the host name w/ a comma.
+ QEMU_CAPS_DEVICE_SCSI_GENERIC);
DO_TEST("debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS);
DO_TEST("master-key", QEMU_CAPS_OBJECT_SECRET);
8:06 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Escape commas for qemuBuildSCSIiSCSIHostdevDrvStr
On 06/21/2018 06:55 PM, John Ferlan wrote:
On 06/20/2018 09:17 AM, Anya Harter wrote:
> Add comma escaping for netsource
>
> Signed-off-by: Anya Harter <aharter(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 4 +++-
> tests/qemuxml2argvdata/name-escape.args | 6 +++++-
> tests/qemuxml2argvdata/name-escape.xml | 7 +++++++
> tests/qemuxml2argvtest.c | 5 ++++-
> 4 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index a99240992a..96c6c08c2d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4677,7 +4677,9 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
> if (!(netsource = qemuBuildNetworkDriveStr(iscsisrc->src, srcPriv ?
> srcPriv->secinfo : NULL)))
> goto cleanup;
> - virBufferAsprintf(&buf, "file=%s,if=none,format=raw",
netsource);
> + virBufferAddLit(&buf, "file=");
> + virQEMUBuildBufferEscapeComma(&buf, netsource);
> + virBufferAddLit(&buf, ",if=none,format=raw");
> }
>
> if (virBufferCheckError(&buf) < 0)
> diff --git a/tests/qemuxml2argvdata/name-escape.args
b/tests/qemuxml2argvdata/name-escape.args
> index aef7c238ca..1cbb1efd66 100644
> --- a/tests/qemuxml2argvdata/name-escape.args
> +++ b/tests/qemuxml2argvdata/name-escape.args
> @@ -22,6 +22,7 @@ bar=2/monitor.sock,server,nowait \
> -no-shutdown \
> -no-acpi \
> -boot c \
> +-device lsi,id=scsi0,bus=pci.0,addr=0x3 \
[1]
> -device usb-ccid,id=ccid0,bus=usb.0,port=1 \
> -usb \
> -drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\
> @@ -41,4 +42,7 @@ cert3=cert3,db=/etc/pki/nssdb,,foo,id=smartcard0,bus=ccid0.0 \
> -spice unix,addr=/tmp/lib/domain--1-foo=1,,bar=2/spice.sock,gl=on,\
> rendernode=/dev/dri/foo,,bar \
> -device cirrus-vga,id=video0,bus=pci.0,addr=0x2 \
> --device virtio-balloon-pci,id=balloon0,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 \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
> diff --git a/tests/qemuxml2argvdata/name-escape.xml
b/tests/qemuxml2argvdata/name-escape.xml
> index 70a1ce09d3..24dd248842 100644
> --- a/tests/qemuxml2argvdata/name-escape.xml
> +++ b/tests/qemuxml2argvdata/name-escape.xml
> @@ -27,6 +27,13 @@
> </iotune>
> <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
> </disk>
> + <controller type='scsi' index='0'/>
[1] If you change this to:
<controller type='scsi' index='0' model='virtio-scsi'/>
Then the lsi doesn't show up...
> + <hostdev mode='subsystem' type='scsi'
managed='yes'>
> + <source protocol='iscsi'
name='iqn.1992-01.com.example'>
> + <host name='example,foo.org' port='3260'/>
> + </source>
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.
> + <address type='drive' controller='0' bus='0'
target='0' unit='4'/>
> + </hostdev>
> <graphics type='vnc'>
> <listen type='socket'/>
> </graphics>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 582a9de7bb..2e891a0bea 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -2772,7 +2772,10 @@ mymain(void)
> QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
> QEMU_CAPS_DEVICE_ISA_SERIAL,
> QEMU_CAPS_CHARDEV_FILE_APPEND,
> - QEMU_CAPS_CCID_EMULATED);
> + QEMU_CAPS_CCID_EMULATED,
> + QEMU_CAPS_VIRTIO_SCSI,
> + QEMU_CAPS_SCSI_LSI,
[1] and the LSI wouldn't be needed here.
John
first patch looks fine, but I'm not so convinced on this one only
because of the host name w/ a comma.
> + QEMU_CAPS_DEVICE_SCSI_GENERIC);
> DO_TEST("debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS);
>
> DO_TEST("master-key", QEMU_CAPS_OBJECT_SECRET);
>
7:52 a.m.
New subject: [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.
[...]
8:37 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: Escape commas for qemuBuildSCSIiSCSIHostdevDrvStr
On 06/26/2018 08:52 AM, John Ferlan wrote:
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.
I think that sounds good.
Thanks,
Anya
John
and yes, once this is pushed, that item comes off the wiki list.
[...]
2341
days inactive
2347
days old
6 comments
2 participants
participants (2)
-
Anya Harter -
John Ferlan