[libvirt] [PATCH 1/3] conf: Properly truncate wide character names in virDomainObjGetShortName

John Ferlan jferlan at redhat.com
Wed Aug 23 20:47:03 UTC 2017 


On 08/23/2017 07:47 AM, Martin Kletzander wrote:
> We always truncated the name at 20 bytes instead of characters.  In
> case 20 bytes were in the middle of a multi-byte character, then the
> string became invalid and various parts of the code would error
> out (e.g. XML parsing of that string).  Let's instead properly
> truncate it after 20 characters instead.
> 
> In some cases the truncation didn't even work (as can be seen from the
> modified tests), which is fixed by this as well.

Didn't work as well?  Try at all...

As a test I changed the name of pcie-expander-bus to 80 characters and
all 80 were printed.  I think this goes against your original intention
of a shortname...

> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1448766
> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  src/conf/domain_conf.c                             | 30 +++++++++++++++++++---
>  .../qemuxml2argv-aarch64-virt-default-nic.args     |  2 +-
>  .../qemuxml2argv-hugepages-pages2.args             |  4 +--
>  .../qemuxml2argv-hugepages-pages3.args             |  5 ++--
>  .../qemuxml2argv-hugepages-pages5.args             |  4 +--
>  .../qemuxml2argv-hugepages-pages6.args             |  2 +-
>  .../qemuxml2argv-pcie-expander-bus.args            |  2 +-
>  7 files changed, 37 insertions(+), 12 deletions(-)
> 

Can we add a multibyte name test?  These just modify existing tests (I
think wrongly too).


> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 47eba4dbb315..9a46ceece2d6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -27131,6 +27131,8 @@ virDomainDefHasMemballoon(const virDomainDef *def)
>  }
>  
>  
> +#define VIR_DOMAIN_SHORT_NAME_MAX 20
> +
>  /**
>   * virDomainObjGetShortName:
>   * @vm: Machine for which to get a name
> @@ -27141,15 +27143,37 @@ virDomainDefHasMemballoon(const virDomainDef *def)
>  char *
>  virDomainObjGetShortName(const virDomainDef *def)
>  {
> -    const int dommaxlen = 20;
> +    wchar_t wshortname[VIR_DOMAIN_SHORT_NAME_MAX + 1] = {0};
> +    size_t len = 0;
> +    char *shortname = NULL;
>      char *ret = NULL;
>  

It would seem that we could figure there were multibyte chars in the
name "if strlen(def->name) != mbstowcs(NULL, def->name, 0)" and not do
any conversion, leaving the old code "as is" (mostly) and then
implementing something for these wide characters.

Of course I'm not getting anything other than -1 returned for the
mbstowcs and I didn't really want to dig any further, so I leave it up
to you!

I tried to modify the same test and change the name to "ÄèÎØÛ", but the
mbstowcs kept returning -1, until I added:

    if (setlocale(LC_ALL, "en_US.UTF-8") == NULL) {

But how does one know which locale to use? From my quick read of the
mbstowcs man page it seems a locale needs to be set first. I also tried
the man page example of "Grüße!" and that worked with UTF-8.

> -    ignore_value(virAsprintf(&ret, "%d-%.*s",
> -                             def->id, dommaxlen, def->name));
> +    if (mbstowcs(wshortname, def->name, VIR_DOMAIN_SHORT_NAME_MAX) == (size_t) -1) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("Cannot convert domain name to wide character string"));
> +        return NULL;
> +    }
> +
> +    len = wcstombs(NULL, wshortname, 0);
> +    if (len == (size_t) -1)
> +        return NULL;
>  
> +    if (VIR_ALLOC_N(shortname, len + 1) < 0)
> +        return NULL;
> +
> +    if (wcstombs(shortname, wshortname, len) == (size_t) -1) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("Cannot convert domain name from wide character string"));
> +        goto cleanup;
> +    }
> +
> +    ignore_value(virAsprintf(&ret, "%d-%s", def->id, def->name));

You go through all this trouble and still write def->name! I didn't
realize that at first either - kept wondering why the whole damn name
was being printed!

Anyway - I'll wait for v2.

John

> + cleanup:
> +    VIR_FREE(shortname);
>      return ret;
>  }
>  
> +#undef VIR_DOMAIN_SHORT_NAME_MAX
>  
>  int
>  virDomainGetBlkioParametersAssignFromDef(virDomainDefPtr def,
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.args b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.args
> index f27fe0a1d364..08930df8a218 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virt-default-nic.args
> @@ -16,7 +16,7 @@ QEMU_AUDIO_DRV=none \
>  -nodefconfig \
>  -nodefaults \
>  -chardev socket,id=charmonitor,\
> -path=/tmp/lib/domain--1-aarch64-virt-default/monitor.sock,server,nowait \
> +path=/tmp/lib/domain--1-aarch64-virt-default-nic/monitor.sock,server,nowait \
>  -mon chardev=charmonitor,id=monitor,mode=readline \
>  -no-acpi \
>  -boot c \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
> index 7ea277a7cd65..ab0eb7ff0ca3 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages2.args
> @@ -11,14 +11,14 @@ QEMU_AUDIO_DRV=none \
>  -m 1024 \
>  -smp 2,sockets=2,cores=1,threads=1 \
>  -mem-prealloc \
> --mem-path /dev/hugepages2M/libvirt/qemu/-1-SomeDummyHugepagesGu \
> +-mem-path /dev/hugepages2M/libvirt/qemu/-1-SomeDummyHugepagesGuest \
>  -numa node,nodeid=0,cpus=0,mem=256 \
>  -numa node,nodeid=1,cpus=1,mem=768 \
>  -uuid ef1bdff4-27f3-4e85-a807-5fb4d58463cc \
>  -nographic \
>  -nodefaults \
>  -chardev socket,id=charmonitor,\
> -path=/tmp/lib/domain--1-SomeDummyHugepagesGu/monitor.sock,server,nowait \
> +path=/tmp/lib/domain--1-SomeDummyHugepagesGuest/monitor.sock,server,nowait \
>  -mon chardev=charmonitor,id=monitor,mode=readline \
>  -no-acpi \
>  -boot c \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
> index 2291d6d72e8a..4794a940b1b3 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages3.args
> @@ -13,13 +13,14 @@ QEMU_AUDIO_DRV=none \
>  -object memory-backend-ram,id=ram-node0,size=268435456 \
>  -numa node,nodeid=0,cpus=0,memdev=ram-node0 \
>  -object memory-backend-file,id=ram-node1,prealloc=yes,\
> -mem-path=/dev/hugepages1G/libvirt/qemu/-1-SomeDummyHugepagesGu,size=805306368 \
> +mem-path=/dev/hugepages1G/libvirt/qemu/-1-SomeDummyHugepagesGuest,\
> +size=805306368 \
>  -numa node,nodeid=1,cpus=1,memdev=ram-node1 \
>  -uuid ef1bdff4-27f3-4e85-a807-5fb4d58463cc \
>  -nographic \
>  -nodefaults \
>  -chardev socket,id=charmonitor,\
> -path=/tmp/lib/domain--1-SomeDummyHugepagesGu/monitor.sock,server,nowait \
> +path=/tmp/lib/domain--1-SomeDummyHugepagesGuest/monitor.sock,server,nowait \
>  -mon chardev=charmonitor,id=monitor,mode=readline \
>  -no-acpi \
>  -boot c \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.args
> index c5bf7784ec23..124dd578f390 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages5.args
> @@ -10,13 +10,13 @@ QEMU_AUDIO_DRV=none \
>  -M pc \
>  -m 1024 \
>  -mem-prealloc \
> --mem-path /dev/hugepages2M/libvirt/qemu/-1-SomeDummyHugepagesGu \
> +-mem-path /dev/hugepages2M/libvirt/qemu/-1-SomeDummyHugepagesGuest \
>  -smp 2,sockets=2,cores=1,threads=1 \
>  -uuid ef1bdff4-27f3-4e85-a807-5fb4d58463cc \
>  -nographic \
>  -nodefaults \
>  -chardev socket,id=charmonitor,\
> -path=/tmp/lib/domain--1-SomeDummyHugepagesGu/monitor.sock,server,nowait \
> +path=/tmp/lib/domain--1-SomeDummyHugepagesGuest/monitor.sock,server,nowait \
>  -mon chardev=charmonitor,id=monitor,mode=readline \
>  -no-acpi \
>  -boot c \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.args b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.args
> index c1cc0017f335..31a17db44994 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages6.args
> @@ -14,7 +14,7 @@ QEMU_AUDIO_DRV=none \
>  -nographic \
>  -nodefaults \
>  -chardev socket,id=charmonitor,\
> -path=/tmp/lib/domain--1-SomeDummyHugepagesGu/monitor.sock,server,nowait \
> +path=/tmp/lib/domain--1-SomeDummyHugepagesGuest/monitor.sock,server,nowait \
>  -mon chardev=charmonitor,id=monitor,mode=readline \
>  -no-acpi \
>  -boot c \
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-expander-bus.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-expander-bus.args
> index 23852b45e56a..f42a69fc3fd1 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-expander-bus.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-expander-bus.args
> @@ -16,7 +16,7 @@ QEMU_AUDIO_DRV=none \
>  -nographic \
>  -nodefaults \
>  -chardev socket,id=charmonitor,\
> -path=/tmp/lib/domain--1-pcie-expander-bus-te/monitor.sock,server,nowait \
> +path=/tmp/lib/domain--1-pcie-expander-bus-test/monitor.sock,server,nowait \
>  -mon chardev=charmonitor,id=monitor,mode=readline \
>  -no-acpi \
>  -boot c \
>

More information about the libvir-list mailing list