1:13 p.m.
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.
1:13 p.m.
New subject: [libvirt] [PATCH 1/2] QEmu: support disk filenames with comma
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
6:34 p.m.
New subject: [libvirt] [PATCH 1/2] QEmu: support disk filenames with comma
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
1:13 p.m.
New subject: [libvirt] [PATCH 2/2] Add myself to the AUTHORS file
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(a)gmail.com>
Benjamin Cama <benoar(a)dolka.fr>
Duncan Rance <libvirt(a)dunquino.com>
+ Crístian Viana <vianac(a)linux.vnet.ibm.com>
[....send patches to get your name here....]
--
1.7.8.5
6:40 p.m.
New subject: [libvirt] [PATCHv2] qemu: support disk filenames with comma
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(a)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
2:38 a.m.
New subject: [libvirt] [PATCHv2] qemu: support disk filenames with comma
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(a)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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
3:50 a.m.
New subject: [libvirt] [PATCHv2] qemu: support disk filenames with comma
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(a)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.
9:44 a.m.
New subject: [libvirt] [PATCHv2] qemu: support disk filenames with comma
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(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4636
days inactive
4641
days old
8 comments
4 participants
participants (4)
-
Crístian Viana -
Daniel Veillard -
Eric Blake -
Osier Yang