Re: [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&gt; >> --- >> 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&gt; > > 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