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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list