[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [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 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_\.\+\-\\&amp;&quot;&apos;&lt;&gt;/%]+</param>
> +      <param name="pattern">/[a-zA-Z0-9_\.\+\-\\&amp;&quot;&apos;&lt;&gt;/%,]+</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/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]