[libvirt] [PATCH 1/2] QEmu: support disk filenames with comma

[Eric Blake]

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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120309/41b4c032/attachment-0001.sig>