[libvirt] QEmu: support disk filenames with comma
Hi! This patch fixes RHBZ #801036 adding support to filenames with comma using QEmu. As this is my first patch to libvirt, I'm not completely sure if I should have also changed other parts of the code, or if I have used the wrong memory allocation functions/macros, or if I shouldn't have created a separate commit to add my name to the AUTHORS file... :-) I also need to make it clear that this patch only fixes the problem in the "libvirt data -> QEmu args" way (e.g., when QEmu reads an XML file to start QEmu, or when using virsh domxml-to-native). The other way, "QEmu args -> libvirt data" is *not* implemented (e.g., virsh domxml-from-native). I would like to know if I'm going in the right way so I can continue working on this patch, and if that half of the feature is also needed. Any feedback is welcome, thanks! Best regards, Crístian.
If there is a disk file with a comma in the name, QEmu expects a double comma instead of a single one (e.g., the file "virtual,disk.img" needs to be specified as "virtual,,disk.img" in QEmu's command line). This patch fixes libvirt to work with that feature. Fix RHBZ #801036. diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index de2d4a1..685a278 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1628,6 +1628,48 @@ qemuSafeSerialParamValue(const char *value) return 0; } +static const char * +qemuEscapeCommas(const char *name) +{ + const char *name_ptr = name; + char *escaped_name, *escaped_name_ptr; + size_t escaped_name_max_size = (strlen(name) * 2) + 1; + + /* If there is no comma in the string, return a copy of the original one. */ + if (!strchr(name, ',')) { + if (!(escaped_name = strdup(name))) { + virReportOOMError(); + return NULL; + } + return escaped_name; + } + + /* In the worst case the string will have a sequence of only commas, + * which will require double the space for the escaped string. */ + if (VIR_ALLOC_N(escaped_name, escaped_name_max_size) < 0) { + virReportOOMError(); + return NULL; + } + + /* Insert a comma after each comma in the string. */ + escaped_name_ptr = escaped_name; + while (*name_ptr != '\0') { + if (*name_ptr == ',') { + *escaped_name_ptr++ = ','; + } + + *escaped_name_ptr++ = *name_ptr++; + } + + escaped_name_ptr = '\0'; + + /* Reduce the string memory size to its current length. */ + VIR_SHRINK_N(escaped_name, escaped_name_max_size, + strlen(escaped_name) - strlen(name)); + + return escaped_name; +} + static int qemuBuildRBDString(virConnectPtr conn, virDomainDiskDefPtr disk, @@ -1638,7 +1680,7 @@ qemuBuildRBDString(virConnectPtr conn, char *secret = NULL; size_t secret_size; - virBufferAsprintf(opt, "rbd:%s", disk->src); + virBufferAsprintf(opt, "rbd:%s", qemuEscapeCommas(disk->src)); if (disk->auth.username) { virBufferEscape(opt, ":", ":id=%s", disk->auth.username); /* look up secret */ @@ -1922,9 +1964,9 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, goto error; } if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - virBufferAsprintf(&opt, "file=fat:floppy:%s,", disk->src); + virBufferAsprintf(&opt, "file=fat:floppy:%s,", qemuEscapeCommas(disk->src)); else - virBufferAsprintf(&opt, "file=fat:%s,", disk->src); + virBufferAsprintf(&opt, "file=fat:%s,", qemuEscapeCommas(disk->src)); } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { switch (disk->protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: @@ -1944,16 +1986,16 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, break; case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: if (disk->nhosts == 0) - virBufferAsprintf(&opt, "file=sheepdog:%s,", disk->src); + virBufferAsprintf(&opt, "file=sheepdog:%s,", qemuEscapeCommas(disk->src)); else /* only one host is supported now */ virBufferAsprintf(&opt, "file=sheepdog:%s:%s:%s,", disk->hosts->name, disk->hosts->port, - disk->src); + qemuEscapeCommas(disk->src)); break; } } else { - virBufferAsprintf(&opt, "file=%s,", disk->src); + virBufferAsprintf(&opt, "file=%s,", qemuEscapeCommas(disk->src)); } } if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) @@ -2134,7 +2176,6 @@ error: return NULL; } - char * qemuBuildDriveDevStr(virDomainDefPtr def, virDomainDiskDefPtr disk, -- 1.7.8.5
On 03/09/2012 12:13 PM, Crístian Viana wrote:
If there is a disk file with a comma in the name, QEmu expects a double comma instead of a single one (e.g., the file "virtual,disk.img" needs to be specified as "virtual,,disk.img" in QEmu's command line). This patch fixes libvirt to work with that feature. Fix RHBZ #801036.
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index de2d4a1..685a278 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1628,6 +1628,48 @@ qemuSafeSerialParamValue(const char *value) return 0; }
+static const char *
Use 'char *', not 'const char *', when returning a malloc'd string; that gives the caller a hint that they should free what they are receiving.
+qemuEscapeCommas(const char *name) +{ + const char *name_ptr = name; + char *escaped_name, *escaped_name_ptr; + size_t escaped_name_max_size = (strlen(name) * 2) + 1;
Technically, this could overflow. Copying from virBufferEscape() in buf.c shows that we can use xalloc_oversized to avoid this (admittedly unlikely) scenario.
+ + /* Reduce the string memory size to its current length. */ + VIR_SHRINK_N(escaped_name, escaped_name_max_size, + strlen(escaped_name) - strlen(name));
Indentation. That's a lot of calls to strlen(name); it's nicer to cache it up front.
+ + return escaped_name; +} + static int qemuBuildRBDString(virConnectPtr conn, virDomainDiskDefPtr disk, @@ -1638,7 +1680,7 @@ qemuBuildRBDString(virConnectPtr conn, char *secret = NULL; size_t secret_size;
- virBufferAsprintf(opt, "rbd:%s", disk->src); + virBufferAsprintf(opt, "rbd:%s", qemuEscapeCommas(disk->src));
Memory leak. Not good. Oooh - we're printing to a virBuffer, and we already have virBufferEscape which _almost_ does what we want - it's just that we want to escape with ',' instead of '\\'. I think I have a more efficient solution coming on :)
@@ -2134,7 +2176,6 @@ error: return NULL; }
- char *
Spurious whitespace change. You also need a testsuite addition to ensure that this actually does what we want. And in fact, it didn't - we need to enhance the RNG to allow commas in file names to validate, and we need to fix the domxml-from-native routine after all. If I were to keep the patch authorship in your name, I would squash 2/2 into this one (otherwise, 'make syntax-check' will fail during a 'git bisect' that settles on 1/2 in isolation). But since I practically rewrote the thing, I will instead post a v2 with myself as author but mentioning your name in the commit message. Stay tuned... -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
diff --git a/AUTHORS b/AUTHORS index 954fd1a..b09bd62 100644 --- a/AUTHORS +++ b/AUTHORS @@ -224,6 +224,7 @@ Patches have also been contributed by: Peter Robinson <pbrobinson@gmail.com> Benjamin Cama <benoar@dolka.fr> Duncan Rance <libvirt@dunquino.com> + Crístian Viana <vianac@linux.vnet.ibm.com> [....send patches to get your name here....] -- 1.7.8.5
If there is a disk file with a comma in the name, QEmu expects a double comma instead of a single one (e.g., the file "virtual,disk.img" needs to be specified as "virtual,,disk.img" in QEmu's command line). This patch fixes libvirt to work with that feature. Fix RHBZ #801036. Based on an initial patch by Crístian Viana. * src/util/buf.h (virBufferEscape): Alter signature. * src/util/buf.c (virBufferEscape): Add parameter. (virBufferEscapeSexpr): Fix caller. * src/qemu/qemu_command.c (qemuBuildRBDString): Likewise. Also escape commas in file names. (qemuBuildDriveStr): Escape commas in file names. * docs/schemas/basictypes.rng (absFilePath): Relax RNG to allow commas in input file names. * tests/qemuxml2argvdata/*-disk-drive-network-sheepdog.*: Update test. Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/schemas/basictypes.rng | 2 +- src/qemu/qemu_command.c | 49 ++++++++++++++------ src/util/buf.c | 11 ++-- src/util/buf.h | 4 +- .../qemuxml2argv-disk-drive-network-sheepdog.args | 6 +- .../qemuxml2argv-disk-drive-network-sheepdog.xml | 4 +- 6 files changed, 49 insertions(+), 27 deletions(-) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index cc0bc12..7d7911d 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -128,7 +128,7 @@ <define name="absFilePath"> <data type="string"> - <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%]+</param> + <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%,]+</param> </data> </define> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6ec1eb9..daf3c89 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1629,6 +1629,7 @@ qemuSafeSerialParamValue(const char *value) return 0; } + static int qemuBuildRBDString(virConnectPtr conn, virDomainDiskDefPtr disk, @@ -1639,9 +1640,9 @@ qemuBuildRBDString(virConnectPtr conn, char *secret = NULL; size_t secret_size; - virBufferAsprintf(opt, "rbd:%s", disk->src); + virBufferEscape(opt, ',', ",", "rbd:%s", disk->src); if (disk->auth.username) { - virBufferEscape(opt, ":", ":id=%s", disk->auth.username); + virBufferEscape(opt, '\\', ":", ":id=%s", disk->auth.username); /* look up secret */ switch (disk->auth.secretType) { case VIR_DOMAIN_DISK_SECRET_TYPE_UUID: @@ -1672,7 +1673,8 @@ qemuBuildRBDString(virConnectPtr conn, virReportOOMError(); goto error; } - virBufferEscape(opt, ":", ":key=%s:auth_supported=cephx none", + virBufferEscape(opt, '\\', ":", + ":key=%s:auth_supported=cephx none", base64); VIR_FREE(base64); } else { @@ -1923,9 +1925,10 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, goto error; } if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - virBufferAsprintf(&opt, "file=fat:floppy:%s,", disk->src); + virBufferEscape(&opt, ',', ",", "file=fat:floppy:%s,", + disk->src); else - virBufferAsprintf(&opt, "file=fat:%s,", disk->src); + virBufferEscape(&opt, ',', ",", "file=fat:%s,", disk->src); } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { switch (disk->protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: @@ -1944,17 +1947,19 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferAddChar(&opt, ','); break; case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: - if (disk->nhosts == 0) - virBufferAsprintf(&opt, "file=sheepdog:%s,", disk->src); - else + if (disk->nhosts == 0) { + virBufferEscape(&opt, ',', ",", "file=sheepdog:%s,", + disk->src); + } else { /* only one host is supported now */ - virBufferAsprintf(&opt, "file=sheepdog:%s:%s:%s,", - disk->hosts->name, disk->hosts->port, - disk->src); + virBufferAsprintf(&opt, "file=sheepdog:%s:%s:", + disk->hosts->name, disk->hosts->port); + virBufferEscape(&opt, ',', ",", "%s,", disk->src); + } break; } } else { - virBufferAsprintf(&opt, "file=%s,", disk->src); + virBufferEscape(&opt, ',', ",", "file=%s,", disk->src); } } if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) @@ -2135,7 +2140,6 @@ error: return NULL; } - char * qemuBuildDriveDevStr(virDomainDefPtr def, virDomainDiskDefPtr disk, @@ -6130,7 +6134,14 @@ qemuParseKeywords(const char *str, char *keyword; char *value = NULL; - if (!(endmark = strchr(start, ','))) + endmark = start; + do { + /* Qemu accepts ',,' as an escape for a literal comma; + * skip past those here while searching for the end of the + * value, then strip them down below */ + endmark = strchr(endmark, ','); + } while (endmark && endmark[1] == ',' && (endmark += 2)); + if (!endmark) endmark = end; if (!(separator = strchr(start, '='))) separator = end; @@ -6153,6 +6164,16 @@ qemuParseKeywords(const char *str, VIR_FREE(keyword); goto no_memory; } + if (strchr(value, ',')) { + char *p = strchr(value, ',') + 1; + char *q = p + 1; + while (*q) { + if (*q == ',') + q++; + *p++ = *q++; + } + *p = '\0'; + } } if (keywordAlloc == keywordCount) { diff --git a/src/util/buf.c b/src/util/buf.c index 206a39a..d18f6af 100644 --- a/src/util/buf.c +++ b/src/util/buf.c @@ -1,7 +1,7 @@ /* * buf.c: buffers for libvirt * - * Copyright (C) 2005-2008, 2010-2011 Red Hat, Inc. + * Copyright (C) 2005-2008, 2010-2012 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -423,22 +423,23 @@ virBufferEscapeSexpr(virBufferPtr buf, const char *format, const char *str) { - virBufferEscape(buf, "\\'", format, str); + virBufferEscape(buf, '\\', "\\'", format, str); } /** * virBufferEscape: * @buf: the buffer to append to + * @escape: the escape character to inject * @toescape: NUL-terminated list of characters to escape * @format: a printf like format string but with only one %s parameter * @str: the string argument which needs to be escaped * * Do a formatted print with a single string to a buffer. Any characters - * in the provided list are escaped with a preceeding \. Auto indentation + * in the provided list are escaped with the given escape. Auto indentation * may be applied. */ void -virBufferEscape(virBufferPtr buf, const char *toescape, +virBufferEscape(virBufferPtr buf, char escape, const char *toescape, const char *format, const char *str) { int len; @@ -471,7 +472,7 @@ virBufferEscape(virBufferPtr buf, const char *toescape, */ char needle[2] = { *cur, 0 }; if (strstr(toescape, needle)) - *out++ = '\\'; + *out++ = escape; *out++ = *cur; cur++; } diff --git a/src/util/buf.h b/src/util/buf.h index 1a8ebfb..6648b23 100644 --- a/src/util/buf.h +++ b/src/util/buf.h @@ -1,7 +1,7 @@ /* * buf.h: buffers for libvirt * - * Copyright (C) 2005-2008, 2011 Red Hat, Inc. + * Copyright (C) 2005-2008, 2011, 2012 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -49,7 +49,7 @@ void virBufferVasprintf(virBufferPtr buf, const char *format, va_list ap) ATTRIBUTE_FMT_PRINTF(2, 0); void virBufferStrcat(virBufferPtr buf, ...) ATTRIBUTE_SENTINEL; -void virBufferEscape(virBufferPtr buf, const char *toescape, +void virBufferEscape(virBufferPtr buf, char escape, const char *toescape, const char *format, const char *str); void virBufferEscapeString(virBufferPtr buf, const char *format, const char *str); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args index 4b3f816..c6089e1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ -file=sheepdog:example.org:6000:image,if=virtio,format=raw -net none -serial \ -none -parallel none -usb +-no-acpi -boot c -drive file=/dev/HostVG/QEMU,,Guest,,1,if=ide,bus=0,unit=0 \ +-drive file=sheepdog:example.org:6000:image,,with,,commas,if=virtio,format=raw \ +-net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml index 04cfd3c..790ca92 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml @@ -15,13 +15,13 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest1'/> + <source dev='/dev/HostVG/QEMU,Guest,1'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='network' device='disk'> <driver name='qemu' type='raw'/> - <source protocol='sheepdog' name='image'> + <source protocol='sheepdog' name='image,with,commas'> <host name='example.org' port='6000'/> </source> <target dev='vda' bus='virtio'/> -- 1.7.7.6
On Fri, Mar 09, 2012 at 05:40:19PM -0700, Eric Blake wrote:
If there is a disk file with a comma in the name, QEmu expects a double comma instead of a single one (e.g., the file "virtual,disk.img" needs to be specified as "virtual,,disk.img" in QEmu's command line). This patch fixes libvirt to work with that feature. Fix RHBZ #801036.
Based on an initial patch by Crístian Viana.
* src/util/buf.h (virBufferEscape): Alter signature. * src/util/buf.c (virBufferEscape): Add parameter. (virBufferEscapeSexpr): Fix caller. * src/qemu/qemu_command.c (qemuBuildRBDString): Likewise. Also escape commas in file names. (qemuBuildDriveStr): Escape commas in file names. * docs/schemas/basictypes.rng (absFilePath): Relax RNG to allow commas in input file names. * tests/qemuxml2argvdata/*-disk-drive-network-sheepdog.*: Update test.
Signed-off-by: Eric Blake <eblake@redhat.com> --- docs/schemas/basictypes.rng | 2 +- src/qemu/qemu_command.c | 49 ++++++++++++++------ src/util/buf.c | 11 ++-- src/util/buf.h | 4 +- .../qemuxml2argv-disk-drive-network-sheepdog.args | 6 +- .../qemuxml2argv-disk-drive-network-sheepdog.xml | 4 +- 6 files changed, 49 insertions(+), 27 deletions(-)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index cc0bc12..7d7911d 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -128,7 +128,7 @@
<define name="absFilePath"> <data type="string"> - <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%]+</param> + <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%,]+</param> </data> </define>
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6ec1eb9..daf3c89 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1629,6 +1629,7 @@ qemuSafeSerialParamValue(const char *value) return 0; }
+ static int qemuBuildRBDString(virConnectPtr conn, virDomainDiskDefPtr disk, @@ -1639,9 +1640,9 @@ qemuBuildRBDString(virConnectPtr conn, char *secret = NULL; size_t secret_size;
- virBufferAsprintf(opt, "rbd:%s", disk->src); + virBufferEscape(opt, ',', ",", "rbd:%s", disk->src); if (disk->auth.username) { - virBufferEscape(opt, ":", ":id=%s", disk->auth.username); + virBufferEscape(opt, '\\', ":", ":id=%s", disk->auth.username); /* look up secret */ switch (disk->auth.secretType) { case VIR_DOMAIN_DISK_SECRET_TYPE_UUID: @@ -1672,7 +1673,8 @@ qemuBuildRBDString(virConnectPtr conn, virReportOOMError(); goto error; } - virBufferEscape(opt, ":", ":key=%s:auth_supported=cephx none", + virBufferEscape(opt, '\\', ":", + ":key=%s:auth_supported=cephx none", base64); VIR_FREE(base64); } else { @@ -1923,9 +1925,10 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, goto error; } if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - virBufferAsprintf(&opt, "file=fat:floppy:%s,", disk->src); + virBufferEscape(&opt, ',', ",", "file=fat:floppy:%s,", + disk->src); else - virBufferAsprintf(&opt, "file=fat:%s,", disk->src); + virBufferEscape(&opt, ',', ",", "file=fat:%s,", disk->src); } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { switch (disk->protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: @@ -1944,17 +1947,19 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferAddChar(&opt, ','); break; case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: - if (disk->nhosts == 0) - virBufferAsprintf(&opt, "file=sheepdog:%s,", disk->src); - else + if (disk->nhosts == 0) { + virBufferEscape(&opt, ',', ",", "file=sheepdog:%s,", + disk->src); + } else { /* only one host is supported now */ - virBufferAsprintf(&opt, "file=sheepdog:%s:%s:%s,", - disk->hosts->name, disk->hosts->port, - disk->src); + virBufferAsprintf(&opt, "file=sheepdog:%s:%s:", + disk->hosts->name, disk->hosts->port); + virBufferEscape(&opt, ',', ",", "%s,", disk->src); + } break; } } else { - virBufferAsprintf(&opt, "file=%s,", disk->src); + virBufferEscape(&opt, ',', ",", "file=%s,", disk->src); } } if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) @@ -2135,7 +2140,6 @@ error: return NULL; }
- char * qemuBuildDriveDevStr(virDomainDefPtr def, virDomainDiskDefPtr disk, @@ -6130,7 +6134,14 @@ qemuParseKeywords(const char *str, char *keyword; char *value = NULL;
- if (!(endmark = strchr(start, ','))) + endmark = start; + do { + /* Qemu accepts ',,' as an escape for a literal comma; + * skip past those here while searching for the end of the + * value, then strip them down below */ + endmark = strchr(endmark, ','); + } while (endmark && endmark[1] == ',' && (endmark += 2)); + if (!endmark) endmark = end; if (!(separator = strchr(start, '='))) separator = end; @@ -6153,6 +6164,16 @@ qemuParseKeywords(const char *str, VIR_FREE(keyword); goto no_memory; } + if (strchr(value, ',')) { + char *p = strchr(value, ',') + 1; + char *q = p + 1; + while (*q) { + if (*q == ',') + q++; + *p++ = *q++; + } + *p = '\0'; + } }
if (keywordAlloc == keywordCount) { diff --git a/src/util/buf.c b/src/util/buf.c index 206a39a..d18f6af 100644 --- a/src/util/buf.c +++ b/src/util/buf.c @@ -1,7 +1,7 @@ /* * buf.c: buffers for libvirt * - * Copyright (C) 2005-2008, 2010-2011 Red Hat, Inc. + * Copyright (C) 2005-2008, 2010-2012 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -423,22 +423,23 @@ virBufferEscapeSexpr(virBufferPtr buf, const char *format, const char *str) { - virBufferEscape(buf, "\\'", format, str); + virBufferEscape(buf, '\\', "\\'", format, str); }
/** * virBufferEscape: * @buf: the buffer to append to + * @escape: the escape character to inject * @toescape: NUL-terminated list of characters to escape * @format: a printf like format string but with only one %s parameter * @str: the string argument which needs to be escaped * * Do a formatted print with a single string to a buffer. Any characters - * in the provided list are escaped with a preceeding \. Auto indentation + * in the provided list are escaped with the given escape. Auto indentation * may be applied. */ void -virBufferEscape(virBufferPtr buf, const char *toescape, +virBufferEscape(virBufferPtr buf, char escape, const char *toescape, const char *format, const char *str) { int len; @@ -471,7 +472,7 @@ virBufferEscape(virBufferPtr buf, const char *toescape, */ char needle[2] = { *cur, 0 }; if (strstr(toescape, needle)) - *out++ = '\\'; + *out++ = escape; *out++ = *cur; cur++; } diff --git a/src/util/buf.h b/src/util/buf.h index 1a8ebfb..6648b23 100644 --- a/src/util/buf.h +++ b/src/util/buf.h @@ -1,7 +1,7 @@ /* * buf.h: buffers for libvirt * - * Copyright (C) 2005-2008, 2011 Red Hat, Inc. + * Copyright (C) 2005-2008, 2011, 2012 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -49,7 +49,7 @@ void virBufferVasprintf(virBufferPtr buf, const char *format, va_list ap) ATTRIBUTE_FMT_PRINTF(2, 0); void virBufferStrcat(virBufferPtr buf, ...) ATTRIBUTE_SENTINEL; -void virBufferEscape(virBufferPtr buf, const char *toescape, +void virBufferEscape(virBufferPtr buf, char escape, const char *toescape, const char *format, const char *str); void virBufferEscapeString(virBufferPtr buf, const char *format, const char *str); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args index 4b3f816..c6089e1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ -file=sheepdog:example.org:6000:image,if=virtio,format=raw -net none -serial \ -none -parallel none -usb +-no-acpi -boot c -drive file=/dev/HostVG/QEMU,,Guest,,1,if=ide,bus=0,unit=0 \ +-drive file=sheepdog:example.org:6000:image,,with,,commas,if=virtio,format=raw \ +-net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml index 04cfd3c..790ca92 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml @@ -15,13 +15,13 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest1'/> + <source dev='/dev/HostVG/QEMU,Guest,1'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='network' device='disk'> <driver name='qemu' type='raw'/> - <source protocol='sheepdog' name='image'> + <source protocol='sheepdog' name='image,with,commas'> <host name='example.org' port='6000'/> </source> <target dev='vda' bus='virtio'/>
I was suprized first by the few occurence of virBufferEscape in the patch as we change its signature but we mostly use the wrapper virBufferEscapeString within libvirt code, ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 03/10/2012 08:40 AM, Eric Blake wrote:
If there is a disk file with a comma in the name, QEmu expects a double comma instead of a single one (e.g., the file "virtual,disk.img" needs to be specified as "virtual,,disk.img" in QEmu's command line). This patch fixes libvirt to work with that feature. Fix RHBZ #801036.
Based on an initial patch by Crístian Viana.
* src/util/buf.h (virBufferEscape): Alter signature. * src/util/buf.c (virBufferEscape): Add parameter. (virBufferEscapeSexpr): Fix caller. * src/qemu/qemu_command.c (qemuBuildRBDString): Likewise. Also escape commas in file names. (qemuBuildDriveStr): Escape commas in file names. * docs/schemas/basictypes.rng (absFilePath): Relax RNG to allow commas in input file names. * tests/qemuxml2argvdata/*-disk-drive-network-sheepdog.*: Update test.
Signed-off-by: Eric Blake<eblake@redhat.com> --- docs/schemas/basictypes.rng | 2 +- src/qemu/qemu_command.c | 49 ++++++++++++++------ src/util/buf.c | 11 ++-- src/util/buf.h | 4 +- .../qemuxml2argv-disk-drive-network-sheepdog.args | 6 +- .../qemuxml2argv-disk-drive-network-sheepdog.xml | 4 +- 6 files changed, 49 insertions(+), 27 deletions(-)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index cc0bc12..7d7911d 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -128,7 +128,7 @@
<define name="absFilePath"> <data type="string"> -<param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%]+</param> +<param name="pattern">/[a-zA-Z0-9_\.\+\-\\&"'<>/%,]+</param> </data> </define>
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6ec1eb9..daf3c89 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1629,6 +1629,7 @@ qemuSafeSerialParamValue(const char *value) return 0; }
+ static int qemuBuildRBDString(virConnectPtr conn, virDomainDiskDefPtr disk, @@ -1639,9 +1640,9 @@ qemuBuildRBDString(virConnectPtr conn, char *secret = NULL; size_t secret_size;
- virBufferAsprintf(opt, "rbd:%s", disk->src); + virBufferEscape(opt, ',', ",", "rbd:%s", disk->src); if (disk->auth.username) { - virBufferEscape(opt, ":", ":id=%s", disk->auth.username); + virBufferEscape(opt, '\\', ":", ":id=%s", disk->auth.username); /* look up secret */ switch (disk->auth.secretType) { case VIR_DOMAIN_DISK_SECRET_TYPE_UUID: @@ -1672,7 +1673,8 @@ qemuBuildRBDString(virConnectPtr conn, virReportOOMError(); goto error; } - virBufferEscape(opt, ":", ":key=%s:auth_supported=cephx none", + virBufferEscape(opt, '\\', ":", + ":key=%s:auth_supported=cephx none", base64); VIR_FREE(base64); } else { @@ -1923,9 +1925,10 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, goto error; } if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) - virBufferAsprintf(&opt, "file=fat:floppy:%s,", disk->src); + virBufferEscape(&opt, ',', ",", "file=fat:floppy:%s,", + disk->src); else - virBufferAsprintf(&opt, "file=fat:%s,", disk->src); + virBufferEscape(&opt, ',', ",", "file=fat:%s,", disk->src); } else if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { switch (disk->protocol) { case VIR_DOMAIN_DISK_PROTOCOL_NBD: @@ -1944,17 +1947,19 @@ qemuBuildDriveStr(virConnectPtr conn ATTRIBUTE_UNUSED, virBufferAddChar(&opt, ','); break; case VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG: - if (disk->nhosts == 0) - virBufferAsprintf(&opt, "file=sheepdog:%s,", disk->src); - else + if (disk->nhosts == 0) { + virBufferEscape(&opt, ',', ",", "file=sheepdog:%s,", + disk->src); + } else { /* only one host is supported now */ - virBufferAsprintf(&opt, "file=sheepdog:%s:%s:%s,", - disk->hosts->name, disk->hosts->port, - disk->src); + virBufferAsprintf(&opt, "file=sheepdog:%s:%s:", + disk->hosts->name, disk->hosts->port); + virBufferEscape(&opt, ',', ",", "%s,", disk->src); + } break; } } else { - virBufferAsprintf(&opt, "file=%s,", disk->src); + virBufferEscape(&opt, ',', ",", "file=%s,", disk->src); } } if (qemuCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) @@ -2135,7 +2140,6 @@ error: return NULL; }
- char * qemuBuildDriveDevStr(virDomainDefPtr def, virDomainDiskDefPtr disk, @@ -6130,7 +6134,14 @@ qemuParseKeywords(const char *str, char *keyword; char *value = NULL;
- if (!(endmark = strchr(start, ','))) + endmark = start; + do { + /* Qemu accepts ',,' as an escape for a literal comma; + * skip past those here while searching for the end of the + * value, then strip them down below */ + endmark = strchr(endmark, ','); + } while (endmark&& endmark[1] == ','&& (endmark += 2)); + if (!endmark) endmark = end; if (!(separator = strchr(start, '='))) separator = end; @@ -6153,6 +6164,16 @@ qemuParseKeywords(const char *str, VIR_FREE(keyword); goto no_memory; } + if (strchr(value, ',')) { + char *p = strchr(value, ',') + 1; + char *q = p + 1; + while (*q) { + if (*q == ',') + q++; + *p++ = *q++; + } + *p = '\0'; + } }
if (keywordAlloc == keywordCount) { diff --git a/src/util/buf.c b/src/util/buf.c index 206a39a..d18f6af 100644 --- a/src/util/buf.c +++ b/src/util/buf.c @@ -1,7 +1,7 @@ /* * buf.c: buffers for libvirt * - * Copyright (C) 2005-2008, 2010-2011 Red Hat, Inc. + * Copyright (C) 2005-2008, 2010-2012 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -423,22 +423,23 @@ virBufferEscapeSexpr(virBufferPtr buf, const char *format, const char *str) { - virBufferEscape(buf, "\\'", format, str); + virBufferEscape(buf, '\\', "\\'", format, str); }
/** * virBufferEscape: * @buf: the buffer to append to + * @escape: the escape character to inject * @toescape: NUL-terminated list of characters to escape * @format: a printf like format string but with only one %s parameter * @str: the string argument which needs to be escaped * * Do a formatted print with a single string to a buffer. Any characters - * in the provided list are escaped with a preceeding \. Auto indentation + * in the provided list are escaped with the given escape. Auto indentation * may be applied. */ void -virBufferEscape(virBufferPtr buf, const char *toescape, +virBufferEscape(virBufferPtr buf, char escape, const char *toescape, const char *format, const char *str) { int len; @@ -471,7 +472,7 @@ virBufferEscape(virBufferPtr buf, const char *toescape, */ char needle[2] = { *cur, 0 }; if (strstr(toescape, needle)) - *out++ = '\\'; + *out++ = escape; *out++ = *cur; cur++; } diff --git a/src/util/buf.h b/src/util/buf.h index 1a8ebfb..6648b23 100644 --- a/src/util/buf.h +++ b/src/util/buf.h @@ -1,7 +1,7 @@ /* * buf.h: buffers for libvirt * - * Copyright (C) 2005-2008, 2011 Red Hat, Inc. + * Copyright (C) 2005-2008, 2011, 2012 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -49,7 +49,7 @@ void virBufferVasprintf(virBufferPtr buf, const char *format, va_list ap) ATTRIBUTE_FMT_PRINTF(2, 0); void virBufferStrcat(virBufferPtr buf, ...) ATTRIBUTE_SENTINEL; -void virBufferEscape(virBufferPtr buf, const char *toescape, +void virBufferEscape(virBufferPtr buf, char escape, const char *toescape, const char *format, const char *str); void virBufferEscapeString(virBufferPtr buf, const char *format, const char *str); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args index 4b3f816..c6089e1 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.args @@ -1,5 +1,5 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -monitor unix:/tmp/test-monitor,server,nowait \ --no-acpi -boot c -drive file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \ -file=sheepdog:example.org:6000:image,if=virtio,format=raw -net none -serial \ -none -parallel none -usb +-no-acpi -boot c -drive file=/dev/HostVG/QEMU,,Guest,,1,if=ide,bus=0,unit=0 \ +-drive file=sheepdog:example.org:6000:image,,with,,commas,if=virtio,format=raw \ +-net none -serial none -parallel none -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml index 04cfd3c..790ca92 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-sheepdog.xml @@ -15,13 +15,13 @@ <devices> <emulator>/usr/bin/qemu</emulator> <disk type='block' device='disk'> -<source dev='/dev/HostVG/QEMUGuest1'/> +<source dev='/dev/HostVG/QEMU,Guest,1'/> <target dev='hda' bus='ide'/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='network' device='disk'> <driver name='qemu' type='raw'/> -<source protocol='sheepdog' name='image'> +<source protocol='sheepdog' name='image,with,commas'> <host name='example.org' port='6000'/> </source> <target dev='vda' bus='virtio'/>
ACK.
On 03/12/2012 02:50 AM, Osier Yang wrote:
On 03/10/2012 08:40 AM, Eric Blake wrote:
If there is a disk file with a comma in the name, QEmu expects a double comma instead of a single one (e.g., the file "virtual,disk.img" needs to be specified as "virtual,,disk.img" in QEmu's command line). This patch fixes libvirt to work with that feature. Fix RHBZ #801036.
Based on an initial patch by Crístian Viana.
ACK.
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
Crístian Viana -
Daniel Veillard -
Eric Blake -
Osier Yang