Re: [libvirt] [PATCH 00/16] Revert Jansson usage
On 08/13/2018 06:55 AM, Ján Tomko wrote: For QEMU, we need a JSON parser that is able to handle its non-compliant JSON usage: https://bugzilla.redhat.com/show_bug.cgi?id=1614569 QEMU is not non-compliant in its use of JSON. Rather, https://tools.ietf.org/html/rfc7159 states that "This specification allows implementations to set limits on the range and precision of numbers accepted." and Jansson has merely chosen to enforce tighter limits than what qemu emits, with no (obvious) recourse into being patched to allow us to hook in our own parser to work around Jansson's inherent limits. Unfortunately, this does not seem to be possible with Jansson. Revert back to using yajl, which also lets us get rid of the 'dlopen' hacks and their bugfixes. (The QEMU default changes are not strictly required, but the patches specifically use JANSSON) That said, I'm not opposed to the revert, if we can't convince the Jansson developers of our need. :( -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/16] Revert Jansson usage
On Mon, Aug 13, 2018 at 01:58:56PM +0100, Daniel P. Berrangé wrote: On Mon, Aug 13, 2018 at 02:54:00PM +0200, Ján Tomko wrote: On Mon, Aug 13, 2018 at 01:55:10PM +0200, Ján Tomko wrote: > For QEMU, we need a JSON parser that is able to handle its non-compliant > JSON usage: > https://bugzilla.redhat.com/show_bug.cgi?id=1614569 > > Unfortunately, this does not seem to be possible with Jansson. > > Revert back to using yajl, which also lets us get rid of the 'dlopen' > hacks and their bugfixes. > > (The QEMU default changes are not strictly required, but the patches > specifically use JANSSON) > > Also available on: > git://repo.or.cz/libvirt/jtomko.git revert > > Ján Tomko (16): > Revert "src: Move DLOPEN_LIBS to libraries introducing the dependency" Note that this patch broke the build with clang by adding DLOPEN_LIBS to libvirt_setuid_rpc_client_la_CFLAGS instead of LDFLAGS: https://www.redhat.com/archives/libvir-list/2018-August/msg00604.html So technically this series is a build breaker fix ;) > Revert "Fix link errors in tools/nss and tests" > Revert "remote: daemon: Make sure that JSON symbols are properly >loaded at startup" > Revert "util: jsoncompat: Stub out virJSONInitialize when compiling >without jansson" > Revert "tests: qemucapsprobe: Fix output after switching to jansson" > Revert "util: avoid symbol clash between json libraries" > Revert "tests: also skip qemuagenttest with old jansson" > Revert "m4: Introduce STABLE_ORDERING_JANSSON" > Revert "build: require Jansson if QEMU driver is enabled" > Revert "build: switch --with-qemu default from yes to check" > Revert "Remove virJSONValueNewStringLen" > Revert "build: remove references to WITH_YAJL for SETUID_RPC_CLIENT" > Revert "Remove functions using yajl" > Revert "Switch from yajl to Jansson" > Revert "build: undef WITH_JANSSON for SETUID_RPC_CLIENT" > Revert "build: add --with-jansson" And this patch was the only one that did not revert cleanly. Great, on the basis that these are all clean reverts, and the last looks sane: Reviewed-by: Daniel P. Berrangé Thanks, pushed now. Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/16] Revert Jansson usage
On Mon, Aug 13, 2018 at 02:54:00PM +0200, Ján Tomko wrote: > On Mon, Aug 13, 2018 at 01:55:10PM +0200, Ján Tomko wrote: > > For QEMU, we need a JSON parser that is able to handle its non-compliant > > JSON usage: > > https://bugzilla.redhat.com/show_bug.cgi?id=1614569 > > > > Unfortunately, this does not seem to be possible with Jansson. > > > > Revert back to using yajl, which also lets us get rid of the 'dlopen' > > hacks and their bugfixes. > > > > (The QEMU default changes are not strictly required, but the patches > > specifically use JANSSON) > > > > Also available on: > > git://repo.or.cz/libvirt/jtomko.git revert > > > > Ján Tomko (16): > > Revert "src: Move DLOPEN_LIBS to libraries introducing the dependency" > > Note that this patch broke the build with clang by adding DLOPEN_LIBS to > libvirt_setuid_rpc_client_la_CFLAGS instead of LDFLAGS: > https://www.redhat.com/archives/libvir-list/2018-August/msg00604.html > So technically this series is a build breaker fix ;) > > > Revert "Fix link errors in tools/nss and tests" > > Revert "remote: daemon: Make sure that JSON symbols are properly > >loaded at startup" > > Revert "util: jsoncompat: Stub out virJSONInitialize when compiling > >without jansson" > > Revert "tests: qemucapsprobe: Fix output after switching to jansson" > > Revert "util: avoid symbol clash between json libraries" > > Revert "tests: also skip qemuagenttest with old jansson" > > Revert "m4: Introduce STABLE_ORDERING_JANSSON" > > Revert "build: require Jansson if QEMU driver is enabled" > > Revert "build: switch --with-qemu default from yes to check" > > Revert "Remove virJSONValueNewStringLen" > > Revert "build: remove references to WITH_YAJL for SETUID_RPC_CLIENT" > > Revert "Remove functions using yajl" > > Revert "Switch from yajl to Jansson" > > Revert "build: undef WITH_JANSSON for SETUID_RPC_CLIENT" > > > Revert "build: add --with-jansson" > > And this patch was the only one that did not revert cleanly. Great, on the basis that these are all clean reverts, and the last looks sane: Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/16] Revert Jansson usage
On Mon, Aug 13, 2018 at 01:55:10PM +0200, Ján Tomko wrote: For QEMU, we need a JSON parser that is able to handle its non-compliant JSON usage: https://bugzilla.redhat.com/show_bug.cgi?id=1614569 Unfortunately, this does not seem to be possible with Jansson. Revert back to using yajl, which also lets us get rid of the 'dlopen' hacks and their bugfixes. (The QEMU default changes are not strictly required, but the patches specifically use JANSSON) Also available on: git://repo.or.cz/libvirt/jtomko.git revert Ján Tomko (16): Revert "src: Move DLOPEN_LIBS to libraries introducing the dependency" Note that this patch broke the build with clang by adding DLOPEN_LIBS to libvirt_setuid_rpc_client_la_CFLAGS instead of LDFLAGS: https://www.redhat.com/archives/libvir-list/2018-August/msg00604.html So technically this series is a build breaker fix ;) Revert "Fix link errors in tools/nss and tests" Revert "remote: daemon: Make sure that JSON symbols are properly loaded at startup" Revert "util: jsoncompat: Stub out virJSONInitialize when compiling without jansson" Revert "tests: qemucapsprobe: Fix output after switching to jansson" Revert "util: avoid symbol clash between json libraries" Revert "tests: also skip qemuagenttest with old jansson" Revert "m4: Introduce STABLE_ORDERING_JANSSON" Revert "build: require Jansson if QEMU driver is enabled" Revert "build: switch --with-qemu default from yes to check" Revert "Remove virJSONValueNewStringLen" Revert "build: remove references to WITH_YAJL for SETUID_RPC_CLIENT" Revert "Remove functions using yajl" Revert "Switch from yajl to Jansson" Revert "build: undef WITH_JANSSON for SETUID_RPC_CLIENT" Revert "build: add --with-jansson" And this patch was the only one that did not revert cleanly. Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/16] Revert Jansson usage
For QEMU, we need a JSON parser that is able to handle its non-compliant JSON usage: https://bugzilla.redhat.com/show_bug.cgi?id=1614569 Unfortunately, this does not seem to be possible with Jansson. Revert back to using yajl, which also lets us get rid of the 'dlopen' hacks and their bugfixes. (The QEMU default changes are not strictly required, but the patches specifically use JANSSON) Also available on: git://repo.or.cz/libvirt/jtomko.git revert Ján Tomko (16): Revert "src: Move DLOPEN_LIBS to libraries introducing the dependency" Revert "Fix link errors in tools/nss and tests" Revert "remote: daemon: Make sure that JSON symbols are properly loaded at startup" Revert "util: jsoncompat: Stub out virJSONInitialize when compiling without jansson" Revert "tests: qemucapsprobe: Fix output after switching to jansson" Revert "util: avoid symbol clash between json libraries" Revert "tests: also skip qemuagenttest with old jansson" Revert "m4: Introduce STABLE_ORDERING_JANSSON" Revert "build: require Jansson if QEMU driver is enabled" Revert "build: switch --with-qemu default from yes to check" Revert "Remove virJSONValueNewStringLen" Revert "build: remove references to WITH_YAJL for SETUID_RPC_CLIENT" Revert "Remove functions using yajl" Revert "Switch from yajl to Jansson" Revert "build: undef WITH_JANSSON for SETUID_RPC_CLIENT" Revert "build: add --with-jansson" config-post.h| 3 +- configure.ac | 3 - libvirt.spec.in | 6 +- m4/virt-driver-qemu.m4 | 9 +- m4/virt-jansson.m4 | 32 -- m4/virt-nss.m4 | 4 +- m4/virt-yajl.m4 | 27 +- src/Makefile.am | 14 +- src/libvirt_private.syms | 5 +- src/qemu/qemu_driver.c | 2 +- src/remote/remote_daemon.c | 4 - src/util/Makefile.inc.am | 6 +- src/util/virjson.c | 617 --- src/util/virjson.h | 1 + src/util/virjsoncompat.c | 285 -- src/util/virjsoncompat.h | 88 - tests/Makefile.am| 12 +- tests/cputest.c | 16 +- tests/libxlxml2domconfigtest.c | 4 +- tests/qemuagenttest.c| 4 +- tests/qemublocktest.c| 6 - tests/qemucapabilitiestest.c | 7 +- tests/qemucaps2xmltest.c | 2 +- tests/qemucapsprobemock.c| 2 - tests/qemucommandutiltest.c | 7 +- tests/qemuhotplugtest.c | 7 +- tests/qemumigparamsdata/empty.json | 4 +- tests/qemumigparamsdata/unsupported.json | 4 +- tests/qemumigparamstest.c| 7 +- tests/qemumonitorjsontest.c | 7 +- tests/virjsontest.c | 5 - tests/virmacmaptest.c| 5 - tests/virmacmaptestdata/empty.json | 4 +- tests/virmocklibxl.c | 4 +- tests/virnetdaemontest.c | 7 +- tests/virstoragetest.c | 4 +- tools/Makefile.am| 6 +- 37 files changed, 557 insertions(+), 673 deletions(-) delete mode 100644 m4/virt-jansson.m4 delete mode 100644 src/util/virjsoncompat.c delete mode 100644 src/util/virjsoncompat.h -- 2.16.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list