This patch adds virQEMUBuildBufferEscapeComma to properly escape commas in user provided data fields for qemu command line processing. Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@xxxxxxxxx> --- Thank you for the helpful feedback and apologies for the delay. 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 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 When I tried to change src->path in qemuBuildNetworkDriveStr for this portion 961 } else if (src->nhosts == 1) { 962 if (virAsprintf(&ret, "sheepdog:%s:%u:%s", 963 src->hosts->name, src->hosts->port, 964 src->path) < 0) 965 goto cleanup; 966 } else { make check reported the following error. 141) QEMU XML-2-ARGV disk-drive-network-sheepdog ... In '/home/skrtbhtngr/libvirt/tests/qemuxml2argvdata/disk-drive-network-sheepdog.args': Offset 0 Expect [LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none /usr/bin/qemu-system-i686 -name QEMUGuest1 -S -M pc -m 214 -smp 1,sockets=1,cores=1,threads=1 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 -nographic -nodefaults -chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb -drive file=/dev/HostVG/QEMU,,Guest,,,,1,format=raw,if=none,id=drive-ide0-0-0 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive file=sheepdog:example.org:6000:image,,with,,commas,format=raw,if=none,id=drive-virtio-disk0 -device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0 ] Actual [LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none /usr/bin/qemu-system-i686 -name QEMUGuest1 -S -M pc -m 214 -smp 1,sockets=1,cores=1,threads=1 -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 -nographic -nodefaults -chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -usb -drive file=/dev/HostVG/QEMU,,Guest,,,,1,format=raw,if=none,id=drive-ide0-0-0 -device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -drive file=sheepdog:example.org:6000:image,,,,with,,,,commas,format=raw,if=none,id=drive-virtio-disk0 -device virtio-blk-pci,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0 ] ... FAILED In disk-drive-network-sheepdog.args: ... -drive file=sheepdog:example.org:6000:image,,with,,commas,format=raw,if=none,\ ... I was not quite sure how to handle this part. Adding virQEMUBuildBufferEscapeComma there is escaping the twin commas in the file name again. I have left that part unchanged. src/qemu/qemu_command.c | 111 +++++++++++++++++++++++++----------------------- 1 file changed, 59 insertions(+), 52 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6f76f18ab..26b36551c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -844,14 +844,18 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src, qemuDomainSecretInfoPtr secinfo) { virURIPtr uri = NULL; - char *ret = NULL; + char *ret = NULL, *socket = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; if (!(uri = qemuBlockStorageSourceGetURI(src))) goto cleanup; - if (src->hosts->socket && - virAsprintf(&uri->query, "socket=%s", src->hosts->socket) < 0) - goto cleanup; + if (src->hosts->socket) { + virQEMUBuildBufferEscapeComma(&buf, src->hosts->socket); + socket = virBufferContentAndReset(&buf); + if (virAsprintf(&uri->query, "socket=%s", socket) < 0) + goto cleanup; + } if (qemuBuildGeneralSecinfoURI(uri, secinfo) < 0) goto cleanup; @@ -860,6 +864,8 @@ qemuBuildNetworkDriveURI(virStorageSourcePtr src, cleanup: virURIFree(uri); + virBufferFreeAndReset(&buf); + return ret; } @@ -868,8 +874,9 @@ static char * qemuBuildNetworkDriveStr(virStorageSourcePtr src, qemuDomainSecretInfoPtr secinfo) { - char *ret = NULL; + char *ret = NULL, *path = NULL, *file = NULL; virBuffer buf = VIR_BUFFER_INITIALIZER; + virBuffer bufTemp = VIR_BUFFER_INITIALIZER; size_t i; switch ((virStorageNetProtocol) src->protocol) { @@ -914,8 +921,10 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, goto cleanup; } - if (src->path) - virBufferAsprintf(&buf, ":exportname=%s", src->path); + if (src->path) { + virBufferAddLit(&buf, ":exportname="); + virQEMUBuildBufferEscapeComma(&buf, src->path); + } if (virBufferCheckError(&buf) < 0) goto cleanup; @@ -945,7 +954,9 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, } if (src->nhosts == 0) { - if (virAsprintf(&ret, "sheepdog:%s", src->path) < 0) + virQEMUBuildBufferEscapeComma(&bufTemp, src->path); + path = virBufferContentAndReset(&bufTemp); + if (virAsprintf(&ret, "sheepdog:%s", path) < 0) goto cleanup; } else if (src->nhosts == 1) { if (virAsprintf(&ret, "sheepdog:%s:%u:%s", @@ -967,8 +978,9 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, src->path); goto cleanup; } - - virBufferStrcat(&buf, "rbd:", src->volume, "/", src->path, NULL); + virQEMUBuildBufferEscapeComma(&bufTemp, src->path); + path = virBufferContentAndReset(&bufTemp); + virBufferStrcat(&buf, "rbd:", src->volume, "/", path, NULL); if (src->snapshot) virBufferEscape(&buf, '\\', ":", "@%s", src->snapshot); @@ -994,8 +1006,11 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, } } - if (src->configFile) - virBufferEscape(&buf, '\\', ":", ":conf=%s", src->configFile); + if (src->configFile) { + virQEMUBuildBufferEscapeComma(&bufTemp, src->configFile); + file = virBufferContentAndReset(&bufTemp); + virBufferEscape(&buf, '\\', ":", ":conf=%s", file); + } if (virBufferCheckError(&buf) < 0) goto cleanup; @@ -1022,6 +1037,7 @@ qemuBuildNetworkDriveStr(virStorageSourcePtr src, } cleanup: + virBufferFreeAndReset(&bufTemp); virBufferFreeAndReset(&buf); return ret; @@ -1630,6 +1646,8 @@ qemuBuildDiskThrottling(virDomainDiskDefPtr disk, virBufferAsprintf(buf, ",throttling." _label "=%llu", \ disk->blkdeviotune._field); \ } + virBuffer bufTemp = VIR_BUFFER_INITIALIZER; + char *name = NULL; IOTUNE_ADD(total_bytes_sec, "bps-total"); IOTUNE_ADD(read_bytes_sec, "bps-read"); @@ -1647,8 +1665,9 @@ qemuBuildDiskThrottling(virDomainDiskDefPtr disk, IOTUNE_ADD(size_iops_sec, "iops-size"); if (disk->blkdeviotune.group_name) { - virBufferEscapeString(buf, ",throttling.group=%s", - disk->blkdeviotune.group_name); + virQEMUBuildBufferEscapeComma(&bufTemp, disk->blkdeviotune.group_name); + name = virBufferContentAndReset(&bufTemp); + virBufferEscapeString(buf, ",throttling.group=%s", name); } IOTUNE_ADD(total_bytes_sec_max_length, "bps-total-max-length"); @@ -1657,6 +1676,8 @@ qemuBuildDiskThrottling(virDomainDiskDefPtr disk, IOTUNE_ADD(total_iops_sec_max_length, "iops-total-max-length"); IOTUNE_ADD(read_iops_sec_max_length, "iops-read-max-length"); IOTUNE_ADD(write_iops_sec_max_length, "iops-write-max-length"); + + virBufferFreeAndReset(&bufTemp); #undef IOTUNE_ADD } @@ -3651,27 +3672,25 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, break; case VIR_DOMAIN_NET_TYPE_SERVER: - virBufferAsprintf(&buf, "socket%clisten=%s:%d,", - type_sep, + virBufferAsprintf(&buf, "socket%clisten=", type_sep); + virQEMUBuildBufferEscapeComma(&buf, net->data.socket.address ? net->data.socket.address - : "", - net->data.socket.port); + : ""); + virBufferAsprintf(&buf, ":%d,", net->data.socket.port); break; case VIR_DOMAIN_NET_TYPE_MCAST: - virBufferAsprintf(&buf, "socket%cmcast=%s:%d,", - type_sep, - net->data.socket.address, - net->data.socket.port); + virBufferAsprintf(&buf, "socket%cmcast=", type_sep); + virQEMUBuildBufferEscapeComma(&buf, net->data.socket.address); + virBufferAsprintf(&buf, ":%d,", net->data.socket.port); break; case VIR_DOMAIN_NET_TYPE_UDP: - virBufferAsprintf(&buf, "socket%cudp=%s:%d,localaddr=%s:%d,", - type_sep, - net->data.socket.address, - net->data.socket.port, - net->data.socket.localaddr, - net->data.socket.localport); + virBufferAsprintf(&buf, "socket%cudp=", type_sep); + virQEMUBuildBufferEscapeComma(&buf, net->data.socket.address); + virBufferAsprintf(&buf, ":%d,localaddr=", net->data.socket.port); + virQEMUBuildBufferEscapeComma(&buf, net->data.socket.localaddr); + virBufferAsprintf(&buf, ":%d,", net->data.socket.localport); break; case VIR_DOMAIN_NET_TYPE_USER: @@ -4954,9 +4973,10 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, bool chardevStdioLogd) { virBuffer buf = VIR_BUFFER_INITIALIZER; + virBuffer bufTemp = VIR_BUFFER_INITIALIZER; bool telnet; char *charAlias = NULL; - char *ret = NULL; + char *ret = NULL, *path = NULL; if (!(charAlias = qemuAliasChardevFromDevAlias(alias))) goto cleanup; @@ -4990,9 +5010,11 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager, _("append not supported in this QEMU binary")); goto cleanup; } + virQEMUBuildBufferEscapeComma(&bufTemp, dev->data.file.path); + path = virBufferContentAndReset(&bufTemp); if (qemuBuildChrChardevFileStr(chardevStdioLogd ? logManager : NULL, cmd, def, &buf, - "path", dev->data.file.path, + "path", path, "append", dev->data.file.append) < 0) goto cleanup; break; @@ -8150,8 +8172,8 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, _("This QEMU doesn't support spice OpenGL rendernode")); goto error; } - - virBufferAsprintf(&opt, "rendernode=%s,", graphics->data.spice.rendernode); + virBufferAddLit(&opt, "rendernode="); + virQEMUBuildBufferEscapeComma(&opt, graphics->data.spice.rendernode); } } @@ -8771,7 +8793,6 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager, virDomainSmartcardDefPtr smartcard; char *devstr; virBuffer opt = VIR_BUFFER_INITIALIZER; - const char *database; const char *contAlias = NULL; if (!def->nsmartcards) @@ -8814,29 +8835,15 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager, virBufferAddLit(&opt, "ccid-card-emulated,backend=certificates"); for (i = 0; i < VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES; i++) { - if (strchr(smartcard->data.cert.file[i], ',')) { - virBufferFreeAndReset(&opt); - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid certificate name: %s"), - smartcard->data.cert.file[i]); - return -1; - } - virBufferAsprintf(&opt, ",cert%zu=%s", i + 1, - smartcard->data.cert.file[i]); + virBufferAsprintf(&opt, ",cert%zu=", i + 1); + virQEMUBuildBufferEscapeComma(&opt, smartcard->data.cert.file[i]); } if (smartcard->data.cert.database) { - if (strchr(smartcard->data.cert.database, ',')) { - virBufferFreeAndReset(&opt); - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid database name: %s"), - smartcard->data.cert.database); - return -1; - } - database = smartcard->data.cert.database; + virBufferAddLit(&opt, ",db="); + virQEMUBuildBufferEscapeComma(&opt, smartcard->data.cert.database); } else { - database = VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE; + virBufferAsprintf(&opt, ",db=%s", VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE); } - virBufferAsprintf(&opt, ",db=%s", database); break; case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH: -- 2.16.2 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
