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@xxxxxxxxxx> > --- > 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 \ > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
