Per the discussion here:
https://www.redhat.com/archives/libvir-list/2017-November/msg00225.html
Switch from using yajl to Jansson.

v1:
https://www.redhat.com/archives/libvir-list/2018-March/msg01781.html
v2:
https://www.redhat.com/archives/libvir-list/2018-May/msg00695.html

Patches 3-9 are meant to be squashed together before pushing, but are
posted separately here in hope it will make reviewing them easier.

v3: separated the removal of yajl functions, also removed WITH_YAJL from
config-post.h and split the virt-driver-qemu.m4 changes into two
patches, now with AC_REQUIRE for the CHECK_JANSSON dependency

Ján Tomko (13):
  build: add --with-jansson
  build: undef WITH_JANSSON for SETUID_RPC_CLIENT
  Switch from yajl to Jansson
  FIXUP: fix tests
  FIXUP: Deprecate building --with-yajl
  FIXUP: make nss depend on Jansson instead of yajl
  FIXUP: compile and link with Jansson instead of yajl
  FIXUP: s/WITH_YAJL/WITH_JANSSON/
  FIXUP: libvirt.spec: use jansson instead of yajl
  Remove functions using yajl
  build: remove references to WITH_YAJL for SETUID_RPC_CLIENT
  build: switch --with-qemu default from yes to check
  build: require Jansson if QEMU driver is enabled

 config-post.h                            |   3 +-
 configure.ac                             |   3 +
 libvirt.spec.in                          |   4 +-
 m4/virt-driver-qemu.m4                   |   9 +-
 m4/virt-jansson.m4                       |  29 ++
 m4/virt-nss.m4                           |   4 +-
 m4/virt-yajl.m4                          |  27 +-
 src/Makefile.am                          |   8 +-
 src/qemu/qemu_driver.c                   |   2 +-
 src/util/Makefile.inc.am                 |   4 +-
 src/util/virjson.c                       | 597 ++++++++-----------------------
 tests/Makefile.am                        |  12 +-
 tests/cputest.c                          |  16 +-
 tests/libxlxml2domconfigtest.c           |   4 +-
 tests/qemuagenttest.c                    |   2 +-
 tests/qemublocktest.c                    |   1 +
 tests/qemucapabilitiestest.c             |   2 +-
 tests/qemucaps2xmltest.c                 |   2 +-
 tests/qemucommandutiltest.c              |   2 +-
 tests/qemuhotplugtest.c                  |   2 +-
 tests/qemumigparamsdata/empty.json       |   4 +-
 tests/qemumigparamsdata/unsupported.json |   4 +-
 tests/qemumigparamstest.c                |   2 +-
 tests/qemumonitorjsontest.c              |   2 +-
 tests/virmacmaptestdata/empty.json       |   4 +-
 tests/virmocklibxl.c                     |   4 +-
 tests/virnetdaemontest.c                 |   2 +-
 tests/virstoragetest.c                   |   4 +-
 28 files changed, 227 insertions(+), 532 deletions(-)
 create mode 100644 m4/virt-jansson.m4

-- 
2.16.1

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

On Fri, 2018-05-11 at 14:59 +0200, Ján Tomko wrote:
> Per the discussion here:
> https://www.redhat.com/archives/libvir-list/2017-November/msg00225.html
> Switch from using yajl to Jansson.

I expect Peter will review this, like he did for v1 and v2; however,
I wanted to point out that, while your series requires jansson 2.7,
Ubuntu 14.04 only ships jansson 2.5.

While Ubuntu 14.04 is *not* a supported platform as per our recently
formalized support policy, it's also the only Linux platform
available on Travis CI, so dropping support for it would mean losing
the ability to perform Travis CI builds, at least with the QEMU
driver enabled.

Personally, I think Travis CI being limited to Ubuntu, and stuck to
an obsolete version at that, makes it close to useless for Linux
builds, but I know other developers (CC'd one of them ;) use it for
smoke testing before posting patches and would probably be unhappy
if that was no longer possible.

It should however be noted that the libvirt-jenkins-ci project
provides pretty much everything you need to create and manage a
local build farm which covers our supported platforms much better;
the only thing missing there is the convenience of Travis CI, where
you only need to push a branch to GitHub and wait for the results,
but I'm actually working on something that will improve the status
quo significantly, so once that's in place maybe it will be okay
for us to use Travis CI for macOS builds exclusively.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

On Tue, May 15, 2018 at 10:23:52AM +0200, Andrea Bolognani wrote:
>On Fri, 2018-05-11 at 14:59 +0200, Ján Tomko wrote:
>> Per the discussion here:
>> https://www.redhat.com/archives/libvir-list/2017-November/msg00225.html
>> Switch from using yajl to Jansson.
>
>I expect Peter will review this, like he did for v1 and v2; however,
>I wanted to point out that, while your series requires jansson 2.7,
>Ubuntu 14.04 only ships jansson 2.5.
>

Given that the only 2.7 features my series currently uses are:
json_stringn (which allows parsing strings with NULL-bytes, not used
anyway) and the json_boolean_value alias for json_is_true,
downgrading the version to 2.5 is not a problem in this case.

>While Ubuntu 14.04 is *not* a supported platform as per our recently
>formalized support policy, it's also the only Linux platform
>available on Travis CI, so dropping support for it would mean losing
>the ability to perform Travis CI builds, at least with the QEMU
>driver enabled.
>

But if this effectively adds another platform to our formalized support
policy, I will be unhappy when this prevents us from dropping some
outdated code.

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

On Tue, May 15, 2018 at 10:23:52AM +0200, Andrea Bolognani wrote:
> On Fri, 2018-05-11 at 14:59 +0200, Ján Tomko wrote:
> > Per the discussion here:
> > https://www.redhat.com/archives/libvir-list/2017-November/msg00225.html
> > Switch from using yajl to Jansson.
> 
> I expect Peter will review this, like he did for v1 and v2; however,
> I wanted to point out that, while your series requires jansson 2.7,
> Ubuntu 14.04 only ships jansson 2.5.
> 
> While Ubuntu 14.04 is *not* a supported platform as per our recently
> formalized support policy, it's also the only Linux platform
> available on Travis CI, so dropping support for it would mean losing
> the ability to perform Travis CI builds, at least with the QEMU
> driver enabled.
> 
> Personally, I think Travis CI being limited to Ubuntu, and stuck to
> an obsolete version at that, makes it close to useless for Linux
> builds, but I know other developers (CC'd one of them ;) use it for
> smoke testing before posting patches and would probably be unhappy
> if that was no longer possible.

Indeed, and from what I see they are testing availability of "precise"
distro, to replace it. So I'd rather assume that we will be able to switch
it to more recent distro in the near future, and not aggressively rip it
out now.

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 Tue, 2018-05-15 at 10:19 +0100, Daniel P. Berrangé wrote:
> On Tue, May 15, 2018 at 10:23:52AM +0200, Andrea Bolognani wrote:
> > Personally, I think Travis CI being limited to Ubuntu, and stuck to
> > an obsolete version at that, makes it close to useless for Linux
> > builds, but I know other developers (CC'd one of them ;) use it for
> > smoke testing before posting patches and would probably be unhappy
> > if that was no longer possible.
> 
> Indeed, and from what I see they are testing availability of "precise"
> distro, to replace it. So I'd rather assume that we will be able to switch
> it to more recent distro in the near future, and not aggressively rip it
> out now.

"precise" is the codename for Ubuntu 12.04 LTS, which has been
dropped from Travis CI a few months back.

"xenial" (Ubuntu 16.04 LTS) support has been in the works for a
long time: it was "just around the corner" last December[1], but
it's six months (and one more Ubuntu LTS release) later now and
it still hasn't materialized; that, along with the fact that just
a couple of months ago[2] the folks at Travis were "super excited"
to introduce "trusty" (Ubuntu 14.04 LTS) to their Enterprise
offering makes it very hard for me to believe we will be able to
run builds on 16.04 anytime soon.


[1] https://blog.travis-ci.com/2017-12-01-new-update-schedule-for-linux-build-images
[2] https://blog.travis-ci.com/2018-03-01-new-enterprise-build-environments-trusty
-- 
Andrea Bolognani / Red Hat / Virtualization

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

On Tue, May 15, 2018 at 11:46:57AM +0200, Andrea Bolognani wrote:
> On Tue, 2018-05-15 at 10:19 +0100, Daniel P. Berrangé wrote:
> > On Tue, May 15, 2018 at 10:23:52AM +0200, Andrea Bolognani wrote:
> > > Personally, I think Travis CI being limited to Ubuntu, and stuck to
> > > an obsolete version at that, makes it close to useless for Linux
> > > builds, but I know other developers (CC'd one of them ;) use it for
> > > smoke testing before posting patches and would probably be unhappy
> > > if that was no longer possible.
> > 
> > Indeed, and from what I see they are testing availability of "precise"
> > distro, to replace it. So I'd rather assume that we will be able to switch
> > it to more recent distro in the near future, and not aggressively rip it
> > out now.
> 
> "precise" is the codename for Ubuntu 12.04 LTS, which has been
> dropped from Travis CI a few months back.

Sigh, I of course meant xenial.

> "xenial" (Ubuntu 16.04 LTS) support has been in the works for a
> long time: it was "just around the corner" last December[1], but
> it's six months (and one more Ubuntu LTS release) later now and
> it still hasn't materialized; that, along with the fact that just
> a couple of months ago[2] the folks at Travis were "super excited"
> to introduce "trusty" (Ubuntu 14.04 LTS) to their Enterprise
> offering makes it very hard for me to believe we will be able to
> run builds on 16.04 anytime soon.

I just had a chat with spice folks and learnt that gitlab.com provides
a CI system with free shared runners. The key difference from travis,
is that gitlab supports a choice of docker images to use, but does not
support os-x.

So we could drop ubuntu from travis, leaving just the os-x builder,
and enable use of some docker images under gitlab, to get an equiv
level of coverage.

In fact I wonder if we can reuse the ansible scripts we have to build
suitable docker images ?

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 Tue, 2018-05-15 at 12:08 +0100, Daniel P. Berrangé wrote:
> On Tue, May 15, 2018 at 11:46:57AM +0200, Andrea Bolognani wrote:
> > "xenial" (Ubuntu 16.04 LTS) support has been in the works for a
> > long time: it was "just around the corner" last December[1], but
> > it's six months (and one more Ubuntu LTS release) later now and
> > it still hasn't materialized; that, along with the fact that just
> > a couple of months ago[2] the folks at Travis were "super excited"
> > to introduce "trusty" (Ubuntu 14.04 LTS) to their Enterprise
> > offering makes it very hard for me to believe we will be able to
> > run builds on 16.04 anytime soon.
> 
> I just had a chat with spice folks and learnt that gitlab.com provides
> a CI system with free shared runners. The key difference from travis,
> is that gitlab supports a choice of docker images to use, but does not
> support os-x.
> 
> So we could drop ubuntu from travis, leaving just the os-x builder,
> and enable use of some docker images under gitlab, to get an equiv
> level of coverage.

Why don't we just add Ubuntu 16.04 and Ubuntu 18.04 workers to the
CentOS CI environment? I'd rather avoid fracturing our CI efforts
further.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

On Tue, May 15, 2018 at 02:03:12PM +0200, Andrea Bolognani wrote:
> On Tue, 2018-05-15 at 12:08 +0100, Daniel P. Berrangé wrote:
> > On Tue, May 15, 2018 at 11:46:57AM +0200, Andrea Bolognani wrote:
> > > "xenial" (Ubuntu 16.04 LTS) support has been in the works for a
> > > long time: it was "just around the corner" last December[1], but
> > > it's six months (and one more Ubuntu LTS release) later now and
> > > it still hasn't materialized; that, along with the fact that just
> > > a couple of months ago[2] the folks at Travis were "super excited"
> > > to introduce "trusty" (Ubuntu 14.04 LTS) to their Enterprise
> > > offering makes it very hard for me to believe we will be able to
> > > run builds on 16.04 anytime soon.
> > 
> > I just had a chat with spice folks and learnt that gitlab.com provides
> > a CI system with free shared runners. The key difference from travis,
> > is that gitlab supports a choice of docker images to use, but does not
> > support os-x.
> > 
> > So we could drop ubuntu from travis, leaving just the os-x builder,
> > and enable use of some docker images under gitlab, to get an equiv
> > level of coverage.
> 
> Why don't we just add Ubuntu 16.04 and Ubuntu 18.04 workers to the
> CentOS CI environment? I'd rather avoid fracturing our CI efforts
> further.

The CentOS CI runs post-merge, while the travis CI can run pre-merge
on developer's branches. It is also pushing capacity of the CentOS
CI host to add another 2 VMs to it. 

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 Tue, 2018-05-15 at 13:09 +0100, Daniel P. Berrangé wrote:
> On Tue, May 15, 2018 at 02:03:12PM +0200, Andrea Bolognani wrote:
> > Why don't we just add Ubuntu 16.04 and Ubuntu 18.04 workers to the
> > CentOS CI environment? I'd rather avoid fracturing our CI efforts
> > further.
> 
> The CentOS CI runs post-merge, while the travis CI can run pre-merge
> on developer's branches.

I don't think post-merge vs pre-merge is a big deal, as long as
we keep an eye on CentOS CI and react quickly to breakages, which
IMHO we're doing reasonably well already.

In fact, I believe most libvirt developers don't use Travis CI at
all and just fix stuff once CentOS CI highlights it as broken,
which is perfectly fine in my book.

Additionally, as I mentioned in a previous message, having a local
build farm is not too difficult and almost entirely automated these
days, so for people looking into smoke-testing their changes that's
an overall better route, as it gives full coverage instead of the
reduced coverage a Travis CI build provides.

Another disadvantage of using a third-party CI provider is that
neither Travis CI not GitLab CI allow us to do the kind of
integration testing we have going on with CentOS CI, where each
change in libvirt triggers builds of bindings and other projects
using it.

One additional advantage of CentOS CI is that we can have non-Linux
and, potentially, non-x86 build guests as well: neither Travis CI
nor GitLab CI offer the latter AFAIK, and the former is limited to
macOS only on Travis CI.

> It is also pushing capacity of the CentOS
> CI host to add another 2 VMs to it.

I don't think that would tip the scales too much. Builds might
take a bit longer, but they are far from being instantaneous
already, so in practice a few extra minutes won't change a thing.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

On Fri, 2018-05-11 at 14:59 +0200, Ján Tomko wrote:
> Per the discussion here:
> https://www.redhat.com/archives/libvir-list/2017-November/msg00225.html
> Switch from using yajl to Jansson.

I tried building

  https://repo.or.cz/libvirt/jtomko.git/ jansson

on all platform libvirt supports.

Debian 8, Ubuntu 14.04 and Ubuntu 16.04 all linked successfully
against Jansson, but when running the test suite I got

  FAIL: virnetdaemontest
  FAIL: qemumonitorjsontest
  FAIL: qemucapabilitiestest
  FAIL: qemuhotplugtest
  FAIL: qemucommandutiltest
  FAIL: qemublocktest
  FAIL: qemumigparamstest
  FAIL: virjsontest

>From a quick look, it seems like at least some of the failures
are caused by dictionaries and arrays having their members
shuffled around.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

On Tue, May 15, 2018 at 06:12:05PM +0200, Andrea Bolognani wrote:
> On Fri, 2018-05-11 at 14:59 +0200, Ján Tomko wrote:
> > Per the discussion here:
> > https://www.redhat.com/archives/libvir-list/2017-November/msg00225.html
> > Switch from using yajl to Jansson.
> 
> I tried building
> 
>   https://repo.or.cz/libvirt/jtomko.git/ jansson
> 
> on all platform libvirt supports.
> 
> Debian 8, Ubuntu 14.04 and Ubuntu 16.04 all linked successfully
> against Jansson, but when running the test suite I got
> 
>   FAIL: virnetdaemontest
>   FAIL: qemumonitorjsontest
>   FAIL: qemucapabilitiestest
>   FAIL: qemuhotplugtest
>   FAIL: qemucommandutiltest
>   FAIL: qemublocktest
>   FAIL: qemumigparamstest
>   FAIL: virjsontest
> 
> >From a quick look, it seems like at least some of the failures
> are caused by dictionaries and arrays having their members
> shuffled around.

When using YAJL, object properties are stored in arrays, so order is
stable when printing.

With Jansson, object properties appear to be stored in hash tables,
so order is potentially unstable when printing.

I'm a little surprised it doesn't fail on all platforms - makes me
wonder how good the hash function is.

Looks like we would need JSON_SORT_KEYS to json_dumps(), but that 
may well require all test data to be recreated :-(

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 Tue, May 15, 2018 at 05:24:40PM +0100, Daniel P. Berrangé wrote:
>On Tue, May 15, 2018 at 06:12:05PM +0200, Andrea Bolognani wrote:
>> On Fri, 2018-05-11 at 14:59 +0200, Ján Tomko wrote:
>> > Per the discussion here:
>> > https://www.redhat.com/archives/libvir-list/2017-November/msg00225.html
>> > Switch from using yajl to Jansson.
>>
>> I tried building
>>
>>   https://repo.or.cz/libvirt/jtomko.git/ jansson
>>
>> on all platform libvirt supports.
>>
>> Debian 8, Ubuntu 14.04 and Ubuntu 16.04 all linked successfully
>> against Jansson, but when running the test suite I got
>>
>>   FAIL: virnetdaemontest
>>   FAIL: qemumonitorjsontest
>>   FAIL: qemucapabilitiestest
>>   FAIL: qemuhotplugtest
>>   FAIL: qemucommandutiltest
>>   FAIL: qemublocktest
>>   FAIL: qemumigparamstest
>>   FAIL: virjsontest
>>
>> >From a quick look, it seems like at least some of the failures
>> are caused by dictionaries and arrays having their members
>> shuffled around.
>
>When using YAJL, object properties are stored in arrays, so order is
>stable when printing.
>
>With Jansson, object properties appear to be stored in hash tables,
>so order is potentially unstable when printing.
>
>I'm a little surprised it doesn't fail on all platforms - makes me
>wonder how good the hash function is.
>
>Looks like we would need JSON_SORT_KEYS to json_dumps(), but that
>may well require all test data to be recreated :-(
>

There is a JSON_PRESERVE_ORDER flag since v1.3 that formats the keys
in the same order they were added and it's the default since 2.8.

I have pushed a patch that uses it on my repo:
https://repo.or.cz/libvirt/jtomko.git/ jansson_preserve

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

On Wed, May 16, 2018 at 10:53:22AM +0200, Ján Tomko wrote:
> On Tue, May 15, 2018 at 05:24:40PM +0100, Daniel P. Berrangé wrote:
> > On Tue, May 15, 2018 at 06:12:05PM +0200, Andrea Bolognani wrote:
> > > On Fri, 2018-05-11 at 14:59 +0200, Ján Tomko wrote:
> > > > Per the discussion here:
> > > > https://www.redhat.com/archives/libvir-list/2017-November/msg00225.html
> > > > Switch from using yajl to Jansson.
> > > 
> > > I tried building
> > > 
> > >   https://repo.or.cz/libvirt/jtomko.git/ jansson
> > > 
> > > on all platform libvirt supports.
> > > 
> > > Debian 8, Ubuntu 14.04 and Ubuntu 16.04 all linked successfully
> > > against Jansson, but when running the test suite I got
> > > 
> > >   FAIL: virnetdaemontest
> > >   FAIL: qemumonitorjsontest
> > >   FAIL: qemucapabilitiestest
> > >   FAIL: qemuhotplugtest
> > >   FAIL: qemucommandutiltest
> > >   FAIL: qemublocktest
> > >   FAIL: qemumigparamstest
> > >   FAIL: virjsontest
> > > 
> > > >From a quick look, it seems like at least some of the failures
> > > are caused by dictionaries and arrays having their members
> > > shuffled around.
> > 
> > When using YAJL, object properties are stored in arrays, so order is
> > stable when printing.
> > 
> > With Jansson, object properties appear to be stored in hash tables,
> > so order is potentially unstable when printing.
> > 
> > I'm a little surprised it doesn't fail on all platforms - makes me
> > wonder how good the hash function is.
> > 
> > Looks like we would need JSON_SORT_KEYS to json_dumps(), but that
> > may well require all test data to be recreated :-(
> > 
> 
> There is a JSON_PRESERVE_ORDER flag since v1.3 that formats the keys
> in the same order they were added and it's the default since 2.8.

Ah excellant - that'll explain why only some distros failed - presumably
they have version < 2.8

> I have pushed a patch that uses it on my repo:
> https://repo.or.cz/libvirt/jtomko.git/ jansson_preserve


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