[v2,3/4] qemu: command: escape commas in chardev socket path

Message ID 14c6b73690673be975bd175e447002485385d26d.1462314536.git.crobinso@redhat.com
State New
Headers show

Commit Message

Cole Robinson May 4, 2016, 2:56 p.m.
After this, a default virt-manager VM will startup with a comma
in the VM name:

https://bugzilla.redhat.com/show_bug.cgi?id=639926
---
 src/qemu/qemu_command.c                              | 9 ++++-----
 tests/qemuxml2argvdata/qemuxml2argv-name-escape.args | 2 +-
 2 files changed, 5 insertions(+), 6 deletions(-)

-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

Cole Robinson May 6, 2016, 12:11 p.m. | #1
On 05/06/2016 07:54 AM, John Ferlan wrote:
> 

> 

> On 05/04/2016 10:56 AM, Cole Robinson wrote:

>> After this, a default virt-manager VM will startup with a comma

>> in the VM name:

>>

>> https://bugzilla.redhat.com/show_bug.cgi?id=639926

>> ---

>>  src/qemu/qemu_command.c                              | 9 ++++-----

>>  tests/qemuxml2argvdata/qemuxml2argv-name-escape.args | 2 +-

>>  2 files changed, 5 insertions(+), 6 deletions(-)

>>

> 

> FWIW:

> w/r/t my comment in your v1 about vmagent... It's the "guest_agent" I

> was thinking about.  In any case, I see it uses the chardev path

> generation code, so I think that's covered by your patch 3.

> 

> 

>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c

>> index d54d0d1..c2f55b5 100644

>> --- a/src/qemu/qemu_command.c

>> +++ b/src/qemu/qemu_command.c

>> @@ -4833,11 +4833,10 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,

>>          break;

>>  

>>      case VIR_DOMAIN_CHR_TYPE_UNIX:

>> -        virBufferAsprintf(&buf,

>> -                          "socket,id=char%s,path=%s%s",

>> -                          alias,

>> -                          dev->data.nix.path,

>> -                          dev->data.nix.listen ? ",server,nowait" : "");

>> +        virBufferAsprintf(&buf, "socket,id=char%s,path=", alias);

>> +        virBufferEscape(&buf, ',', ",", "%s", dev->data.nix.path);

> 

> Unless you know/see that the data.nix.path is built using

> domainChannelTargetDir during qemuBuildChannelsCommandLine it may not be

> "intuitively obvious" why doing the escape here is necessary...

> 


It's not specific to the autocreated socket path based on
domainChannelTargetDir, the user could have passed in a manual path with a
comma in it

The rule is that any time a user specified string is passed to the qemu
command line, we should be escaping commas. <name> happens to trickle out into
several other places, but doing this for the unix path is still beneficial
regardless of the <name> focus of this series

- Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patch hide | download patch | download mbox

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d54d0d1..c2f55b5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4833,11 +4833,10 @@  qemuBuildChrChardevStr(virLogManagerPtr logManager,
         break;
 
     case VIR_DOMAIN_CHR_TYPE_UNIX:
-        virBufferAsprintf(&buf,
-                          "socket,id=char%s,path=%s%s",
-                          alias,
-                          dev->data.nix.path,
-                          dev->data.nix.listen ? ",server,nowait" : "");
+        virBufferAsprintf(&buf, "socket,id=char%s,path=", alias);
+        virBufferEscape(&buf, ',', ",", "%s", dev->data.nix.path);
+        if (dev->data.nix.listen)
+            virBufferAddLit(&buf, ",server,nowait");
         break;
 
     case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
index a5e35b8..772d94f 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-name-escape.args
@@ -15,7 +15,7 @@  bar/master-key.aes \
 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
 -nographic \
 -nodefaults \
--chardev socket,id=charmonitor,path=/tmp/lib/domain--1-foo,bar/monitor.sock,\
+-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-foo,,bar/monitor.sock,\
 server,nowait \
 -mon chardev=charmonitor,id=monitor,mode=readline \
 -no-acpi \