Changeset
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
Git apply log
Switched to a new branch 'cover.1526043327.git.jtomko@redhat.com'
Applying: build: add --with-jansson
Applying: build: undef WITH_JANSSON for SETUID_RPC_CLIENT
Applying: Switch from yajl to Jansson
Applying: FIXUP: fix tests
Applying: FIXUP: Deprecate building --with-yajl
Applying: FIXUP: make nss depend on Jansson instead of yajl
Applying: FIXUP: compile and link with Jansson instead of yajl
Applying: FIXUP: s/WITH_YAJL/WITH_JANSSON/
Applying: FIXUP: libvirt.spec: use jansson instead of yajl
Applying: Remove functions using yajl
Applying: build: remove references to WITH_YAJL for SETUID_RPC_CLIENT
Applying: build: switch --with-qemu default from yes to check
Applying: build: require Jansson if QEMU driver is enabled
To https://github.com/patchew-project/libvirt
 * [new tag]         patchew/cover.1526043327.git.jtomko@redhat.com -> patchew/cover.1526043327.git.jtomko@redhat.com
Test passed: syntax-check

loading

[libvirt] [PATCHv3 00/13] Switch from yajl to Jansson
Posted by Ján Tomko, 1 week ago
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
Re: [libvirt] [PATCHv3 00/13] Switch from yajl to Jansson
Posted by Andrea Bolognani, 1 week ago
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
Re: [libvirt] [PATCHv3 00/13] Switch from yajl to Jansson
Posted by Ján Tomko, 1 week ago
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
Re: [libvirt] [PATCHv3 00/13] Switch from yajl to Jansson
Posted by Daniel P. Berrangé, 1 week ago
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
Re: [libvirt] [PATCHv3 00/13] Switch from yajl to Jansson
Posted by Andrea Bolognani, 1 week ago
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
Re: [libvirt] [PATCHv3 00/13] Switch from yajl to Jansson
Posted by Daniel P. Berrangé, 1 week ago
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
Re: [libvirt] [PATCHv3 00/13] Switch from yajl to Jansson
Posted by Andrea Bolognani, 1 week ago
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
Re: [libvirt] [PATCHv3 00/13] Switch from yajl to Jansson
Posted by Daniel P. Berrangé, 1 week ago
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
Re: [libvirt] [PATCHv3 00/13] Switch from yajl to Jansson
Posted by Andrea Bolognani, 1 week ago
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
Re: [libvirt] [PATCHv3 00/13] Switch from yajl to Jansson
Posted by Andrea Bolognani, 1 week ago
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
Re: [libvirt] [PATCHv3 00/13] Switch from yajl to Jansson
Posted by Daniel P. Berrangé, 1 week ago
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
Re: [libvirt] [PATCHv3 00/13] Switch from yajl to Jansson
Posted by Ján Tomko, 6 days ago
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
Re: [libvirt] [PATCHv3 00/13] Switch from yajl to Jansson
Posted by Daniel P. Berrangé, 6 days ago
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
[libvirt] [PATCHv3 01/13] build: add --with-jansson
Posted by Ján Tomko, 1 week ago
Introduce the configure argument and check for Jansson >= 2.7

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 configure.ac       |  3 +++
 m4/virt-jansson.m4 | 29 +++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)
 create mode 100644 m4/virt-jansson.m4

diff --git a/configure.ac b/configure.ac
index cc005ea9e8..c51e239b07 100644
--- a/configure.ac
+++ b/configure.ac
@@ -252,6 +252,7 @@ LIBVIRT_ARG_FUSE
 LIBVIRT_ARG_GLUSTER
 LIBVIRT_ARG_GNUTLS
 LIBVIRT_ARG_HAL
+LIBVIRT_ARG_JANSSON
 LIBVIRT_ARG_LIBPCAP
 LIBVIRT_ARG_LIBSSH
 LIBVIRT_ARG_LIBXML
@@ -292,6 +293,7 @@ LIBVIRT_CHECK_FUSE
 LIBVIRT_CHECK_GLUSTER
 LIBVIRT_CHECK_GNUTLS
 LIBVIRT_CHECK_HAL
+LIBVIRT_CHECK_JANSSON
 LIBVIRT_CHECK_LIBNL
 LIBVIRT_CHECK_LIBPARTED
 LIBVIRT_CHECK_LIBPCAP
@@ -957,6 +959,7 @@ LIBVIRT_RESULT_FUSE
 LIBVIRT_RESULT_GLUSTER
 LIBVIRT_RESULT_GNUTLS
 LIBVIRT_RESULT_HAL
+LIBVIRT_RESULT_JANSSON
 LIBVIRT_RESULT_LIBNL
 LIBVIRT_RESULT_LIBPCAP
 LIBVIRT_RESULT_LIBSSH
diff --git a/m4/virt-jansson.m4 b/m4/virt-jansson.m4
new file mode 100644
index 0000000000..1f1d0a66e6
--- /dev/null
+++ b/m4/virt-jansson.m4
@@ -0,0 +1,29 @@
+dnl The jansson library
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library.  If not, see
+dnl <http://www.gnu.org/licenses/>.
+dnl
+
+AC_DEFUN([LIBVIRT_ARG_JANSSON],[
+  LIBVIRT_ARG_WITH_FEATURE([JANSSON], [jansson], [check])
+])
+
+AC_DEFUN([LIBVIRT_CHECK_JANSSON],[
+  dnl Jansson http://www.digip.org/jansson/
+  LIBVIRT_CHECK_PKG([JANSSON], [jansson], [2.7])
+])
+
+AC_DEFUN([LIBVIRT_RESULT_JANSSON],[
+  LIBVIRT_RESULT_LIB([JANSSON])
+])
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 02/13] build: undef WITH_JANSSON for SETUID_RPC_CLIENT
Posted by Ján Tomko, 1 week ago
There is no code using WITH_JANSSON yet, but once we add it,
it should not be compiled for the setuid_rpc_client.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 config-post.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/config-post.h b/config-post.h
index f7eba0d7ca..cb2fc2b4ee 100644
--- a/config-post.h
+++ b/config-post.h
@@ -37,6 +37,7 @@
 # undef WITH_DTRACE_PROBES
 # undef WITH_GNUTLS
 # undef WITH_GNUTLS_GCRYPT
+# undef WITH_JANSSON
 # undef WITH_LIBSSH
 # undef WITH_MACVTAP
 # undef WITH_NUMACTL
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 03/13] Switch from yajl to Jansson
Posted by Ján Tomko, 1 week ago
Yajl has not seen much activity upstream recently.
Switch to using Jansson >= 2.7.

All the platforms we target on https://libvirt.org/platforms.html
have a version >= 2.7 listed on the sites below:
https://repology.org/metapackage/jansson/versions
https://build.opensuse.org/package/show/devel:libraries:c_c++/libjansson

Implement virJSONValue{From,To}String using Jansson, delete the yajl
code (and the related virJSONParser structure) and report an error
if someone explicitly specifies --with-yajl.

Also adjust the test data to account for Jansson's different whitespace
usage for empty arrays and tune up the specfile to keep 'make rpm'
working when bisecting.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 src/util/virjson.c | 211 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 211 insertions(+)

diff --git a/src/util/virjson.c b/src/util/virjson.c
index 0559d40b64..2f7d624bb3 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -2008,6 +2008,217 @@ virJSONValueToString(virJSONValuePtr object,
 }
 
 
+#elif WITH_JANSSON
+# include <jansson.h>
+
+static virJSONValuePtr
+virJSONValueFromJansson(json_t *json)
+{
+    virJSONValuePtr ret = NULL;
+    const char *key;
+    json_t *cur;
+    size_t i;
+
+    switch (json_typeof(json)) {
+    case JSON_OBJECT:
+        ret = virJSONValueNewObject();
+        if (!ret)
+            goto error;
+
+        json_object_foreach(json, key, cur) {
+            virJSONValuePtr val = virJSONValueFromJansson(cur);
+            if (!val)
+                goto error;
+
+            if (virJSONValueObjectAppend(ret, key, val) < 0) {
+                virJSONValueFree(val);
+                goto error;
+            }
+        }
+
+        break;
+
+    case JSON_ARRAY:
+        ret = virJSONValueNewArray();
+        if (!ret)
+            goto error;
+
+        json_array_foreach(json, i, cur) {
+            virJSONValuePtr val = virJSONValueFromJansson(cur);
+            if (!val)
+                goto error;
+
+            if (virJSONValueArrayAppend(ret, val) < 0) {
+                virJSONValueFree(val);
+                goto error;
+            }
+        }
+        break;
+
+    case JSON_STRING:
+        ret = virJSONValueNewStringLen(json_string_value(json),
+                                       json_string_length(json));
+        break;
+
+    case JSON_INTEGER:
+        ret = virJSONValueNewNumberLong(json_integer_value(json));
+        break;
+
+    case JSON_REAL:
+        ret = virJSONValueNewNumberDouble(json_real_value(json));
+        break;
+
+    case JSON_TRUE:
+    case JSON_FALSE:
+        ret = virJSONValueNewBoolean(json_boolean_value(json));
+        break;
+
+    case JSON_NULL:
+        ret = virJSONValueNewNull();
+        break;
+    }
+
+    return ret;
+
+ error:
+    virJSONValueFree(ret);
+    return NULL;
+}
+
+virJSONValuePtr
+virJSONValueFromString(const char *jsonstring)
+{
+    virJSONValuePtr ret = NULL;
+    json_t *json;
+    json_error_t error;
+    size_t flags = JSON_REJECT_DUPLICATES |
+                   JSON_DECODE_ANY;
+
+    if (!(json = json_loads(jsonstring, flags, &error))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("failed to parse JSON %d:%d: %s"),
+                       error.line, error.column, error.text);
+        return NULL;
+    }
+
+    ret = virJSONValueFromJansson(json);
+    json_decref(json);
+    return ret;
+}
+
+
+static json_t *
+virJSONValueToJansson(virJSONValuePtr object)
+{
+    json_t *ret = NULL;
+    size_t i;
+
+    switch ((virJSONType)object->type) {
+    case VIR_JSON_TYPE_OBJECT:
+        ret = json_object();
+        if (!ret) {
+            virReportOOMError();
+            goto error;
+        }
+        for (i = 0; i < object->data.object.npairs; i++) {
+            virJSONObjectPairPtr cur = object->data.object.pairs + i;
+            json_t *val = virJSONValueToJansson(cur->value);
+
+            if (!val)
+                goto error;
+            if (json_object_set_new(ret, cur->key, val) < 0) {
+                virReportOOMError();
+                goto error;
+            }
+        }
+        break;
+
+    case VIR_JSON_TYPE_ARRAY:
+        ret = json_array();
+        if (!ret) {
+            virReportOOMError();
+            goto error;
+        }
+        for (i = 0; i < object->data.array.nvalues; i++) {
+            virJSONValuePtr cur = object->data.array.values[i];
+            json_t *val = virJSONValueToJansson(cur);
+
+            if (!val)
+                goto error;
+            if (json_array_append_new(ret, val) < 0) {
+                virReportOOMError();
+                goto error;
+            }
+        }
+        break;
+
+    case VIR_JSON_TYPE_STRING:
+        ret = json_string(object->data.string);
+        break;
+
+    case VIR_JSON_TYPE_NUMBER: {
+        long long ll_val;
+        double d_val;
+        if (virStrToLong_ll(object->data.number, NULL, 10, &ll_val) < 0) {
+            if (virStrToDouble(object->data.number, NULL, &d_val) < 0) {
+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                               _("JSON value is not a number"));
+                return NULL;
+            }
+            ret = json_real(d_val);
+        } else {
+            ret = json_integer(ll_val);
+        }
+    }
+        break;
+
+    case VIR_JSON_TYPE_BOOLEAN:
+        ret = json_boolean(object->data.boolean);
+        break;
+
+    case VIR_JSON_TYPE_NULL:
+        ret = json_null();
+        break;
+
+    default:
+        virReportEnumRangeError(virJSONType, object->type);
+        goto error;
+    }
+    if (!ret)
+        virReportOOMError();
+    return ret;
+
+ error:
+    json_decref(ret);
+    return NULL;
+}
+
+
+char *
+virJSONValueToString(virJSONValuePtr object,
+                     bool pretty)
+{
+    size_t flags = JSON_ENCODE_ANY;
+    json_t *json;
+    char *str = NULL;
+
+    if (pretty)
+        flags |= JSON_INDENT(2);
+    else
+        flags |= JSON_COMPACT;
+
+    json = virJSONValueToJansson(object);
+    if (!json)
+        return NULL;
+
+    str = json_dumps(json, flags);
+    if (!str)
+        virReportOOMError();
+    json_decref(json);
+    return str;
+}
+
+
 #else
 virJSONValuePtr
 virJSONValueFromString(const char *jsonstring ATTRIBUTE_UNUSED)
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 03/13] Switch from yajl to Jansson
Posted by Ján Tomko, 1 week ago
On Fri, May 11, 2018 at 02:59:04PM +0200, J�n Tomko wrote:
>Yajl has not seen much activity upstream recently.
>Switch to using Jansson >= 2.7.
>
>All the platforms we target on https://libvirt.org/platforms.html
>have a version >= 2.7 listed on the sites below:
>https://repology.org/metapackage/jansson/versions
>https://build.opensuse.org/package/show/devel:libraries:c_c++/libjansson
>
>Implement virJSONValue{From,To}String using Jansson, delete the yajl
>code (and the related virJSONParser structure) and report an error
>if someone explicitly specifies --with-yajl.
>
>Also adjust the test data to account for Jansson's different whitespace
>usage for empty arrays and tune up the specfile to keep 'make rpm'
>working when bisecting.
>
>Signed-off-by: J�n Tomko <jtomko@redhat.com>
>---
> src/util/virjson.c | 211 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 211 insertions(+)
>

With this change, it should be possible to build with Jansson 2.5 as
well:
diff --git a/src/util/virjson.c b/src/util/virjson.c
index 2f7d624bb3..0d7d368c8a 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -2056,8 +2056,7 @@ virJSONValueFromJansson(json_t *json)
         break;

     case JSON_STRING:
-        ret = virJSONValueNewStringLen(json_string_value(json),
-                                       json_string_length(json));
+        ret = virJSONValueNewString(json_string_value(json));
         break;

     case JSON_INTEGER:
@@ -2070,7 +2069,7 @@ virJSONValueFromJansson(json_t *json)

     case JSON_TRUE:
     case JSON_FALSE:
-        ret = virJSONValueNewBoolean(json_boolean_value(json));
+        ret = virJSONValueNewBoolean(json_is_true(json));
         break;

     case JSON_NULL:

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 03/13] Switch from yajl to Jansson
Posted by Peter Krempa, 1 week ago
On Tue, May 15, 2018 at 14:45:05 +0200, J�n Tomko wrote:
> On Fri, May 11, 2018 at 02:59:04PM +0200, J�n Tomko wrote:
> > Yajl has not seen much activity upstream recently.
> > Switch to using Jansson >= 2.7.
> > 
> > All the platforms we target on https://libvirt.org/platforms.html
> > have a version >= 2.7 listed on the sites below:
> > https://repology.org/metapackage/jansson/versions
> > https://build.opensuse.org/package/show/devel:libraries:c_c++/libjansson
> > 
> > Implement virJSONValue{From,To}String using Jansson, delete the yajl
> > code (and the related virJSONParser structure) and report an error
> > if someone explicitly specifies --with-yajl.
> > 
> > Also adjust the test data to account for Jansson's different whitespace
> > usage for empty arrays and tune up the specfile to keep 'make rpm'
> > working when bisecting.
> > 
> > Signed-off-by: J�n Tomko <jtomko@redhat.com>
> > ---
> > src/util/virjson.c | 211 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 211 insertions(+)
> > 
> 
> With this change, it should be possible to build with Jansson 2.5 as
> well:
> diff --git a/src/util/virjson.c b/src/util/virjson.c
> index 2f7d624bb3..0d7d368c8a 100644
> --- a/src/util/virjson.c
> +++ b/src/util/virjson.c
> @@ -2056,8 +2056,7 @@ virJSONValueFromJansson(json_t *json)
>         break;
> 
>     case JSON_STRING:
> -        ret = virJSONValueNewStringLen(json_string_value(json),
> -                                       json_string_length(json));
> +        ret = virJSONValueNewString(json_string_value(json));
>         break;
> 
>     case JSON_INTEGER:
> @@ -2070,7 +2069,7 @@ virJSONValueFromJansson(json_t *json)
> 
>     case JSON_TRUE:
>     case JSON_FALSE:
> -        ret = virJSONValueNewBoolean(json_boolean_value(json));
> +        ret = virJSONValueNewBoolean(json_is_true(json));

Given how that macro works. I'd rather see following:
     case JSON_TRUE:
          ret = virJSONValueNewBoolean(true);
          break;

     case JSON_FALSE:
          ret = virJSONValueNewBoolean(false);
          break;
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 03/13] Switch from yajl to Jansson
Posted by Peter Krempa, 1 week ago
On Fri, May 11, 2018 at 14:59:04 +0200, J�n Tomko wrote:
> Yajl has not seen much activity upstream recently.

[0]

> Switch to using Jansson >= 2.7.
> 
> All the platforms we target on https://libvirt.org/platforms.html
> have a version >= 2.7 listed on the sites below:
> https://repology.org/metapackage/jansson/versions
> https://build.opensuse.org/package/show/devel:libraries:c_c++/libjansson
> 
> Implement virJSONValue{From,To}String using Jansson, delete the yajl
> code (and the related virJSONParser structure) and report an error
> if someone explicitly specifies --with-yajl.
> 
> Also adjust the test data to account for Jansson's different whitespace
> usage for empty arrays and tune up the specfile to keep 'make rpm'
> working when bisecting.
> 
> Signed-off-by: J�n Tomko <jtomko@redhat.com>
> ---
>  src/util/virjson.c | 211 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 211 insertions(+)
> 
> diff --git a/src/util/virjson.c b/src/util/virjson.c
> index 0559d40b64..2f7d624bb3 100644
> --- a/src/util/virjson.c
> +++ b/src/util/virjson.c
> @@ -2008,6 +2008,217 @@ virJSONValueToString(virJSONValuePtr object,

[...]

> +static json_t *
> +virJSONValueToJansson(virJSONValuePtr object)
> +{
> +    json_t *ret = NULL;
> +    size_t i;
> +
> +    switch ((virJSONType)object->type) {
> +    case VIR_JSON_TYPE_OBJECT:
> +        ret = json_object();
> +        if (!ret) {
> +            virReportOOMError();
> +            goto error;
> +        }
> +        for (i = 0; i < object->data.object.npairs; i++) {
> +            virJSONObjectPairPtr cur = object->data.object.pairs + i;
> +            json_t *val = virJSONValueToJansson(cur->value);
> +
> +            if (!val)
> +                goto error;
> +            if (json_object_set_new(ret, cur->key, val) < 0) {
> +                virReportOOMError();

'val' is leaked here.

> +                goto error;
> +            }
> +        }
> +        break;
> +
> +    case VIR_JSON_TYPE_ARRAY:
> +        ret = json_array();
> +        if (!ret) {
> +            virReportOOMError();
> +            goto error;
> +        }
> +        for (i = 0; i < object->data.array.nvalues; i++) {
> +            virJSONValuePtr cur = object->data.array.values[i];
> +            json_t *val = virJSONValueToJansson(cur);
> +
> +            if (!val)
> +                goto error;
> +            if (json_array_append_new(ret, val) < 0) {
> +                virReportOOMError();

'val' is leaked here.

> +                goto error;
> +            }
> +        }
> +        break;
> +
> +    case VIR_JSON_TYPE_STRING:
> +        ret = json_string(object->data.string);
> +        break;
> +
> +    case VIR_JSON_TYPE_NUMBER: {
> +        long long ll_val;
> +        double d_val;
> +        if (virStrToLong_ll(object->data.number, NULL, 10, &ll_val) < 0) {
> +            if (virStrToDouble(object->data.number, NULL, &d_val) < 0) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("JSON value is not a number"));
> +                return NULL;
> +            }
> +            ret = json_real(d_val);
> +        } else {
> +            ret = json_integer(ll_val);
> +        }
> +    }
> +        break;
> +
> +    case VIR_JSON_TYPE_BOOLEAN:
> +        ret = json_boolean(object->data.boolean);
> +        break;
> +
> +    case VIR_JSON_TYPE_NULL:
> +        ret = json_null();
> +        break;
> +
> +    default:
> +        virReportEnumRangeError(virJSONType, object->type);
> +        goto error;
> +    }
> +    if (!ret)
> +        virReportOOMError();
> +    return ret;

Few lines could be saved by having a dedicated label for cases when
virReportOOMError needs to be called.

> +
> + error:
> +    json_decref(ret);
> +    return NULL;

ACK if you fix the leaks. The separate label is up to you. Don't forget
to update the commit message after the squash-in.

Peter

[0] For anyone following current meme trends, this looks like it's
relevant to our YAJL->janson switch:

https://i.redditmedia.com/J46fZN24lFx3fMRlJNpkNEOFqU79zWTsRDBMla1u0HE.jpg?s=4757d31d1cbd758962407917e0d3aacb
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 03/13] Switch from yajl to Jansson
Posted by Daniel P. Berrangé, 1 week ago
On Tue, May 15, 2018 at 03:05:55PM +0200, Peter Krempa wrote:
> On Fri, May 11, 2018 at 14:59:04 +0200, Ján Tomko wrote:
> > Yajl has not seen much activity upstream recently.
> 
> [0]

[snip]
> [0] For anyone following current meme trends, this looks like it's
> relevant to our YAJL->janson switch:
> 
> https://i.redditmedia.com/J46fZN24lFx3fMRlJNpkNEOFqU79zWTsRDBMla1u0HE.jpg?s=4757d31d1cbd758962407917e0d3aacb

If you think we should stay with YAJL because it "just works" then
take a look at the bug reports against it upstream

  https://github.com/lloyd/yajl/issues

double frees, memory leaks, parser gets stuck after parsing bad JSON, etc
all ignored for years.  The robustness of the JSON parser we use is
critical to the security of the libvirt QEMU driver.  I'm not saying
janson is perfect, but it is at least maintained, so when problem in
it as discovered, there's a real possibility to get them fixed, instead
of ignored with the YAJL abandonware.

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] [PATCHv3 03/13] Switch from yajl to Jansson
Posted by Peter Krempa, 1 week ago
On Tue, May 15, 2018 at 14:21:07 +0100, Daniel Berrange wrote:
> On Tue, May 15, 2018 at 03:05:55PM +0200, Peter Krempa wrote:
> > On Fri, May 11, 2018 at 14:59:04 +0200, J�n Tomko wrote:
> > > Yajl has not seen much activity upstream recently.
> > 
> > [0]
> 
> [snip]
> > [0] For anyone following current meme trends, this looks like it's
> > relevant to our YAJL->janson switch:
> > 
> > https://i.redditmedia.com/J46fZN24lFx3fMRlJNpkNEOFqU79zWTsRDBMla1u0HE.jpg?s=4757d31d1cbd758962407917e0d3aacb
> 
> If you think we should stay with YAJL because it "just works" then
> take a look at the bug reports against it upstream
> 
>   https://github.com/lloyd/yajl/issues
> 
> double frees, memory leaks, parser gets stuck after parsing bad JSON, etc
> all ignored for years.  The robustness of the JSON parser we use is
> critical to the security of the libvirt QEMU driver.  I'm not saying
> janson is perfect, but it is at least maintained, so when problem in
> it as discovered, there's a real possibility to get them fixed, instead
> of ignored with the YAJL abandonware.

No. I agree with the change, but "having serious flaws" is slightly
different than "not very active upstream".
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 03/13] Switch from yajl to Jansson
Posted by Daniel P. Berrangé, 1 week ago
On Tue, May 15, 2018 at 03:23:17PM +0200, Peter Krempa wrote:
> On Tue, May 15, 2018 at 14:21:07 +0100, Daniel Berrange wrote:
> > On Tue, May 15, 2018 at 03:05:55PM +0200, Peter Krempa wrote:
> > > On Fri, May 11, 2018 at 14:59:04 +0200, Ján Tomko wrote:
> > > > Yajl has not seen much activity upstream recently.
> > > 
> > > [0]
> > 
> > [snip]
> > > [0] For anyone following current meme trends, this looks like it's
> > > relevant to our YAJL->janson switch:
> > > 
> > > https://i.redditmedia.com/J46fZN24lFx3fMRlJNpkNEOFqU79zWTsRDBMla1u0HE.jpg?s=4757d31d1cbd758962407917e0d3aacb
> > 
> > If you think we should stay with YAJL because it "just works" then
> > take a look at the bug reports against it upstream
> > 
> >   https://github.com/lloyd/yajl/issues
> > 
> > double frees, memory leaks, parser gets stuck after parsing bad JSON, etc
> > all ignored for years.  The robustness of the JSON parser we use is
> > critical to the security of the libvirt QEMU driver.  I'm not saying
> > janson is perfect, but it is at least maintained, so when problem in
> > it as discovered, there's a real possibility to get them fixed, instead
> > of ignored with the YAJL abandonware.
> 
> No. I agree with the change, but "having serious flaws" is slightly
> different than "not very active upstream".

I don't think that makes much difference. All non-trivial software has
serious flaws, especially if written in memory-unsafe languages like
C. The only question is whether the flaws have been found yet. Given
that, whether there is an active upstream maintaining it, is a key
factor to decide whether to continue using something. The fact that
we already see significant unfixed flaws just reinforces how important
it is to have an active upstream.

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
[libvirt] [PATCHv3 04/13] FIXUP: fix tests
Posted by Ján Tomko, 1 week ago
Jansson does not add an extra newline after the output
and it formats empty elements differently.
---
 tests/qemublocktest.c                    | 1 +
 tests/qemumigparamsdata/empty.json       | 4 +---
 tests/qemumigparamsdata/unsupported.json | 4 +---
 tests/virmacmaptestdata/empty.json       | 4 +---
 4 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index eae1ca8ee3..4bed82b4f5 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -301,6 +301,7 @@ testQemuDiskXMLToPropsValidateFile(const void *opaque)
             goto cleanup;
 
         virBufferAdd(&buf, jsonstr, -1);
+        virBufferAddLit(&buf, "\n");
         VIR_FREE(jsonstr);
     }
 
diff --git a/tests/qemumigparamsdata/empty.json b/tests/qemumigparamsdata/empty.json
index 0db3279e44..0967ef424b 100644
--- a/tests/qemumigparamsdata/empty.json
+++ b/tests/qemumigparamsdata/empty.json
@@ -1,3 +1 @@
-{
-
-}
+{}
diff --git a/tests/qemumigparamsdata/unsupported.json b/tests/qemumigparamsdata/unsupported.json
index 0db3279e44..0967ef424b 100644
--- a/tests/qemumigparamsdata/unsupported.json
+++ b/tests/qemumigparamsdata/unsupported.json
@@ -1,3 +1 @@
-{
-
-}
+{}
diff --git a/tests/virmacmaptestdata/empty.json b/tests/virmacmaptestdata/empty.json
index 41b42e677b..fe51488c70 100644
--- a/tests/virmacmaptestdata/empty.json
+++ b/tests/virmacmaptestdata/empty.json
@@ -1,3 +1 @@
-[
-
-]
+[]
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 04/13] FIXUP: fix tests
Posted by Peter Krempa, 1 week ago
On Fri, May 11, 2018 at 14:59:05 +0200, J�n Tomko wrote:
> Jansson does not add an extra newline after the output
> and it formats empty elements differently.
> ---
>  tests/qemublocktest.c                    | 1 +
>  tests/qemumigparamsdata/empty.json       | 4 +---
>  tests/qemumigparamsdata/unsupported.json | 4 +---
>  tests/virmacmaptestdata/empty.json       | 4 +---
>  4 files changed, 4 insertions(+), 9 deletions(-)

ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 05/13] FIXUP: Deprecate building --with-yajl
Posted by Ján Tomko, 1 week ago
---
 m4/virt-yajl.m4 | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/m4/virt-yajl.m4 b/m4/virt-yajl.m4
index c4ea0102a3..8d4c43a6b2 100644
--- a/m4/virt-yajl.m4
+++ b/m4/virt-yajl.m4
@@ -23,31 +23,10 @@ AC_DEFUN([LIBVIRT_ARG_YAJL],[
 
 AC_DEFUN([LIBVIRT_CHECK_YAJL],[
   dnl YAJL JSON library http://lloyd.github.com/yajl/
-  if test "$with_qemu:$with_yajl" = yes:check; then
-    dnl Some versions of qemu require the use of yajl; try to detect them
-    dnl here, although we do not require qemu to exist in order to compile.
-    dnl This check mirrors src/qemu/qemu_capabilities.c
-    AC_PATH_PROGS([QEMU], [qemu-kvm qemu kvm qemu-system-x86_64],
-                  [], [$PATH:/usr/bin:/usr/libexec])
-    if test -x "$QEMU"; then
-      if $QEMU -help 2>/dev/null | grep -q libvirt; then
-        with_yajl=yes
-      else
-        [qemu_version_sed='s/.*ersion \([0-9.,]*\).*/\1/']
-        qemu_version=`$QEMU -version | sed "$qemu_version_sed"`
-        case $qemu_version in
-          [[1-9]].* | 0.15.* ) with_yajl=yes ;;
-          0.* | '' ) ;;
-          *) AC_MSG_ERROR([Unexpected qemu version string]) ;;
-        esac
-      fi
-    fi
+  if test "$with_yajl" = yes; then
+    AC_MSG_ERROR([Compilation with YAJL is no longer supported])
   fi
-
-  LIBVIRT_CHECK_LIB_ALT([YAJL], [yajl],
-                        [yajl_parse_complete], [yajl/yajl_common.h],
-                        [YAJL2], [yajl],
-                        [yajl_tree_parse], [yajl/yajl_common.h])
+  with_yajl=no
 ])
 
 AC_DEFUN([LIBVIRT_RESULT_YAJL],[
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 05/13] FIXUP: Deprecate building --with-yajl
Posted by Peter Krempa, 1 week ago
On Fri, May 11, 2018 at 14:59:06 +0200, J�n Tomko wrote:
> ---
>  m4/virt-yajl.m4 | 27 +++------------------------
>  1 file changed, 3 insertions(+), 24 deletions(-)
> 

ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 06/13] FIXUP: make nss depend on Jansson instead of yajl
Posted by Ján Tomko, 1 week ago
---
 m4/virt-nss.m4 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/m4/virt-nss.m4 b/m4/virt-nss.m4
index 951a74e835..082b7b14f6 100644
--- a/m4/virt-nss.m4
+++ b/m4/virt-nss.m4
@@ -27,9 +27,9 @@ AC_DEFUN([LIBVIRT_CHECK_NSS],[
   bsd_nss=no
   fail=0
   if test "x$with_nss_plugin" != "xno" ; then
-    if test "x$with_yajl" != "xyes" ; then
+    if test "x$with_jansson" != "xyes" ; then
       if test "x$with_nss_plugin" = "xyes" ; then
-        AC_MSG_ERROR([Can't build nss plugin without yajl])
+        AC_MSG_ERROR([Can't build nss plugin without JSON support])
       else
         with_nss_plugin=no
       fi
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 07/13] FIXUP: compile and link with Jansson instead of yajl
Posted by Ján Tomko, 1 week ago
---
 src/Makefile.am          | 8 ++++----
 src/util/Makefile.inc.am | 4 ++--
 tests/Makefile.am        | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 0c380780c3..361cc98f0f 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -550,7 +550,7 @@ libvirt_admin_la_CFLAGS = \
 libvirt_admin_la_CFLAGS += \
 		$(XDR_CFLAGS) \
 		$(CAPNG_CFLAGS) \
-		$(YAJL_CFLAGS) \
+		$(JANSSON_CFLAGS) \
 		$(SSH2_CFLAGS) \
 		$(SASL_CFLAGS) \
 		$(GNUTLS_CFLAGS) \
@@ -558,7 +558,7 @@ libvirt_admin_la_CFLAGS += \
 
 libvirt_admin_la_LIBADD += \
 		$(CAPNG_LIBS) \
-		$(YAJL_LIBS) \
+		$(JANSSON_LIBS) \
 		$(DEVMAPPER_LIBS) \
 		$(LIBXML_LIBS) \
 		$(SSH2_LIBS) \
@@ -1000,14 +1000,14 @@ libvirt_nss_la_SOURCES = \
 libvirt_nss_la_CFLAGS = \
 		-DLIBVIRT_NSS \
 		$(AM_CFLAGS) \
-		$(YAJL_CFLAGS) \
+		$(JANSSON_CFLAGS) \
 		$(NULL)
 libvirt_nss_la_LDFLAGS = \
 		$(AM_LDFLAGS) \
 		$(NULL)
 
 libvirt_nss_la_LIBADD = \
-		$(YAJL_LIBS) \
+		$(JANSSON_LIBS) \
 		$(NULL)
 endif WITH_NSS
 
diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
index ec8745da7e..b8f547d321 100644
--- a/src/util/Makefile.inc.am
+++ b/src/util/Makefile.inc.am
@@ -249,7 +249,7 @@ libvirt_util_la_SOURCES = \
 	$(NULL)
 libvirt_util_la_CFLAGS = \
 	$(CAPNG_CFLAGS) \
-	$(YAJL_CFLAGS) \
+	$(JANSSON_CFLAGS) \
 	$(LIBNL_CFLAGS) \
 	$(AM_CFLAGS) \
 	$(AUDIT_CFLAGS) \
@@ -262,7 +262,7 @@ libvirt_util_la_CFLAGS = \
 	$(NULL)
 libvirt_util_la_LIBADD = \
 	$(CAPNG_LIBS) \
-	$(YAJL_LIBS) \
+	$(JANSSON_LIBS) \
 	$(LIBNL_LIBS) \
 	$(THREAD_LIBS) \
 	$(AUDIT_LIBS) \
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 621480dd0c..ddff0bbcea 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -46,7 +46,7 @@ AM_CFLAGS = \
 	$(SASL_CFLAGS) \
 	$(SELINUX_CFLAGS) \
 	$(APPARMOR_CFLAGS) \
-	$(YAJL_CFLAGS) \
+	$(JANSSON_CFLAGS) \
 	$(COVERAGE_CFLAGS) \
 	$(XDR_CFLAGS) \
 	$(WARN_CFLAGS)
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 07/13] FIXUP: compile and link with Jansson instead of yajl
Posted by Peter Krempa, 1 week ago
On Fri, May 11, 2018 at 14:59:08 +0200, J�n Tomko wrote:
> ---
>  src/Makefile.am          | 8 ++++----
>  src/util/Makefile.inc.am | 4 ++--
>  tests/Makefile.am        | 2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)

ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 08/13] FIXUP: s/WITH_YAJL/WITH_JANSSON/
Posted by Ján Tomko, 1 week ago
---
 src/qemu/qemu_driver.c         |  2 +-
 tests/Makefile.am              | 10 +++++-----
 tests/cputest.c                | 16 ++++++++--------
 tests/libxlxml2domconfigtest.c |  4 ++--
 tests/qemuagenttest.c          |  2 +-
 tests/qemucapabilitiestest.c   |  2 +-
 tests/qemucaps2xmltest.c       |  2 +-
 tests/qemucommandutiltest.c    |  2 +-
 tests/qemuhotplugtest.c        |  2 +-
 tests/qemumigparamstest.c      |  2 +-
 tests/qemumonitorjsontest.c    |  2 +-
 tests/virmocklibxl.c           |  4 ++--
 tests/virnetdaemontest.c       |  2 +-
 tests/virstoragetest.c         |  4 ++--
 14 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b03eb3042d..da23b7be2c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2112,7 +2112,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags)
      */
     if ((!useAgent) ||
         (ret < 0 && (acpiRequested || !flags))) {
-#if !WITH_YAJL
+#if !WITH_JANSSON
         virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                        _("ACPI reboot is not supported without the JSON monitor"));
         goto endjob;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index ddff0bbcea..4092f99713 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -331,9 +331,9 @@ if WITH_CIL
 test_programs += objectlocking
 endif WITH_CIL
 
-if WITH_YAJL
+if WITH_JANSSON
 test_programs += virjsontest
-endif WITH_YAJL
+endif WITH_JANSSON
 
 test_programs += \
 		networkxml2xmltest \
@@ -1227,15 +1227,15 @@ virdeterministichashmock_la_LIBADD = $(MOCKLIBS_LIBS)
 
 test_libraries += virdeterministichashmock.la
 
-if WITH_YAJL
+if WITH_JANSSON
 virmacmaptest_SOURCES = \
 	virmacmaptest.c testutils.h testutils.c
 virmacmaptest_LDADD = $(LDADDS)
 
 test_programs += virmacmaptest
-else ! WITH_YAJL
+else ! WITH_JANSSON
 EXTRA_DIST +=  virmacmaptest.c
-endif ! WITH_YAJL
+endif ! WITH_JANSSON
 
 virnetdevtest_SOURCES = \
 	virnetdevtest.c testutils.h testutils.c
diff --git a/tests/cputest.c b/tests/cputest.c
index e86cd0b9bc..83c9cb0a35 100644
--- a/tests/cputest.c
+++ b/tests/cputest.c
@@ -40,7 +40,7 @@
 #include "cpu/cpu_map.h"
 #include "virstring.h"
 
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU && WITH_JANSSON
 # include "testutilsqemu.h"
 # include "qemumonitortestutils.h"
 # define __QEMU_CAPSPRIV_H_ALLOW__
@@ -67,7 +67,7 @@ struct data {
     int result;
 };
 
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU && WITH_JANSSON
 static virQEMUDriver driver;
 #endif
 
@@ -479,7 +479,7 @@ typedef enum {
     JSON_MODELS_REQUIRED,
 } cpuTestCPUIDJson;
 
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU && WITH_JANSSON
 static virQEMUCapsPtr
 cpuTestMakeQEMUCaps(const struct data *data)
 {
@@ -554,7 +554,7 @@ cpuTestGetCPUModels(const struct data *data,
     return 0;
 }
 
-#else /* if WITH_QEMU && WITH_YAJL */
+#else /* if WITH_QEMU && WITH_JANSSON */
 
 static int
 cpuTestGetCPUModels(const struct data *data,
@@ -834,7 +834,7 @@ cpuTestUpdateLive(const void *arg)
 }
 
 
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU && WITH_JANSSON
 static int
 cpuTestJSONCPUID(const void *arg)
 {
@@ -911,7 +911,7 @@ mymain(void)
     virDomainCapsCPUModelsPtr ppc_models = NULL;
     int ret = 0;
 
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU && WITH_JANSSON
     if (qemuTestDriverInit(&driver) < 0)
         return EXIT_FAILURE;
 
@@ -1004,7 +1004,7 @@ mymain(void)
             host "/" cpu " (" #models ")", \
             host, cpu, models, 0, result)
 
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU && WITH_JANSSON
 # define DO_TEST_JSON(arch, host, json) \
     do { \
         if (json == JSON_MODELS) { \
@@ -1205,7 +1205,7 @@ mymain(void)
     DO_TEST_CPUID(VIR_ARCH_X86_64, "Xeon-X5460", JSON_NONE);
 
  cleanup:
-#if WITH_QEMU && WITH_YAJL
+#if WITH_QEMU && WITH_JANSSON
     qemuTestDriverFree(&driver);
 #endif
 
diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c
index 0d2a7385e5..5105874cdc 100644
--- a/tests/libxlxml2domconfigtest.c
+++ b/tests/libxlxml2domconfigtest.c
@@ -33,7 +33,7 @@
 
 #include "testutils.h"
 
-#if defined(WITH_LIBXL) && defined(WITH_YAJL) && defined(HAVE_LIBXL_DOMAIN_CONFIG_FROM_JSON)
+#if defined(WITH_LIBXL) && defined(WITH_JANSSON) && defined(HAVE_LIBXL_DOMAIN_CONFIG_FROM_JSON)
 
 # include "internal.h"
 # include "viralloc.h"
@@ -225,4 +225,4 @@ int main(void)
     return EXIT_AM_SKIP;
 }
 
-#endif /* WITH_LIBXL && WITH_YAJL && HAVE_LIBXL_DOMAIN_CONFIG_FROM_JSON */
+#endif /* WITH_LIBXL && WITH_JANSSON && HAVE_LIBXL_DOMAIN_CONFIG_FROM_JSON */
diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index 2f79986207..232b34f9cd 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -907,7 +907,7 @@ mymain(void)
 {
     int ret = 0;
 
-#if !WITH_YAJL
+#if !WITH_JANSSON
     fputs("libvirt not compiled with JSON support, skipping this test\n", stderr);
     return EXIT_AM_SKIP;
 #endif
diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c
index 5a42a35c11..ff7610ffb3 100644
--- a/tests/qemucapabilitiestest.c
+++ b/tests/qemucapabilitiestest.c
@@ -139,7 +139,7 @@ mymain(void)
     virQEMUDriver driver;
     testQemuData data;
 
-#if !WITH_YAJL
+#if !WITH_JANSSON
     fputs("libvirt not compiled with JSON support, skipping this test\n", stderr);
     return EXIT_AM_SKIP;
 #endif
diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c
index 5b9152b04d..e3b7b97925 100644
--- a/tests/qemucaps2xmltest.c
+++ b/tests/qemucaps2xmltest.c
@@ -165,7 +165,7 @@ mymain(void)
 
     testQemuData data;
 
-#if !WITH_YAJL
+#if !WITH_JANSSON
     fputs("libvirt not compiled with JSON support, skipping this test\n", stderr);
     return EXIT_AM_SKIP;
 #endif
diff --git a/tests/qemucommandutiltest.c b/tests/qemucommandutiltest.c
index f0921e3b93..8e57a1b79d 100644
--- a/tests/qemucommandutiltest.c
+++ b/tests/qemucommandutiltest.c
@@ -76,7 +76,7 @@ mymain(void)
     int ret = 0;
     testQemuCommandBuildObjectFromJSONData data1;
 
-#if !WITH_YAJL
+#if !WITH_JANSSON
     fputs("libvirt not compiled with JSON support, skipping this test\n", stderr);
     return EXIT_AM_SKIP;
 #endif
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index dc0d2cf284..ddd3ae40c1 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -588,7 +588,7 @@ mymain(void)
     struct qemuHotplugTestData data = {0};
     struct testQemuHotplugCpuParams cpudata;
 
-#if !WITH_YAJL
+#if !WITH_JANSSON
     fputs("libvirt not compiled with JSON support, skipping this test\n", stderr);
     return EXIT_AM_SKIP;
 #endif
diff --git a/tests/qemumigparamstest.c b/tests/qemumigparamstest.c
index 0532053722..b8af68211b 100644
--- a/tests/qemumigparamstest.c
+++ b/tests/qemumigparamstest.c
@@ -203,7 +203,7 @@ mymain(void)
     virQEMUDriver driver;
     int ret = 0;
 
-#if !WITH_YAJL
+#if !WITH_JANSSON
     fputs("libvirt not compiled with JSON support, skipping this test\n", stderr);
     return EXIT_AM_SKIP;
 #endif
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index edd57067bd..1e427c8bb0 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -2864,7 +2864,7 @@ mymain(void)
     struct testQAPISchemaData qapiData;
     char *metaschema = NULL;
 
-#if !WITH_YAJL
+#if !WITH_JANSSON
     fputs("libvirt not compiled with JSON support, skipping this test\n", stderr);
     return EXIT_AM_SKIP;
 #endif
diff --git a/tests/virmocklibxl.c b/tests/virmocklibxl.c
index 546c6d6a43..0a047c239f 100644
--- a/tests/virmocklibxl.c
+++ b/tests/virmocklibxl.c
@@ -22,7 +22,7 @@
 
 #include <config.h>
 
-#if defined(WITH_LIBXL) && defined(WITH_YAJL)
+#if defined(WITH_LIBXL) && defined(WITH_JANSSON)
 # include "virmock.h"
 # include <sys/stat.h>
 # include <unistd.h>
@@ -136,4 +136,4 @@ VIR_MOCK_IMPL_RET_ARGS(stat, int,
     return real_stat(path, sb);
 }
 
-#endif /* WITH_LIBXL && WITH_YAJL */
+#endif /* WITH_LIBXL && WITH_JANSSON */
diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c
index ef869b16e3..7167147b8b 100644
--- a/tests/virnetdaemontest.c
+++ b/tests/virnetdaemontest.c
@@ -26,7 +26,7 @@
 
 #define VIR_FROM_THIS VIR_FROM_RPC
 
-#if defined(HAVE_SOCKETPAIR) && defined(WITH_YAJL)
+#if defined(HAVE_SOCKETPAIR) && defined(WITH_JANSSON)
 struct testClientPriv {
     int magic;
 };
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 0e11602e84..3092134edd 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -1364,7 +1364,7 @@ mymain(void)
                        "  <host name='example.org' port='6000'/>\n"
                        "</source>\n");
 
-#ifdef WITH_YAJL
+#ifdef WITH_JANSSON
     TEST_BACKING_PARSE("json:", NULL);
     TEST_BACKING_PARSE("json:asdgsdfg", NULL);
     TEST_BACKING_PARSE("json:{}", NULL);
@@ -1628,7 +1628,7 @@ mymain(void)
                        "<source protocol='vxhs' name='c6718f6b-0401-441d-a8c3-1f0064d75ee0'>\n"
                        "  <host name='example.com' port='9999'/>\n"
                        "</source>\n");
-#endif /* WITH_YAJL */
+#endif /* WITH_JANSSON */
 
  cleanup:
     /* Final cleanup */
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 08/13] FIXUP: s/WITH_YAJL/WITH_JANSSON/
Posted by Peter Krempa, 1 week ago
On Fri, May 11, 2018 at 14:59:09 +0200, J�n Tomko wrote:
> ---
>  src/qemu/qemu_driver.c         |  2 +-
>  tests/Makefile.am              | 10 +++++-----
>  tests/cputest.c                | 16 ++++++++--------
>  tests/libxlxml2domconfigtest.c |  4 ++--
>  tests/qemuagenttest.c          |  2 +-
>  tests/qemucapabilitiestest.c   |  2 +-
>  tests/qemucaps2xmltest.c       |  2 +-
>  tests/qemucommandutiltest.c    |  2 +-
>  tests/qemuhotplugtest.c        |  2 +-
>  tests/qemumigparamstest.c      |  2 +-
>  tests/qemumonitorjsontest.c    |  2 +-
>  tests/virmocklibxl.c           |  4 ++--
>  tests/virnetdaemontest.c       |  2 +-
>  tests/virstoragetest.c         |  4 ++--
>  14 files changed, 28 insertions(+), 28 deletions(-)

ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 09/13] FIXUP: libvirt.spec: use jansson instead of yajl
Posted by Ján Tomko, 1 week ago
---
 libvirt.spec.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 9ea5e6b32a..f653b98465 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -336,7 +336,7 @@ BuildRequires: systemd-devel >= 185
 BuildRequires: libudev-devel >= 145
 %endif
 BuildRequires: libpciaccess-devel >= 0.10.9
-BuildRequires: yajl-devel
+BuildRequires: jansson-devel
 %if %{with_sanlock}
 BuildRequires: sanlock-devel >= 2.4
 %endif
@@ -1304,7 +1304,7 @@ rm -f po/stamp-po
            --without-apparmor \
            --without-hal \
            --with-udev \
-           --with-yajl \
+           --with-jansson \
            %{?arg_sanlock} \
            --with-libpcap \
            --with-macvtap \
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 10/13] Remove functions using yajl
Posted by Ján Tomko, 1 week ago
We no longer support building WITH_YAJL, remove the dead code
as well as the virJSONParser structures that are no longer used.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 src/util/virjson.c | 530 +----------------------------------------------------
 1 file changed, 1 insertion(+), 529 deletions(-)

diff --git a/src/util/virjson.c b/src/util/virjson.c
index 2f7d624bb3..b5d3de86f8 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -30,22 +30,6 @@
 #include "virstring.h"
 #include "virutil.h"
 
-#if WITH_YAJL
-# include <yajl/yajl_gen.h>
-# include <yajl/yajl_parse.h>
-
-# ifdef WITH_YAJL2
-#  define yajl_size_t size_t
-#  define VIR_YAJL_STATUS_OK(status) ((status) == yajl_status_ok)
-# else
-#  define yajl_size_t unsigned int
-#  define yajl_complete_parse yajl_parse_complete
-#  define VIR_YAJL_STATUS_OK(status) \
-    ((status) == yajl_status_ok || (status) == yajl_status_insufficient_data)
-# endif
-
-#endif
-
 /* XXX fixme */
 #define VIR_FROM_THIS VIR_FROM_NONE
 
@@ -89,23 +73,6 @@ struct _virJSONValue {
 };
 
 
-typedef struct _virJSONParserState virJSONParserState;
-typedef virJSONParserState *virJSONParserStatePtr;
-struct _virJSONParserState {
-    virJSONValuePtr value;
-    char *key;
-};
-
-typedef struct _virJSONParser virJSONParser;
-typedef virJSONParser *virJSONParserPtr;
-struct _virJSONParser {
-    virJSONValuePtr head;
-    virJSONParserStatePtr state;
-    size_t nstate;
-    int wrap;
-};
-
-
 virJSONType
 virJSONValueGetType(const virJSONValue *value)
 {
@@ -1513,502 +1480,7 @@ virJSONValueCopy(const virJSONValue *in)
 }
 
 
-#if WITH_YAJL
-static int
-virJSONParserInsertValue(virJSONParserPtr parser,
-                         virJSONValuePtr value)
-{
-    if (!parser->head) {
-        parser->head = value;
-    } else {
-        virJSONParserStatePtr state;
-        if (!parser->nstate) {
-            VIR_DEBUG("got a value to insert without a container");
-            return -1;
-        }
-
-        state = &parser->state[parser->nstate-1];
-
-        switch (state->value->type) {
-        case VIR_JSON_TYPE_OBJECT: {
-            if (!state->key) {
-                VIR_DEBUG("missing key when inserting object value");
-                return -1;
-            }
-
-            if (virJSONValueObjectAppend(state->value,
-                                         state->key,
-                                         value) < 0)
-                return -1;
-
-            VIR_FREE(state->key);
-        }   break;
-
-        case VIR_JSON_TYPE_ARRAY: {
-            if (state->key) {
-                VIR_DEBUG("unexpected key when inserting array value");
-                return -1;
-            }
-
-            if (virJSONValueArrayAppend(state->value,
-                                        value) < 0)
-                return -1;
-        }   break;
-
-        default:
-            VIR_DEBUG("unexpected value type, not a container");
-            return -1;
-        }
-    }
-
-    return 0;
-}
-
-
-static int
-virJSONParserHandleNull(void *ctx)
-{
-    virJSONParserPtr parser = ctx;
-    virJSONValuePtr value = virJSONValueNewNull();
-
-    VIR_DEBUG("parser=%p", parser);
-
-    if (!value)
-        return 0;
-
-    if (virJSONParserInsertValue(parser, value) < 0) {
-        virJSONValueFree(value);
-        return 0;
-    }
-
-    return 1;
-}
-
-
-static int
-virJSONParserHandleBoolean(void *ctx,
-                           int boolean_)
-{
-    virJSONParserPtr parser = ctx;
-    virJSONValuePtr value = virJSONValueNewBoolean(boolean_);
-
-    VIR_DEBUG("parser=%p boolean=%d", parser, boolean_);
-
-    if (!value)
-        return 0;
-
-    if (virJSONParserInsertValue(parser, value) < 0) {
-        virJSONValueFree(value);
-        return 0;
-    }
-
-    return 1;
-}
-
-
-static int
-virJSONParserHandleNumber(void *ctx,
-                          const char *s,
-                          yajl_size_t l)
-{
-    virJSONParserPtr parser = ctx;
-    char *str;
-    virJSONValuePtr value;
-
-    if (VIR_STRNDUP(str, s, l) < 0)
-        return -1;
-    value = virJSONValueNewNumber(str);
-    VIR_FREE(str);
-
-    VIR_DEBUG("parser=%p str=%s", parser, str);
-
-    if (!value)
-        return 0;
-
-    if (virJSONParserInsertValue(parser, value) < 0) {
-        virJSONValueFree(value);
-        return 0;
-    }
-
-    return 1;
-}
-
-
-static int
-virJSONParserHandleString(void *ctx,
-                          const unsigned char *stringVal,
-                          yajl_size_t stringLen)
-{
-    virJSONParserPtr parser = ctx;
-    virJSONValuePtr value = virJSONValueNewStringLen((const char *)stringVal,
-                                                     stringLen);
-
-    VIR_DEBUG("parser=%p str=%p", parser, (const char *)stringVal);
-
-    if (!value)
-        return 0;
-
-    if (virJSONParserInsertValue(parser, value) < 0) {
-        virJSONValueFree(value);
-        return 0;
-    }
-
-    return 1;
-}
-
-
-static int
-virJSONParserHandleMapKey(void *ctx,
-                          const unsigned char *stringVal,
-                          yajl_size_t stringLen)
-{
-    virJSONParserPtr parser = ctx;
-    virJSONParserStatePtr state;
-
-    VIR_DEBUG("parser=%p key=%p", parser, (const char *)stringVal);
-
-    if (!parser->nstate)
-        return 0;
-
-    state = &parser->state[parser->nstate-1];
-    if (state->key)
-        return 0;
-    if (VIR_STRNDUP(state->key, (const char *)stringVal, stringLen) < 0)
-        return 0;
-    return 1;
-}
-
-
-static int
-virJSONParserHandleStartMap(void *ctx)
-{
-    virJSONParserPtr parser = ctx;
-    virJSONValuePtr value = virJSONValueNewObject();
-
-    VIR_DEBUG("parser=%p", parser);
-
-    if (!value)
-        return 0;
-
-    if (virJSONParserInsertValue(parser, value) < 0) {
-        virJSONValueFree(value);
-        return 0;
-    }
-
-    if (VIR_REALLOC_N(parser->state,
-                      parser->nstate + 1) < 0) {
-        return 0;
-    }
-
-    parser->state[parser->nstate].value = value;
-    parser->state[parser->nstate].key = NULL;
-    parser->nstate++;
-
-    return 1;
-}
-
-
-static int
-virJSONParserHandleEndMap(void *ctx)
-{
-    virJSONParserPtr parser = ctx;
-    virJSONParserStatePtr state;
-
-    VIR_DEBUG("parser=%p", parser);
-
-    if (!parser->nstate)
-        return 0;
-
-    state = &(parser->state[parser->nstate-1]);
-    if (state->key) {
-        VIR_FREE(state->key);
-        return 0;
-    }
-
-    VIR_DELETE_ELEMENT(parser->state, parser->nstate - 1, parser->nstate);
-
-    return 1;
-}
-
-
-static int
-virJSONParserHandleStartArray(void *ctx)
-{
-    virJSONParserPtr parser = ctx;
-    virJSONValuePtr value = virJSONValueNewArray();
-
-    VIR_DEBUG("parser=%p", parser);
-
-    if (!value)
-        return 0;
-
-    if (virJSONParserInsertValue(parser, value) < 0) {
-        virJSONValueFree(value);
-        return 0;
-    }
-
-    if (VIR_REALLOC_N(parser->state,
-                      parser->nstate + 1) < 0)
-        return 0;
-
-    parser->state[parser->nstate].value = value;
-    parser->state[parser->nstate].key = NULL;
-    parser->nstate++;
-
-    return 1;
-}
-
-
-static int
-virJSONParserHandleEndArray(void *ctx)
-{
-    virJSONParserPtr parser = ctx;
-    virJSONParserStatePtr state;
-
-    VIR_DEBUG("parser=%p", parser);
-
-    if (!(parser->nstate - parser->wrap))
-        return 0;
-
-    state = &(parser->state[parser->nstate-1]);
-    if (state->key) {
-        VIR_FREE(state->key);
-        return 0;
-    }
-
-    VIR_DELETE_ELEMENT(parser->state, parser->nstate - 1, parser->nstate);
-
-    return 1;
-}
-
-
-static const yajl_callbacks parserCallbacks = {
-    virJSONParserHandleNull,
-    virJSONParserHandleBoolean,
-    NULL,
-    NULL,
-    virJSONParserHandleNumber,
-    virJSONParserHandleString,
-    virJSONParserHandleStartMap,
-    virJSONParserHandleMapKey,
-    virJSONParserHandleEndMap,
-    virJSONParserHandleStartArray,
-    virJSONParserHandleEndArray
-};
-
-
-/* XXX add an incremental streaming parser - yajl trivially supports it */
-virJSONValuePtr
-virJSONValueFromString(const char *jsonstring)
-{
-    yajl_handle hand;
-    virJSONParser parser = { NULL, NULL, 0, 0 };
-    virJSONValuePtr ret = NULL;
-    int rc;
-    size_t len = strlen(jsonstring);
-# ifndef WITH_YAJL2
-    yajl_parser_config cfg = { 0, 1 }; /* Match yajl 2 default behavior */
-    virJSONValuePtr tmp;
-# endif
-
-    VIR_DEBUG("string=%s", jsonstring);
-
-# ifdef WITH_YAJL2
-    hand = yajl_alloc(&parserCallbacks, NULL, &parser);
-# else
-    hand = yajl_alloc(&parserCallbacks, &cfg, NULL, &parser);
-# endif
-    if (!hand) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Unable to create JSON parser"));
-        goto cleanup;
-    }
-
-    /* Yajl 2 is nice enough to default to rejecting trailing garbage.
-     * Yajl 1.0.12 has yajl_get_bytes_consumed to make that detection
-     * simpler.  But we're stuck with yajl 1.0.7 on RHEL 6, which
-     * happily quits parsing at the end of a valid JSON construct,
-     * with no visibility into how much more input remains.  Wrapping
-     * things in an array forces yajl to confess the truth.  */
-# ifdef WITH_YAJL2
-    rc = yajl_parse(hand, (const unsigned char *)jsonstring, len);
-# else
-    rc = yajl_parse(hand, (const unsigned char *)"[", 1);
-    parser.wrap = 1;
-    if (VIR_YAJL_STATUS_OK(rc))
-        rc = yajl_parse(hand, (const unsigned char *)jsonstring, len);
-    parser.wrap = 0;
-    if (VIR_YAJL_STATUS_OK(rc))
-        rc = yajl_parse(hand, (const unsigned char *)"]", 1);
-# endif
-    if (!VIR_YAJL_STATUS_OK(rc) ||
-        yajl_complete_parse(hand) != yajl_status_ok) {
-        unsigned char *errstr = yajl_get_error(hand, 1,
-                                               (const unsigned char*)jsonstring,
-                                               strlen(jsonstring));
-
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("cannot parse json %s: %s"),
-                       jsonstring, (const char*) errstr);
-        yajl_free_error(hand, errstr);
-        virJSONValueFree(parser.head);
-        goto cleanup;
-    }
-
-    if (parser.nstate != 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("cannot parse json %s: unterminated string/map/array"),
-                       jsonstring);
-        virJSONValueFree(parser.head);
-    } else {
-        ret = parser.head;
-# ifndef WITH_YAJL2
-        /* Undo the array wrapping above */
-        tmp = ret;
-        ret = NULL;
-        if (virJSONValueArraySize(tmp) > 1)
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("cannot parse json %s: too many items present"),
-                           jsonstring);
-        else
-            ret = virJSONValueArraySteal(tmp, 0);
-        virJSONValueFree(tmp);
-# endif
-    }
-
- cleanup:
-    yajl_free(hand);
-
-    if (parser.nstate) {
-        size_t i;
-        for (i = 0; i < parser.nstate; i++)
-            VIR_FREE(parser.state[i].key);
-        VIR_FREE(parser.state);
-    }
-
-    VIR_DEBUG("result=%p", ret);
-
-    return ret;
-}
-
-
-static int
-virJSONValueToStringOne(virJSONValuePtr object,
-                        yajl_gen g)
-{
-    size_t i;
-
-    VIR_DEBUG("object=%p type=%d gen=%p", object, object->type, g);
-
-    switch (object->type) {
-    case VIR_JSON_TYPE_OBJECT:
-        if (yajl_gen_map_open(g) != yajl_gen_status_ok)
-            return -1;
-        for (i = 0; i < object->data.object.npairs; i++) {
-            if (yajl_gen_string(g,
-                                (unsigned char *)object->data.object.pairs[i].key,
-                                strlen(object->data.object.pairs[i].key))
-                                != yajl_gen_status_ok)
-                return -1;
-            if (virJSONValueToStringOne(object->data.object.pairs[i].value, g) < 0)
-                return -1;
-        }
-        if (yajl_gen_map_close(g) != yajl_gen_status_ok)
-            return -1;
-        break;
-    case VIR_JSON_TYPE_ARRAY:
-        if (yajl_gen_array_open(g) != yajl_gen_status_ok)
-            return -1;
-        for (i = 0; i < object->data.array.nvalues; i++) {
-            if (virJSONValueToStringOne(object->data.array.values[i], g) < 0)
-                return -1;
-        }
-        if (yajl_gen_array_close(g) != yajl_gen_status_ok)
-            return -1;
-        break;
-
-    case VIR_JSON_TYPE_STRING:
-        if (yajl_gen_string(g, (unsigned char *)object->data.string,
-                            strlen(object->data.string)) != yajl_gen_status_ok)
-            return -1;
-        break;
-
-    case VIR_JSON_TYPE_NUMBER:
-        if (yajl_gen_number(g, object->data.number,
-                            strlen(object->data.number)) != yajl_gen_status_ok)
-            return -1;
-        break;
-
-    case VIR_JSON_TYPE_BOOLEAN:
-        if (yajl_gen_bool(g, object->data.boolean) != yajl_gen_status_ok)
-            return -1;
-        break;
-
-    case VIR_JSON_TYPE_NULL:
-        if (yajl_gen_null(g) != yajl_gen_status_ok)
-            return -1;
-        break;
-
-    default:
-        return -1;
-    }
-
-    return 0;
-}
-
-
-char *
-virJSONValueToString(virJSONValuePtr object,
-                     bool pretty)
-{
-    yajl_gen g;
-    const unsigned char *str;
-    char *ret = NULL;
-    yajl_size_t len;
-# ifndef WITH_YAJL2
-    yajl_gen_config conf = { pretty ? 1 : 0, pretty ? "  " : " "};
-# endif
-
-    VIR_DEBUG("object=%p", object);
-
-# ifdef WITH_YAJL2
-    g = yajl_gen_alloc(NULL);
-    if (g) {
-        yajl_gen_config(g, yajl_gen_beautify, pretty ? 1 : 0);
-        yajl_gen_config(g, yajl_gen_indent_string, pretty ? "  " : " ");
-        yajl_gen_config(g, yajl_gen_validate_utf8, 1);
-    }
-# else
-    g = yajl_gen_alloc(&conf, NULL);
-# endif
-    if (!g) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Unable to create JSON formatter"));
-        goto cleanup;
-    }
-
-    if (virJSONValueToStringOne(object, g) < 0) {
-        virReportOOMError();
-        goto cleanup;
-    }
-
-    if (yajl_gen_get_buf(g, &str, &len) != yajl_gen_status_ok) {
-        virReportOOMError();
-        goto cleanup;
-    }
-
-    ignore_value(VIR_STRDUP(ret, (const char *)str));
-
- cleanup:
-    yajl_gen_free(g);
-
-    VIR_DEBUG("result=%s", NULLSTR(ret));
-
-    return ret;
-}
-
-
-#elif WITH_JANSSON
+#if WITH_JANSSON
 # include <jansson.h>
 
 static virJSONValuePtr
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 10/13] Remove functions using yajl
Posted by Peter Krempa, 1 week ago
On Fri, May 11, 2018 at 14:59:11 +0200, J�n Tomko wrote:
> We no longer support building WITH_YAJL, remove the dead code
> as well as the virJSONParser structures that are no longer used.
> 
> Signed-off-by: J�n Tomko <jtomko@redhat.com>
> ---
>  src/util/virjson.c | 530 +----------------------------------------------------
>  1 file changed, 1 insertion(+), 529 deletions(-)

ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 11/13] build: remove references to WITH_YAJL for SETUID_RPC_CLIENT
Posted by Ján Tomko, 1 week ago
We no longer allow building WITH_YAJL, remove the remaining
uses of the macro.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 config-post.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/config-post.h b/config-post.h
index cb2fc2b4ee..f561935bd7 100644
--- a/config-post.h
+++ b/config-post.h
@@ -45,8 +45,6 @@
 # undef WITH_SSH2
 # undef WITH_SYSTEMD_DAEMON
 # undef WITH_VIRTUALPORT
-# undef WITH_YAJL
-# undef WITH_YAJL2
 #endif
 
 /*
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 11/13] build: remove references to WITH_YAJL for SETUID_RPC_CLIENT
Posted by Peter Krempa, 1 week ago
On Fri, May 11, 2018 at 14:59:12 +0200, J�n Tomko wrote:
> We no longer allow building WITH_YAJL, remove the remaining
> uses of the macro.
> 
> Signed-off-by: J�n Tomko <jtomko@redhat.com>
> ---

ACK
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 12/13] build: switch --with-qemu default from yes to check
Posted by Ján Tomko, 1 week ago
Unless explicitly requested, enable the QEMU driver
only if the Jansson library is present.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 m4/virt-driver-qemu.m4 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
index 80e1d3ad46..ddb2834705 100644
--- a/m4/virt-driver-qemu.m4
+++ b/m4/virt-driver-qemu.m4
@@ -18,7 +18,7 @@ dnl <http://www.gnu.org/licenses/>.
 dnl
 
 AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
-  LIBVIRT_ARG_WITH_FEATURE([QEMU], [QEMU/KVM], [yes])
+  LIBVIRT_ARG_WITH_FEATURE([QEMU], [QEMU/KVM], [check])
   LIBVIRT_ARG_WITH([QEMU_USER], [username to run QEMU system instance as],
                    ['platform dependent'])
   LIBVIRT_ARG_WITH([QEMU_GROUP], [groupname to run QEMU system instance as],
@@ -26,6 +26,10 @@ AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
 ])
 
 AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [
+  AC_REQUIRE([LIBVIRT_CHECK_JANSSON])
+  if test "$with_qemu" = "check"; then
+    with_qemu=$with_jansson
+  fi
   if test "$with_qemu" = "yes" ; then
     AC_DEFINE_UNQUOTED([WITH_QEMU], 1, [whether QEMU driver is enabled])
   fi
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 12/13] build: switch --with-qemu default from yes to check
Posted by Peter Krempa, 1 week ago
On Fri, May 11, 2018 at 14:59:13 +0200, J�n Tomko wrote:
> Unless explicitly requested, enable the QEMU driver
> only if the Jansson library is present.
> 
> Signed-off-by: J�n Tomko <jtomko@redhat.com>
> ---
>  m4/virt-driver-qemu.m4 | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
> index 80e1d3ad46..ddb2834705 100644
> --- a/m4/virt-driver-qemu.m4
> +++ b/m4/virt-driver-qemu.m4
> @@ -18,7 +18,7 @@ dnl <http://www.gnu.org/licenses/>.
>  dnl
>  
>  AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
> -  LIBVIRT_ARG_WITH_FEATURE([QEMU], [QEMU/KVM], [yes])

Well if we've required it until now, I'd be inclined to continue doing
so.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 12/13] build: switch --with-qemu default from yes to check
Posted by Ján Tomko, 1 week ago
On Tue, May 15, 2018 at 03:14:33PM +0200, Peter Krempa wrote:
>On Fri, May 11, 2018 at 14:59:13 +0200, J�n Tomko wrote:
>> Unless explicitly requested, enable the QEMU driver
>> only if the Jansson library is present.
>>
>> Signed-off-by: J�n Tomko <jtomko@redhat.com>
>> ---
>>  m4/virt-driver-qemu.m4 | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
>> index 80e1d3ad46..ddb2834705 100644
>> --- a/m4/virt-driver-qemu.m4
>> +++ b/m4/virt-driver-qemu.m4
>> @@ -18,7 +18,7 @@ dnl <http://www.gnu.org/licenses/>.
>>  dnl
>>
>>  AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
>> -  LIBVIRT_ARG_WITH_FEATURE([QEMU], [QEMU/KVM], [yes])
>
>Well if we've required it until now, I'd be inclined to continue doing
>so.

The idea was to not introduce another dependency for default
'./autogen.sh', while still producing an error for someone requesting
--with-qemu without having a JSON library installed.

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 12/13] build: switch --with-qemu default from yes to check
Posted by Andrea Bolognani, 5 days ago
On Tue, 2018-05-15 at 16:00 +0200, Ján Tomko wrote:
> On Tue, May 15, 2018 at 03:14:33PM +0200, Peter Krempa wrote:
> > >  AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
> > > -  LIBVIRT_ARG_WITH_FEATURE([QEMU], [QEMU/KVM], [yes])
> > 
> > Well if we've required it until now, I'd be inclined to continue doing
> > so.
> 
> The idea was to not introduce another dependency for default
> './autogen.sh', while still producing an error for someone requesting
> --with-qemu without having a JSON library installed.

I agree with your reasoning, so ACK on my part.

We should do the same for the VirtualBox and ESX drivers, too.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 13/13] build: require Jansson if QEMU driver is enabled
Posted by Ján Tomko, 1 week ago
If the QEMU driver was requested, require Jansson, since we need to use
the JSON monitor to probe capabilities for all QEMU version supported
by libvirt.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 m4/virt-driver-qemu.m4 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
index ddb2834705..ac80f0cd48 100644
--- a/m4/virt-driver-qemu.m4
+++ b/m4/virt-driver-qemu.m4
@@ -27,6 +27,9 @@ AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
 
 AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [
   AC_REQUIRE([LIBVIRT_CHECK_JANSSON])
+  if test "$with_qemu:$with_jansson" = "yes:no"; then
+    AC_MSG_ERROR([Jansson >= 2.7 is required to build QEMU driver])
+  fi
   if test "$with_qemu" = "check"; then
     with_qemu=$with_jansson
   fi
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 13/13] build: require Jansson if QEMU driver is enabled
Posted by Peter Krempa, 1 week ago
On Fri, May 11, 2018 at 14:59:14 +0200, J�n Tomko wrote:
> If the QEMU driver was requested, require Jansson, since we need to use
> the JSON monitor to probe capabilities for all QEMU version supported
> by libvirt.
> 
> Signed-off-by: J�n Tomko <jtomko@redhat.com>
> ---
>  m4/virt-driver-qemu.m4 | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/m4/virt-driver-qemu.m4 b/m4/virt-driver-qemu.m4
> index ddb2834705..ac80f0cd48 100644
> --- a/m4/virt-driver-qemu.m4
> +++ b/m4/virt-driver-qemu.m4
> @@ -27,6 +27,9 @@ AC_DEFUN([LIBVIRT_DRIVER_ARG_QEMU], [
>  
>  AC_DEFUN([LIBVIRT_DRIVER_CHECK_QEMU], [
>    AC_REQUIRE([LIBVIRT_CHECK_JANSSON])
> +  if test "$with_qemu:$with_jansson" = "yes:no"; then
> +    AC_MSG_ERROR([Jansson >= 2.7 is required to build QEMU driver])
> +  fi

Don't forget to change this to 2.5. ACK I think that it does not make
sense to build a crippled version of the qemu driver.

>    if test "$with_qemu" = "check"; then
>      with_qemu=$with_jansson
>    fi
> -- 
> 2.16.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 14/13] Remove virJSONValueNewStringLen
Posted by Ján Tomko, 1 week ago
It is no longer used.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 src/libvirt_private.syms |  1 -
 src/util/virjson.c       | 22 ----------------------
 src/util/virjson.h       |  1 -
 3 files changed, 24 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b03596ad4d..248124e4ba 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2067,7 +2067,6 @@ virJSONValueNewNumberUint;
 virJSONValueNewNumberUlong;
 virJSONValueNewObject;
 virJSONValueNewString;
-virJSONValueNewStringLen;
 virJSONValueObjectAdd;
 virJSONValueObjectAddVArgs;
 virJSONValueObjectAppend;
diff --git a/src/util/virjson.c b/src/util/virjson.c
index ab3c63146d..0c3bc38b5c 100644
--- a/src/util/virjson.c
+++ b/src/util/virjson.c
@@ -421,28 +421,6 @@ virJSONValueNewString(const char *data)
 }
 
 
-virJSONValuePtr
-virJSONValueNewStringLen(const char *data,
-                         size_t length)
-{
-    virJSONValuePtr val;
-
-    if (!data)
-        return virJSONValueNewNull();
-
-    if (VIR_ALLOC(val) < 0)
-        return NULL;
-
-    val->type = VIR_JSON_TYPE_STRING;
-    if (VIR_STRNDUP(val->data.string, data, length) < 0) {
-        VIR_FREE(val);
-        return NULL;
-    }
-
-    return val;
-}
-
-
 static virJSONValuePtr
 virJSONValueNewNumber(const char *data)
 {
diff --git a/src/util/virjson.h b/src/util/virjson.h
index e4a82bdbc8..74e975f1a4 100644
--- a/src/util/virjson.h
+++ b/src/util/virjson.h
@@ -58,7 +58,6 @@ int virJSONValueObjectAddVArgs(virJSONValuePtr obj, va_list args)
 
 
 virJSONValuePtr virJSONValueNewString(const char *data);
-virJSONValuePtr virJSONValueNewStringLen(const char *data, size_t length);
 virJSONValuePtr virJSONValueNewNumberInt(int data);
 virJSONValuePtr virJSONValueNewNumberUint(unsigned int data);
 virJSONValuePtr virJSONValueNewNumberLong(long long data);
-- 
2.16.1

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