12:57 p.m.
The function virQEMUBuildBufferEscapeComma is used to escape commas in
user provided fields for qemu command line processing in the four
functions listed below.
A corresponding test for each comma escaping instance has been added to
tests/qemuxml2argvdata/name-escape.xml.
This should complete the BiteSizedTask entry at
https://wiki.libvirt.org/page/BiteSizedTasks#qemu:_Use_comma_escaping_for....
Anya Harter (4):
qemu: Escape commas for qemuBuildChrChardevStr
qemu: Escape commas for qemuBuildChrChardevFileStr
qemu: Escape commas for qemuBuildSmartcardCommandLine
qemu: Escape commas for qemuBuildGrapicsSPICECommandLine
src/qemu/qemu_command.c | 44 +++++++++++--------------
tests/qemuxml2argvdata/name-escape.args | 14 ++++++--
tests/qemuxml2argvdata/name-escape.xml | 18 ++++++++++
tests/qemuxml2argvtest.c | 10 +++++-
4 files changed, 58 insertions(+), 28 deletions(-)
--
2.17.1
12:57 p.m.
New subject: [libvirt] [PATCH 1/4] qemu: Escape commas for qemuBuildChrChardevStr
Add comma escaping for dev->data.file.path in cases
VIR_DOMAIN_CHR_TYPE_DEV and VIR_DOMAIN_CHR_TYPE_PIPE.
Signed-off-by: Anya Harter <aharter(a)redhat.com>
---
src/qemu/qemu_command.c | 9 +++++----
tests/qemuxml2argvdata/name-escape.args | 4 ++++
tests/qemuxml2argvdata/name-escape.xml | 7 +++++++
tests/qemuxml2argvtest.c | 3 ++-
4 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bb956a77f4..b764008949 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4975,9 +4975,10 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
break;
case VIR_DOMAIN_CHR_TYPE_DEV:
- virBufferAsprintf(&buf, "%s,id=%s,path=%s",
+ virBufferAsprintf(&buf, "%s,id=%s,path=",
STRPREFIX(alias, "parallel") ? "parport" :
"tty",
- charAlias, dev->data.file.path);
+ charAlias);
+ virQEMUBuildBufferEscapeComma(&buf, dev->data.file.path);
break;
case VIR_DOMAIN_CHR_TYPE_FILE:
@@ -4997,8 +4998,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
break;
case VIR_DOMAIN_CHR_TYPE_PIPE:
- virBufferAsprintf(&buf, "pipe,id=%s,path=%s", charAlias,
- dev->data.file.path);
+ virBufferAsprintf(&buf, "pipe,id=%s,path=", charAlias);
+ virQEMUBuildBufferEscapeComma(&buf, dev->data.file.path);
break;
case VIR_DOMAIN_CHR_TYPE_STDIO:
diff --git a/tests/qemuxml2argvdata/name-escape.args
b/tests/qemuxml2argvdata/name-escape.args
index 5ff8c03db8..4b03068f95 100644
--- a/tests/qemuxml2argvdata/name-escape.args
+++ b/tests/qemuxml2argvdata/name-escape.args
@@ -23,6 +23,10 @@ bar=2/monitor.sock,server,nowait \
-no-acpi \
-boot c \
-usb \
+-chardev tty,id=charserial0,path=/dev/ttyS2,,foo \
+-device isa-serial,chardev=charserial0,id=serial0 \
+-chardev pipe,id=charchannel0,path=/tmp/guestfwd,,foo \
+-netdev user,guestfwd=tcp:10.0.2.1:4600-chardev:charchannel0,id=user-channel0 \
-vnc unix:/tmp/lib/domain--1-foo=1,,bar=2/vnc.sock \
-spice unix,addr=/tmp/lib/domain--1-foo=1,,bar=2/spice.sock \
-vga cirrus \
diff --git a/tests/qemuxml2argvdata/name-escape.xml
b/tests/qemuxml2argvdata/name-escape.xml
index 6b93d71798..3f5e1c9829 100644
--- a/tests/qemuxml2argvdata/name-escape.xml
+++ b/tests/qemuxml2argvdata/name-escape.xml
@@ -20,5 +20,12 @@
<graphics type='spice'>
<listen type='socket'/>
</graphics>
+ <serial type='dev'>
+ <source path='/dev/ttyS2,foo'/>
+ </serial>
+ <channel type='pipe'>
+ <source path='/tmp/guestfwd,foo'/>
+ <target type='guestfwd' address='10.0.2.1'
port='4600'/>
+ </channel>
</devices>
</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index f44cac9fef..c194ff59c9 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2760,7 +2760,8 @@ mymain(void)
QEMU_CAPS_NAME_GUEST,
QEMU_CAPS_DEVICE_CIRRUS_VGA,
QEMU_CAPS_SPICE,
- QEMU_CAPS_SPICE_UNIX);
+ QEMU_CAPS_SPICE_UNIX,
+ QEMU_CAPS_DEVICE_ISA_SERIAL);
DO_TEST("debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS);
DO_TEST("master-key", QEMU_CAPS_OBJECT_SECRET);
--
2.17.1
6:37 p.m.
New subject: [libvirt] [PATCH 1/4] qemu: Escape commas for qemuBuildChrChardevStr
On 06/18/2018 01:57 PM, Anya Harter wrote:
Add comma escaping for dev->data.file.path in cases
VIR_DOMAIN_CHR_TYPE_DEV and VIR_DOMAIN_CHR_TYPE_PIPE.
Signed-off-by: Anya Harter <aharter(a)redhat.com>
---
src/qemu/qemu_command.c | 9 +++++----
tests/qemuxml2argvdata/name-escape.args | 4 ++++
tests/qemuxml2argvdata/name-escape.xml | 7 +++++++
tests/qemuxml2argvtest.c | 3 ++-
4 files changed, 18 insertions(+), 5 deletions(-)
Having tests is awesome! Thanks!
Not sure why the bite size tasks omitted VIR_DOMAIN_CHR_TYPE_FILE too.
If you want to investigate the FILE case that'd be good - just to make
sure we aren't missing any! I'll still push this as is since it's fine,
but if the FILE needs something it can be added afterwards.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
9:19 a.m.
New subject: [libvirt] [PATCH 1/4] qemu: Escape commas for qemuBuildChrChardevStr
On 06/18/2018 07:37 PM, John Ferlan wrote:
On 06/18/2018 01:57 PM, Anya Harter wrote:
> Add comma escaping for dev->data.file.path in cases
> VIR_DOMAIN_CHR_TYPE_DEV and VIR_DOMAIN_CHR_TYPE_PIPE.
>
> Signed-off-by: Anya Harter <aharter(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 9 +++++----
> tests/qemuxml2argvdata/name-escape.args | 4 ++++
> tests/qemuxml2argvdata/name-escape.xml | 7 +++++++
> tests/qemuxml2argvtest.c | 3 ++-
> 4 files changed, 18 insertions(+), 5 deletions(-)
>
Having tests is awesome! Thanks!
Not sure why the bite size tasks omitted VIR_DOMAIN_CHR_TYPE_FILE too.
If you want to investigate the FILE case that'd be good - just to make
sure we aren't missing any! I'll still push this as is since it's fine,
but if the FILE needs something it can be added afterwards.
VIR_DOMAIN_CHR_TYPE_FILE calls the function qemuBuildChrChardevFileStr
and passes the dev->data.file.path into the parameter named fileval
which I escape in the second patch.
Please let me know if I am missing something here.
Thanks,
Anya
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
6:29 p.m.
New subject: [libvirt] [PATCH 1/4] qemu: Escape commas for qemuBuildChrChardevStr
On 06/19/2018 10:19 AM, Anya Harter wrote:
On 06/18/2018 07:37 PM, John Ferlan wrote:
>
>
> On 06/18/2018 01:57 PM, Anya Harter wrote:
>> Add comma escaping for dev->data.file.path in cases
>> VIR_DOMAIN_CHR_TYPE_DEV and VIR_DOMAIN_CHR_TYPE_PIPE.
>>
>> Signed-off-by: Anya Harter <aharter(a)redhat.com>
>> ---
>> src/qemu/qemu_command.c | 9 +++++----
>> tests/qemuxml2argvdata/name-escape.args | 4 ++++
>> tests/qemuxml2argvdata/name-escape.xml | 7 +++++++
>> tests/qemuxml2argvtest.c | 3 ++-
>> 4 files changed, 18 insertions(+), 5 deletions(-)
>>
>
> Having tests is awesome! Thanks!
>
> Not sure why the bite size tasks omitted VIR_DOMAIN_CHR_TYPE_FILE too.
> If you want to investigate the FILE case that'd be good - just to make
> sure we aren't missing any! I'll still push this as is since it's fine,
> but if the FILE needs something it can be added afterwards.
VIR_DOMAIN_CHR_TYPE_FILE calls the function qemuBuildChrChardevFileStr
and passes the dev->data.file.path into the parameter named fileval
which I escape in the second patch.
Please let me know if I am missing something here.
Nope I didn't dig far enough...
John
12:57 p.m.
New subject: [libvirt] [PATCH 2/4] qemu: Escape commas for qemuBuildChrChardevFileStr
Add comma escaping for fileval.
Signed-off-by: Anya Harter <aharter(a)redhat.com>
---
src/qemu/qemu_command.c | 3 ++-
tests/qemuxml2argvdata/name-escape.args | 2 ++
tests/qemuxml2argvdata/name-escape.xml | 4 ++++
tests/qemuxml2argvtest.c | 3 ++-
4 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b764008949..40e8f8fccf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4867,7 +4867,8 @@ qemuBuildChrChardevFileStr(virLogManagerPtr logManager,
virBufferAsprintf(buf, ",%s=%s,%s=on", filearg, fdpath, appendarg);
VIR_FREE(fdpath);
} else {
- virBufferAsprintf(buf, ",%s=%s", filearg, fileval);
+ virBufferAsprintf(buf, ",%s=", filearg);
+ virQEMUBuildBufferEscapeComma(buf, fileval);
if (appendval != VIR_TRISTATE_SWITCH_ABSENT) {
virBufferAsprintf(buf, ",%s=%s", appendarg,
virTristateSwitchTypeToString(appendval));
diff --git a/tests/qemuxml2argvdata/name-escape.args
b/tests/qemuxml2argvdata/name-escape.args
index 4b03068f95..35a13b2533 100644
--- a/tests/qemuxml2argvdata/name-escape.args
+++ b/tests/qemuxml2argvdata/name-escape.args
@@ -25,6 +25,8 @@ bar=2/monitor.sock,server,nowait \
-usb \
-chardev tty,id=charserial0,path=/dev/ttyS2,,foo \
-device isa-serial,chardev=charserial0,id=serial0 \
+-chardev file,id=charserial1,path=/tmp/serial.log,,foo,append=on \
+-device isa-serial,chardev=charserial1,id=serial1 \
-chardev pipe,id=charchannel0,path=/tmp/guestfwd,,foo \
-netdev user,guestfwd=tcp:10.0.2.1:4600-chardev:charchannel0,id=user-channel0 \
-vnc unix:/tmp/lib/domain--1-foo=1,,bar=2/vnc.sock \
diff --git a/tests/qemuxml2argvdata/name-escape.xml
b/tests/qemuxml2argvdata/name-escape.xml
index 3f5e1c9829..79c1b34458 100644
--- a/tests/qemuxml2argvdata/name-escape.xml
+++ b/tests/qemuxml2argvdata/name-escape.xml
@@ -23,6 +23,10 @@
<serial type='dev'>
<source path='/dev/ttyS2,foo'/>
</serial>
+ <serial type='file'>
+ <source path='/tmp/serial.log,foo' append='on'/>
+ <target port='0'/>
+ </serial>
<channel type='pipe'>
<source path='/tmp/guestfwd,foo'/>
<target type='guestfwd' address='10.0.2.1'
port='4600'/>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index c194ff59c9..3e02fa576c 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2761,7 +2761,8 @@ mymain(void)
QEMU_CAPS_DEVICE_CIRRUS_VGA,
QEMU_CAPS_SPICE,
QEMU_CAPS_SPICE_UNIX,
- QEMU_CAPS_DEVICE_ISA_SERIAL);
+ QEMU_CAPS_DEVICE_ISA_SERIAL,
+ QEMU_CAPS_CHARDEV_FILE_APPEND);
DO_TEST("debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS);
DO_TEST("master-key", QEMU_CAPS_OBJECT_SECRET);
--
2.17.1
6:32 p.m.
New subject: [libvirt] [PATCH 2/4] qemu: Escape commas for qemuBuildChrChardevFileStr
On 06/18/2018 01:57 PM, Anya Harter wrote:
Add comma escaping for fileval.
Signed-off-by: Anya Harter <aharter(a)redhat.com>
---
src/qemu/qemu_command.c | 3 ++-
tests/qemuxml2argvdata/name-escape.args | 2 ++
tests/qemuxml2argvdata/name-escape.xml | 4 ++++
tests/qemuxml2argvtest.c | 3 ++-
4 files changed, 10 insertions(+), 2 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
12:57 p.m.
New subject: [libvirt] [PATCH 3/4] qemu: Escape commas for qemuBuildSmartcardCommandLine
Add comma escaping for smartcard->data.cert.file[i] and
smartcard->data.cert.database.
Signed-off-by: Anya Harter <aharter(a)redhat.com>
---
src/qemu/qemu_command.c | 21 ++++-----------------
tests/qemuxml2argvdata/name-escape.args | 3 +++
tests/qemuxml2argvdata/name-escape.xml | 6 ++++++
tests/qemuxml2argvtest.c | 3 ++-
4 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 40e8f8fccf..a9a5e200e9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8716,29 +8716,16 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates");
for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) {
- if (strchr(smartcard->data.cert.file[i], ',')) {
- virBufferFreeAndReset(&opt);
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("invalid certificate name: %s"),
- smartcard->data.cert.file[i]);
- return -1;
- }
- virBufferAsprintf(&opt, ",cert%zu=%s", i + 1,
- smartcard->data.cert.file[i]);
+ virBufferAsprintf(&opt, ",cert%zu=", i + 1);
+ virQEMUBuildBufferEscapeComma(&opt, smartcard->data.cert.file[i]);
}
if (smartcard->data.cert.database) {
- if (strchr(smartcard->data.cert.database, ',')) {
- virBufferFreeAndReset(&opt);
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("invalid database name: %s"),
- smartcard->data.cert.database);
- return -1;
- }
database = smartcard->data.cert.database;
} else {
database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE;
}
- virBufferAsprintf(&opt, ",db=%s", database);
+ virBufferAddLit(&opt, ",db=");
+ virQEMUBuildBufferEscapeComma(&opt, database);
break;
case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
diff --git a/tests/qemuxml2argvdata/name-escape.args
b/tests/qemuxml2argvdata/name-escape.args
index 35a13b2533..d3b908a7e6 100644
--- a/tests/qemuxml2argvdata/name-escape.args
+++ b/tests/qemuxml2argvdata/name-escape.args
@@ -22,7 +22,10 @@ bar=2/monitor.sock,server,nowait \
-no-shutdown \
-no-acpi \
-boot c \
+-device usb-ccid,id=ccid0,bus=usb.0,port=1 \
-usb \
+-device ccid-card-emulated,backend=certificates,cert1=cert1,,foo,cert2=cert2,\
+cert3=cert3,db=/etc/pki/nssdb,,foo,id=smartcard0,bus=ccid0.0 \
-chardev tty,id=charserial0,path=/dev/ttyS2,,foo \
-device isa-serial,chardev=charserial0,id=serial0 \
-chardev file,id=charserial1,path=/tmp/serial.log,,foo,append=on \
diff --git a/tests/qemuxml2argvdata/name-escape.xml
b/tests/qemuxml2argvdata/name-escape.xml
index 79c1b34458..9ca7be5968 100644
--- a/tests/qemuxml2argvdata/name-escape.xml
+++ b/tests/qemuxml2argvdata/name-escape.xml
@@ -31,5 +31,11 @@
<source path='/tmp/guestfwd,foo'/>
<target type='guestfwd' address='10.0.2.1'
port='4600'/>
</channel>
+ <smartcard mode='host-certificates'>
+ <certificate>cert1,foo</certificate>
+ <certificate>cert2</certificate>
+ <certificate>cert3</certificate>
+ <database>/etc/pki/nssdb,foo</database>
+ </smartcard>
</devices>
</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 3e02fa576c..7468537c68 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2762,7 +2762,8 @@ mymain(void)
QEMU_CAPS_SPICE,
QEMU_CAPS_SPICE_UNIX,
QEMU_CAPS_DEVICE_ISA_SERIAL,
- QEMU_CAPS_CHARDEV_FILE_APPEND);
+ QEMU_CAPS_CHARDEV_FILE_APPEND,
+ QEMU_CAPS_CCID_EMULATED);
DO_TEST("debug-threads", QEMU_CAPS_NAME_DEBUG_THREADS);
DO_TEST("master-key", QEMU_CAPS_OBJECT_SECRET);
--
2.17.1
6:30 p.m.
New subject: [libvirt] [PATCH 3/4] qemu: Escape commas for qemuBuildSmartcardCommandLine
On 06/18/2018 01:57 PM, Anya Harter wrote:
Add comma escaping for smartcard->data.cert.file[i] and
smartcard->data.cert.database.
Signed-off-by: Anya Harter <aharter(a)redhat.com>
---
src/qemu/qemu_command.c | 21 ++++-----------------
tests/qemuxml2argvdata/name-escape.args | 3 +++
tests/qemuxml2argvdata/name-escape.xml | 6 ++++++
tests/qemuxml2argvtest.c | 3 ++-
4 files changed, 15 insertions(+), 18 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
(and pushed)
12:57 p.m.
New subject: [libvirt] [PATCH 4/4] qemu: Escape commas for qemuBuildGrapicsSPICECommandLine
Add comma escaping for cfg->spiceTLSx509certdir and
graphics->data.spice.rendernode.
Signed-off-by: Anya Harter <aharter(a)redhat.com>
---
src/qemu/qemu_command.c | 11 ++++++++---
tests/qemuxml2argvdata/name-escape.args | 5 +++--
tests/qemuxml2argvdata/name-escape.xml | 1 +
tests/qemuxml2argvtest.c | 5 +++++
4 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a9a5e200e9..36dccea9b2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7974,8 +7974,11 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
!cfg->spicePassword)
virBufferAddLit(&opt, "disable-ticketing,");
- if (hasSecure)
- virBufferAsprintf(&opt, "x509-dir=%s,",
cfg->spiceTLSx509certdir);
+ if (hasSecure) {
+ virBufferAddLit(&opt, "x509-dir=");
+ virQEMUBuildBufferEscapeComma(&opt, cfg->spiceTLSx509certdir);
+ virBufferAsprintf(&opt, ",");
+ }
switch (graphics->data.spice.defaultMode) {
case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
@@ -8082,7 +8085,9 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
goto error;
}
- virBufferAsprintf(&opt, "rendernode=%s,",
graphics->data.spice.rendernode);
+ virBufferAddLit(&opt, "rendernode=");
+ virQEMUBuildBufferEscapeComma(&opt, graphics->data.spice.rendernode);
+ virBufferAsprintf(&opt, ",");
}
}
diff --git a/tests/qemuxml2argvdata/name-escape.args
b/tests/qemuxml2argvdata/name-escape.args
index d3b908a7e6..72ed2e8410 100644
--- a/tests/qemuxml2argvdata/name-escape.args
+++ b/tests/qemuxml2argvdata/name-escape.args
@@ -33,6 +33,7 @@ cert3=cert3,db=/etc/pki/nssdb,,foo,id=smartcard0,bus=ccid0.0 \
-chardev pipe,id=charchannel0,path=/tmp/guestfwd,,foo \
-netdev user,guestfwd=tcp:10.0.2.1:4600-chardev:charchannel0,id=user-channel0 \
-vnc unix:/tmp/lib/domain--1-foo=1,,bar=2/vnc.sock \
--spice unix,addr=/tmp/lib/domain--1-foo=1,,bar=2/spice.sock \
--vga cirrus \
+-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
diff --git a/tests/qemuxml2argvdata/name-escape.xml
b/tests/qemuxml2argvdata/name-escape.xml
index 9ca7be5968..0580de1813 100644
--- a/tests/qemuxml2argvdata/name-escape.xml
+++ b/tests/qemuxml2argvdata/name-escape.xml
@@ -19,6 +19,7 @@
</graphics>
<graphics type='spice'>
<listen type='socket'/>
+ <gl enable='yes' rendernode='/dev/dri/foo,bar'/>
</graphics>
<serial type='dev'>
<source path='/dev/ttyS2,foo'/>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 7468537c68..ade21f5a10 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2761,6 +2761,11 @@ mymain(void)
QEMU_CAPS_DEVICE_CIRRUS_VGA,
QEMU_CAPS_SPICE,
QEMU_CAPS_SPICE_UNIX,
+ QEMU_CAPS_DEVICE_VIRTIO_GPU,
+ QEMU_CAPS_VIRTIO_GPU_VIRGL,
+ QEMU_CAPS_SPICE_GL,
+ QEMU_CAPS_SPICE_RENDERNODE,
+ QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_DEVICE_ISA_SERIAL,
QEMU_CAPS_CHARDEV_FILE_APPEND,
QEMU_CAPS_CCID_EMULATED);
--
2.17.1
6:52 p.m.
New subject: [libvirt] [PATCH 4/4] qemu: Escape commas for qemuBuildGrapicsSPICECommandLine
On 06/18/2018 01:57 PM, Anya Harter wrote:
Add comma escaping for cfg->spiceTLSx509certdir and
graphics->data.spice.rendernode.
Signed-off-by: Anya Harter <aharter(a)redhat.com>
---
src/qemu/qemu_command.c | 11 ++++++++---
tests/qemuxml2argvdata/name-escape.args | 5 +++--
tests/qemuxml2argvdata/name-escape.xml | 1 +
tests/qemuxml2argvtest.c | 5 +++++
4 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a9a5e200e9..36dccea9b2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7974,8 +7974,11 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
!cfg->spicePassword)
virBufferAddLit(&opt, "disable-ticketing,");
- if (hasSecure)
- virBufferAsprintf(&opt, "x509-dir=%s,",
cfg->spiceTLSx509certdir);
+ if (hasSecure) {
+ virBufferAddLit(&opt, "x509-dir=");
+ virQEMUBuildBufferEscapeComma(&opt, cfg->spiceTLSx509certdir);
+ virBufferAsprintf(&opt, ",");
make syntax-check would have told you:
src/qemu/qemu_command.c:7980: virBufferAsprintf(&opt, ",");
src/qemu/qemu_command.c:8090: virBufferAsprintf(&opt, ",");
So here and below, I changed to virBufferAddLit before pushing.
+ }
switch (graphics->data.spice.defaultMode) {
case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
@@ -8082,7 +8085,9 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
goto error;
}
- virBufferAsprintf(&opt, "rendernode=%s,",
graphics->data.spice.rendernode);
+ virBufferAddLit(&opt, "rendernode=");
+ virQEMUBuildBufferEscapeComma(&opt,
graphics->data.spice.rendernode);
+ virBufferAsprintf(&opt, ",");
}
}
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
From the last time I passed through this when Sukrit posted patches,
still to do are qemuBuildHostNetStr, qemuBuildDiskThrottling (for group
name), and various qemuBuildNetworkDriveURI, qemuBuildNetworkDriveStr,
and qemuGetDriveSourceString.
[...]
8:47 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: Escape commas for qemuBuildGrapicsSPICECommandLine
On 06/18/2018 07:52 PM, John Ferlan wrote:
On 06/18/2018 01:57 PM, Anya Harter wrote:
> Add comma escaping for cfg->spiceTLSx509certdir and
> graphics->data.spice.rendernode.
>
> Signed-off-by: Anya Harter <aharter(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 11 ++++++++---
> tests/qemuxml2argvdata/name-escape.args | 5 +++--
> tests/qemuxml2argvdata/name-escape.xml | 1 +
> tests/qemuxml2argvtest.c | 5 +++++
> 4 files changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index a9a5e200e9..36dccea9b2 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7974,8 +7974,11 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
> !cfg->spicePassword)
> virBufferAddLit(&opt, "disable-ticketing,");
>
> - if (hasSecure)
> - virBufferAsprintf(&opt, "x509-dir=%s,",
cfg->spiceTLSx509certdir);
> + if (hasSecure) {
> + virBufferAddLit(&opt, "x509-dir=");
> + virQEMUBuildBufferEscapeComma(&opt, cfg->spiceTLSx509certdir);
> + virBufferAsprintf(&opt, ",");
make syntax-check would have told you:
src/qemu/qemu_command.c:7980: virBufferAsprintf(&opt, ",");
src/qemu/qemu_command.c:8090: virBufferAsprintf(&opt, ",");
So here and below, I changed to virBufferAddLit before pushing.
> + }
>
> switch (graphics->data.spice.defaultMode) {
> case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
> @@ -8082,7 +8085,9 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
> goto error;
> }
>
> - virBufferAsprintf(&opt, "rendernode=%s,",
graphics->data.spice.rendernode);
> + virBufferAddLit(&opt, "rendernode=");
> + virQEMUBuildBufferEscapeComma(&opt,
graphics->data.spice.rendernode);
> + virBufferAsprintf(&opt, ",");
> }
> }
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
>From the last time I passed through this when Sukrit posted patches,
still to do are qemuBuildHostNetStr, qemuBuildDiskThrottling (for group
name), and various qemuBuildNetworkDriveURI, qemuBuildNetworkDriveStr,
and qemuGetDriveSourceString.
Oh cool, I didn't realize you had found more examples! I looked at some
of these with Anya before the patch series.
NetworkDriveURI is a private subset of NetworkDriveStr, so the former
doesn't need any direct changes AFAICT.
qemuGetDriveSourceString is called outside qemu_command.c, for example
passing the result to qemu monitor commands. Anyone know if comma
escaping applies there too? Same with qemuBuildHostNetStr
Thanks,
Cole
9:46 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: Escape commas for qemuBuildGrapicsSPICECommandLine
On 06/19/2018 09:47 AM, Cole Robinson wrote:
On 06/18/2018 07:52 PM, John Ferlan wrote:
>
>
> On 06/18/2018 01:57 PM, Anya Harter wrote:
>> Add comma escaping for cfg->spiceTLSx509certdir and
>> graphics->data.spice.rendernode.
>>
>> Signed-off-by: Anya Harter <aharter(a)redhat.com>
>> ---
>> src/qemu/qemu_command.c | 11 ++++++++---
>> tests/qemuxml2argvdata/name-escape.args | 5 +++--
>> tests/qemuxml2argvdata/name-escape.xml | 1 +
>> tests/qemuxml2argvtest.c | 5 +++++
>> 4 files changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index a9a5e200e9..36dccea9b2 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -7974,8 +7974,11 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr
cfg,
>> !cfg->spicePassword)
>> virBufferAddLit(&opt, "disable-ticketing,");
>>
>> - if (hasSecure)
>> - virBufferAsprintf(&opt, "x509-dir=%s,",
cfg->spiceTLSx509certdir);
>> + if (hasSecure) {
>> + virBufferAddLit(&opt, "x509-dir=");
>> + virQEMUBuildBufferEscapeComma(&opt, cfg->spiceTLSx509certdir);
>> + virBufferAsprintf(&opt, ",");
>
> make syntax-check would have told you:
>
> src/qemu/qemu_command.c:7980: virBufferAsprintf(&opt, ",");
> src/qemu/qemu_command.c:8090: virBufferAsprintf(&opt, ",");
>
> So here and below, I changed to virBufferAddLit before pushing.
>
>> + }
>>
>> switch (graphics->data.spice.defaultMode) {
>> case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
>> @@ -8082,7 +8085,9 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr
cfg,
>> goto error;
>> }
>>
>> - virBufferAsprintf(&opt, "rendernode=%s,",
graphics->data.spice.rendernode);
>> + virBufferAddLit(&opt, "rendernode=");
>> + virQEMUBuildBufferEscapeComma(&opt,
graphics->data.spice.rendernode);
>> + virBufferAsprintf(&opt, ",");
>> }
>> }
>
> Reviewed-by: John Ferlan <jferlan(a)redhat.com>
>
> John
>
> >From the last time I passed through this when Sukrit posted patches,
> still to do are qemuBuildHostNetStr, qemuBuildDiskThrottling (for group
> name), and various qemuBuildNetworkDriveURI, qemuBuildNetworkDriveStr,
> and qemuGetDriveSourceString.
>
Oh cool, I didn't realize you had found more examples! I looked at some
of these with Anya before the patch series.
NetworkDriveURI is a private subset of NetworkDriveStr, so the former
doesn't need any direct changes AFAICT.
qemuGetDriveSourceString is called outside qemu_command.c, for example
passing the result to qemu monitor commands. Anyone know if comma
escaping applies there too? Same with qemuBuildHostNetStr
From what I can tell qemuGetDriveSourceString and qemuBuildHostNetStr
usages outside of qemu_command.c should _not_ have comma escaping, which
makes sense as the comma isn't used as a delimiter in those substrings.
So the comma escaping should be done at the call sites of those
functions in qemu_command.c
Thanks,
Cole
8:27 a.m.
New subject: [libvirt] [PATCH 4/4] qemu: Escape commas for qemuBuildGrapicsSPICECommandLine
On 06/19/2018 10:46 AM, Cole Robinson wrote:
On 06/19/2018 09:47 AM, Cole Robinson wrote:
> On 06/18/2018 07:52 PM, John Ferlan wrote:
>>
>>
>> On 06/18/2018 01:57 PM, Anya Harter wrote:
>>> Add comma escaping for cfg->spiceTLSx509certdir and
>>> graphics->data.spice.rendernode.
>>>
>>> Signed-off-by: Anya Harter <aharter(a)redhat.com>
>>> ---
>>> src/qemu/qemu_command.c | 11 ++++++++---
>>> tests/qemuxml2argvdata/name-escape.args | 5 +++--
>>> tests/qemuxml2argvdata/name-escape.xml | 1 +
>>> tests/qemuxml2argvtest.c | 5 +++++
>>> 4 files changed, 17 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>> index a9a5e200e9..36dccea9b2 100644
>>> --- a/src/qemu/qemu_command.c
>>> +++ b/src/qemu/qemu_command.c
>>> @@ -7974,8 +7974,11 @@
qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>>> !cfg->spicePassword)
>>> virBufferAddLit(&opt, "disable-ticketing,");
>>>
>>> - if (hasSecure)
>>> - virBufferAsprintf(&opt, "x509-dir=%s,",
cfg->spiceTLSx509certdir);
>>> + if (hasSecure) {
>>> + virBufferAddLit(&opt, "x509-dir=");
>>> + virQEMUBuildBufferEscapeComma(&opt,
cfg->spiceTLSx509certdir);
>>> + virBufferAsprintf(&opt, ",");
>>
>> make syntax-check would have told you:
>>
>> src/qemu/qemu_command.c:7980: virBufferAsprintf(&opt, ",");
>> src/qemu/qemu_command.c:8090: virBufferAsprintf(&opt,
",");
>>
>> So here and below, I changed to virBufferAddLit before pushing.
>>
>>> + }
>>>
>>> switch (graphics->data.spice.defaultMode) {
>>> case VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_SECURE:
>>> @@ -8082,7 +8085,9 @@
qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg,
>>> goto error;
>>> }
>>>
>>> - virBufferAsprintf(&opt, "rendernode=%s,",
graphics->data.spice.rendernode);
>>> + virBufferAddLit(&opt, "rendernode=");
>>> + virQEMUBuildBufferEscapeComma(&opt,
graphics->data.spice.rendernode);
>>> + virBufferAsprintf(&opt, ",");
>>> }
>>> }
>>
>> Reviewed-by: John Ferlan <jferlan(a)redhat.com>
>>
>> John
>>
>> >From the last time I passed through this when Sukrit posted patches,
>> still to do are qemuBuildHostNetStr, qemuBuildDiskThrottling (for group
>> name), and various qemuBuildNetworkDriveURI, qemuBuildNetworkDriveStr,
>> and qemuGetDriveSourceString.
>>
>
> Oh cool, I didn't realize you had found more examples! I looked at some
> of these with Anya before the patch series.
>
> NetworkDriveURI is a private subset of NetworkDriveStr, so the former
> doesn't need any direct changes AFAICT.
>
> qemuGetDriveSourceString is called outside qemu_command.c, for example
> passing the result to qemu monitor commands. Anyone know if comma
> escaping applies there too? Same with qemuBuildHostNetStr
>
From what I can tell qemuGetDriveSourceString and qemuBuildHostNetStr
usages outside of qemu_command.c should _not_ have comma escaping, which
makes sense as the comma isn't used as a delimiter in those substrings.
So the comma escaping should be done at the call sites of those
functions in qemu_command.c
By my estimation the BiteSizedTask entry
https://wiki.libvirt.org/page/BiteSizedTasks#qemu:_Use_comma_escaping_for...
is complete.
The four bullet points have been addressed as follows:
* Building the source drive string in qemuBuildNetworkDriveURI,
qemuBuildNetworkDriveStr, and qemuGetDriveSourceString
(src->hosts->socket, NBD exportname=src->path, Sheepdog src->path
processing, rbd src->path & configFile processing)
Since some of these functions are called from outside the building of
the command line, escaping within the functions could lead to problems
elsewhere. Thus, the approach is to escape the output of the functions
from their callers.
All calls to qemuBuildNetworkDriveURI are within qemuBuildNetworkDriveStr.
One call to qemuBuildNetworkDriveStr is within qemuGetDriveSourceString,
and the other is within qemuBuildSCSIiSCSIHostdevDrvStr.
The only call to qemuGetDriveSourceString is within
qemuBuildDriveSourceStr and the output is escaped later within this
function. This escaping handles all but the one call to
qemuBuildNetworkDriveStr.
This case is handled in this patch:
https://www.redhat.com/archives/libvir-list/2018-June/msg01462.html
* Validate whether TYPE_FILE needs to follow TYPE_DEV and TYPE_PIPE
handling in qemuBuildChrChardevStr
Addressed in this email:
https://www.redhat.com/archives/libvir-list/2018-June/msg01384.html
* socket.address processing in qemuBuildHostNetStr (localaddr for UDP too)
Cole believes that these entities cannot have commas in them anyways, so
escaping any commas would just delay the inevitable
* group_name processing in qemuBuildDiskThrottling
Handled in this patch:
https://www.redhat.com/archives/libvir-list/2018-June/msg01409.html
Please let me know if there is any more work to be done on this task or
if it is ready to be deleted from the page.
Thanks so much,
Anya
2325
days inactive
2327
days old
13 comments
3 participants
participants (3)
-
Anya Harter -
Cole Robinson -
John Ferlan