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

[Cole Robinson]

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

Thanks,
Cole