| Message ID | 14c6b73690673be975bd175e447002485385d26d.1462314536.git.crobinso@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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 \