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@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 \