[libvirt] [PATCH 4/4] qemu: Escape commas for qemuBuildGrapicsSPICECommandLine

[Cole Robinson]

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 at 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 at 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