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

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

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

>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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é <berrange@redhat.com>


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

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é <berrange@redhat.com>
>

Thanks, pushed now.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list