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

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(-) -- 1.8.3.1

Add comma escaping for info->romfile. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/qemu/qemu_command.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f864350..e7b8aa3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -459,8 +459,10 @@ qemuBuildRomStr(virBufferPtr buf, default: break; } - if (info->romfile) - virBufferAsprintf(buf, ",romfile=%s", info->romfile); + if (info->romfile) { + virBufferAddLit(buf, ",romfile="); + virQEMUBuildBufferEscapeComma(buf, info->romfile); + } } return 0; } -- 1.8.3.1

Add comma escaping for disk->vendor and disk->product when being built for the command line (and not from hotplug). Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/qemu/qemu_command.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e7b8aa3..b75e441 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2151,11 +2151,15 @@ qemuBuildDriveDevStr(const virDomainDef *def, virBufferAsprintf(&opt, ",wwn=0x%s", disk->wwn); } - if (disk->vendor) - virBufferAsprintf(&opt, ",vendor=%s", disk->vendor); + if (disk->vendor) { + virBufferAddLit(&opt, ",vendor="); + virQEMUBuildBufferEscapeComma(&opt, disk->vendor); + } - if (disk->product) - virBufferAsprintf(&opt, ",product=%s", disk->product); + if (disk->product) { + virBufferAddLit(&opt, ",product="); + virQEMUBuildBufferEscapeComma(&opt, disk->product); + } if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_USB_STORAGE_REMOVABLE)) { -- 1.8.3.1

Add comma escaping for fs->src->path and fs->dst. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/qemu/qemu_command.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b75e441..e903491 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2396,7 +2396,8 @@ qemuBuildFSStr(virDomainFSDefPtr fs, } virBufferAsprintf(&opt, ",id=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); - virBufferAsprintf(&opt, ",path=%s", fs->src->path); + virBufferAddLit(&opt, ",path="); + virQEMUBuildBufferEscapeComma(&opt, fs->src->path); if (fs->readonly) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_FSDEV_READONLY)) { @@ -2441,7 +2442,8 @@ qemuBuildFSDevStr(const virDomainDef *def, virBufferAsprintf(&opt, ",id=%s", fs->info.alias); virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias); - virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst); + virBufferAddLit(&opt, ",mount_tag="); + virQEMUBuildBufferEscapeComma(&opt, fs->dst); if (qemuBuildVirtioOptionsStr(&opt, fs->virtio, qemuCaps) < 0) goto error; -- 1.8.3.1

Add comma escaping for cfg->vncTLSx509certdir. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/qemu/qemu_command.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e903491..8f0dddf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7871,10 +7871,13 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, if (cfg->vncTLS) { virBufferAddLit(&opt, ",tls"); - if (cfg->vncTLSx509verify) - virBufferAsprintf(&opt, ",x509verify=%s", cfg->vncTLSx509certdir); - else - virBufferAsprintf(&opt, ",x509=%s", cfg->vncTLSx509certdir); + if (cfg->vncTLSx509verify) { + virBufferAddLit(&opt, ",x509verify="); + virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + } else { + virBufferAddLit(&opt, ",x509="); + virQEMUBuildBufferEscapeComma(&opt, cfg->vncTLSx509certdir); + } } if (cfg->vncSASL) { -- 1.8.3.1

Add comma escaping for loader->path and loader->nvram. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com> --- src/qemu/qemu_command.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8f0dddf..49dc95b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9542,9 +9542,9 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, NULL); } - virBufferAsprintf(&buf, - "file=%s,if=pflash,format=raw,unit=%d", - loader->path, unit); + virBufferAddLit(&buf, "file="); + virQEMUBuildBufferEscapeComma(&buf, loader->path); + virBufferAsprintf(&buf, ",if=pflash,format=raw,unit=%d", unit); unit++; if (loader->readonly) { @@ -9557,9 +9557,9 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, if (loader->nvram) { virBufferFreeAndReset(&buf); - virBufferAsprintf(&buf, - "file=%s,if=pflash,format=raw,unit=%d", - loader->nvram, unit); + virBufferAddLit(&buf, "file="); + virQEMUBuildBufferEscapeComma(&buf, loader->nvram); + virBufferAsprintf(&buf, ",if=pflash,format=raw,unit=%d", unit); virCommandAddArg(cmd, "-drive"); virCommandAddArgBuffer(cmd, &buf); -- 1.8.3.1

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@redhat.com> (series) and pushed. Congrats on your first libvirt patches, John 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.

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@redhat.com> (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_mo... Sukrit if you hit issues with any of the other entries please let me know, maybe some of them aren't adequately described
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

On 18 April 2018 at 00:41, Cole Robinson <crobinso@redhat.com> 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@redhat.com> (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
participants (3)
-
Cole Robinson
-
John Ferlan
-
Sukrit Bhatnagar