Re: [PATCH v4 0/5] qemu: add virQEMUBuildBufferEscapeComma in qemu_command.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 18 April 2018 at 00:41, Cole Robinson <crobinso@xxxxxxxxxx> wrote:
On 04/17/2018 01:33 PM, John Ferlan wrote:
>
>
> On 04/16/2018 06:56 PM, Sukrit Bhatnagar wrote:
>> Changes in v4:
>> Changes made in v2 anbd v3 to qemu_command.c are discarded.
>> Some changes introduced in v2 are used to create new smaller patches.
>> virQEMUBuildBufferEscapeComma was applied to:
>> - info->romfile in qemuBuildRomStr
>> - disk->vendor and disk->product in qemuBuildDriveDevStr
>> - fs->src->path in qemuBuildFSStr
>> - fs->dst in qemuBuildFSDevStr
>> - cfg->vncTLSx509certdir in qemuBuildGraphicsVNCCommandLine
>> - loader->path and loader->nvram in qemuBuildDomainLoaderCommandLine
>>
>>
>> Changes in v3:
>> virQEMUBuildBufferEscapeComma was applied to:
>> - src->hosts->socket in qemuBuildNetworkDriveURI
>> - src->path, src->configFile in qemuBuildNetworkDriveStr
>> - disk->blkdeviotune.group_name in qemuBuildDiskThrottling
>> - net->data.socket.address, net->data.socket.localaddr in
>>   qemuBuildHostNetStr
>> - dev->data.file.path in qemuBuildChrChardevStr
>> - graphics->data.spice.rendernode in
>>   qemuBuildGraphicsSPICECommandLine
>> - smartcard->data.cert.file[i], smartcard->data.cert.database in
>>   qemuBuildSmartcardCommandLine
>>
>> Link to v3: https://www.redhat.com/archives/libvir-list/2018-April/msg00053.html
>>
>> Changes in v2:
>> virQEMUBuildBufferEscapeComma was applied to:
>> - info->romfile in qemuBuildRomStr
>> - disk->vendor, disk->product in qemuBuildDriveDevStr
>> - fs->src->path in qemuBuildFSStr
>> - fs->dst in qemuBuildFSDevStr
>> - connect= in qemuBuildHostNetStr
>> - fileval handling in qemuBuildChrChardevStr
>> - TYPE_DEV, TYPE_PIPE handling in qemuBuildChrChardevStr
>> - cfg->vncTLSx509certdir in qemuBuildGraphicsVNCCommandLine
>> - cfg->spiceTLSx509certdir in qemuBuildGraphicsSPICECommandLine
>> - loader->path, loader->nvram usage in
>>   qemuBuildDomainLoaderCommandLine
>>
>> Link to v2: https://www.redhat.com/archives/libvir-list/2018-March/msg00965.html
>>
>>
>> Sukrit Bhatnagar (5):
>>   qemu: Escape commas for qemuBuildRomStr
>>   qemu: Escape commas for qemuBuildDriveDevStr
>>   qemu: Escape commas for qemuBuildFSStr and qemuBuildFSDevStr
>>   qemu: Escape commas for qemuBuildGraphicsVNCCommandLine
>>   qemu: Escape commas for qemuBuildDomainLoaderCommandLine
>>
>>  src/qemu/qemu_command.c | 47 +++++++++++++++++++++++++++++------------------
>>  1 file changed, 29 insertions(+), 18 deletions(-)
>>
>
> Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
> (series) and pushed.
>
> Congrats on your first libvirt patches,
>
> John

Since I'm guessing these patches were motivated by the BiteSizedTasks
entry, I updated the list there:

https://wiki.libvirt.org/page/BiteSizedTasks#qemu:_Use_comma_escaping_for_more_command_line_values

Sukrit if you hit issues with any of the other entries please let me
know, maybe some of them aren't adequately described

I had earlier tried the conversion to virConfGetValue* functions listed in Code Cleanup tasks.
I had a few issues there:
- in the enum virConfType, there no value for Int and UInt.
- in the function virConfGetValueInt and virConfGetValueUInt, 
  the value is checked for types LLong and ULLong, which would
  always result in an error.

I think maybe some datatypes need to be added to the virConfType and
appropriate type must be assigned in virConfValue.


>
> BTW: All your patches appeared as would a reply-to in the series, I
> think that may be because of not using --no-chain-reply-to on the git
> send-email or git format-patch command line.
>

In my ~/.gitconfig I have:

[sendemail]
    chainreplyto = false


Thanks,
Cole

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux