3:17 p.m.
This patch adds virQEMUBuildBufferEscapeComma to properly
escape commas in user provided data fields for qemu command
line processing.
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr(a)gmail.com>
---
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