[libvirt] [PATCH 0/9] Add Jansson support
Prefer Jansson, but allow fallback/choice of yajl. Support for yajl can hopefully be dropped after we ditch CentOS 6 which has no Jansson. Ján Tomko (9): virmacmaptest: depend on yajl for 'empty' test virjsontest: Use a more stable floating point number for testing configure: rename LIBVIRT_*_YAJL to LIBVIRT_*_JSON Introduce WITH_JSON Introduce JSON_CFLAGS and JSON_LIBS virt-json.m4: generalize yajl dependency build: link setuid_rpc_client against JSON_LIBS virjson: add support for Jansson virt-json.m4: simplify QEMU check configure.ac | 10 +- m4/virt-json.m4 | 86 ++++++++++++++++ m4/virt-yajl.m4 | 55 ----------- src/Makefile.am | 9 +- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_driver.c | 4 +- src/util/Makefile.inc.am | 4 +- src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++ tests/Makefile.am | 12 +-- 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/qemuhelptest.c | 2 +- tests/qemuhotplugtest.c | 2 +- tests/qemumonitorjsontest.c | 2 +- tests/virjsontest.c | 2 +- tests/virmacmaptest.c | 3 + tests/virmocklibxl.c | 4 +- tests/virnetdaemontest.c | 2 +- tests/virstoragetest.c | 4 +- 23 files changed, 354 insertions(+), 96 deletions(-) create mode 100644 m4/virt-json.m4 delete mode 100644 m4/virt-yajl.m4 -- 2.16.1
While an empty array is formatted with two newlines with yajl, jansson does not put any newlines between the brackets. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virmacmaptest.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/virmacmaptest.c b/tests/virmacmaptest.c index 6e3e9984d..e0cf8f91a 100644 --- a/tests/virmacmaptest.c +++ b/tests/virmacmaptest.c @@ -205,8 +205,11 @@ mymain(void) DO_TEST_BASIC("simple2", "f24", "aa:bb:cc:dd:ee:ff", "a1:b2:c3:d4:e5:f6"); DO_TEST_BASIC("simple2", "f25", "00:11:22:33:44:55", "aa:bb:cc:00:11:22"); +#if WITH_YAJL + /* other JSON libraries might format an empty array differently */ DO_TEST_FLUSH_PROLOGUE; DO_TEST_FLUSH_EPILOGUE("empty"); +#endif DO_TEST_FLUSH_PROLOGUE; DO_TEST_FLUSH("f24", "aa:bb:cc:dd:ee:ff"); -- 2.16.1
On Thu, Mar 29, 2018 at 01:09:50 +0200, Ján Tomko wrote:
While an empty array is formatted with two newlines with yajl, jansson does not put any newlines between the brackets.
Signed-off-by: Ján Tomko <jtomko@redhat.com> ---
This should not be necessary if we replace the JSON library based JSON formatting with virBuffer as I've proposed.
We store all JSON numbers as strings. To allow using json libraries that store them in numeric types, use a more predictable and normalized value. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virjsontest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/virjsontest.c b/tests/virjsontest.c index 685df7276..22bfd028a 100644 --- a/tests/virjsontest.c +++ b/tests/virjsontest.c @@ -540,7 +540,7 @@ mymain(void) DO_TEST_PARSE("number without garbage", "[ 234545 ]", "[234545]"); DO_TEST_PARSE_FAIL("number with garbage", "[ 2345b45 ]"); - DO_TEST_PARSE("float without garbage", "[ 0.0314159e+100 ]", "[0.0314159e+100]"); + DO_TEST_PARSE("float without garbage", "[ 1.024e19 ]", "[1.024e19]"); DO_TEST_PARSE_FAIL("float with garbage", "[ 0.0314159ee+100 ]"); DO_TEST_PARSE("string", "[ \"The meaning of life\" ]", -- 2.16.1
On Thu, Mar 29, 2018 at 01:09:51 +0200, Ján Tomko wrote:
We store all JSON numbers as strings. To allow using json libraries that store them in numeric types, use a more predictable and normalized value.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/virjsontest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
ACK
Rename the top-level functions that deal with yajl to prepare for the possibility of a different JSON library. Introduce a separate JSON libraries section in the configure summary and rename the m4 file to virt-json.m4. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- configure.ac | 9 ++++++--- m4/{virt-yajl.m4 => virt-json.m4} | 8 ++++---- 2 files changed, 10 insertions(+), 7 deletions(-) rename m4/{virt-yajl.m4 => virt-json.m4} (94%) diff --git a/configure.ac b/configure.ac index 115eb6088..bef10c0fb 100644 --- a/configure.ac +++ b/configure.ac @@ -253,6 +253,7 @@ LIBVIRT_ARG_FUSE LIBVIRT_ARG_GLUSTER LIBVIRT_ARG_GNUTLS LIBVIRT_ARG_HAL +LIBVIRT_ARG_JSON LIBVIRT_ARG_LIBPCAP LIBVIRT_ARG_LIBSSH LIBVIRT_ARG_LIBXML @@ -272,7 +273,6 @@ LIBVIRT_ARG_SSH2 LIBVIRT_ARG_UDEV LIBVIRT_ARG_VIRTUALPORT LIBVIRT_ARG_WIRESHARK -LIBVIRT_ARG_YAJL LIBVIRT_CHECK_ACL LIBVIRT_CHECK_APPARMOR @@ -292,6 +292,7 @@ LIBVIRT_CHECK_FUSE LIBVIRT_CHECK_GLUSTER LIBVIRT_CHECK_GNUTLS LIBVIRT_CHECK_HAL +LIBVIRT_CHECK_JSON LIBVIRT_CHECK_LIBNL LIBVIRT_CHECK_LIBPARTED LIBVIRT_CHECK_LIBPCAP @@ -315,7 +316,6 @@ LIBVIRT_CHECK_UDEV LIBVIRT_CHECK_VIRTUALPORT LIBVIRT_CHECK_WIRESHARK LIBVIRT_CHECK_XDR -LIBVIRT_CHECK_YAJL AC_CHECK_SIZEOF([long]) @@ -1011,7 +1011,10 @@ LIBVIRT_RESULT_VIRTUALPORT LIBVIRT_RESULT_XDR LIBVIRT_RESULT_XEN LIBVIRT_RESULT_XENAPI -LIBVIRT_RESULT_YAJL +AC_MSG_NOTICE([]) +AC_MSG_NOTICE([JSON libraries]) +AC_MSG_NOTICE([]) +LIBVIRT_RESULT_JSON AC_MSG_NOTICE([]) AC_MSG_NOTICE([Windows]) AC_MSG_NOTICE([]) diff --git a/m4/virt-yajl.m4 b/m4/virt-json.m4 similarity index 94% rename from m4/virt-yajl.m4 rename to m4/virt-json.m4 index c4ea0102a..1179ff5bb 100644 --- a/m4/virt-yajl.m4 +++ b/m4/virt-json.m4 @@ -1,4 +1,4 @@ -dnl The libyajl.so library +dnl The JSON libraries dnl dnl Copyright (C) 2012-2013 Red Hat, Inc. dnl @@ -17,11 +17,11 @@ dnl License along with this library. If not, see dnl <http://www.gnu.org/licenses/>. dnl -AC_DEFUN([LIBVIRT_ARG_YAJL],[ +AC_DEFUN([LIBVIRT_ARG_JSON],[ LIBVIRT_ARG_WITH_FEATURE([YAJL], [yajl], [check]) ]) -AC_DEFUN([LIBVIRT_CHECK_YAJL],[ +AC_DEFUN([LIBVIRT_CHECK_JSON],[ 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 @@ -50,6 +50,6 @@ AC_DEFUN([LIBVIRT_CHECK_YAJL],[ [yajl_tree_parse], [yajl/yajl_common.h]) ]) -AC_DEFUN([LIBVIRT_RESULT_YAJL],[ +AC_DEFUN([LIBVIRT_RESULT_JSON],[ LIBVIRT_RESULT_LIB([YAJL]) ]) -- 2.16.1
A library-agnostic constant used by all the code that requires a working virjson implementation, but does not depent on yajl. The only remaining usage outside virjson.c is the empty array test in virmacmaptest.c. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- m4/virt-json.m4 | 7 +++++++ src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_driver.c | 4 ++-- 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/qemuhelptest.c | 2 +- tests/qemuhotplugtest.c | 2 +- tests/qemumonitorjsontest.c | 2 +- tests/virmocklibxl.c | 4 ++-- tests/virnetdaemontest.c | 2 +- tests/virstoragetest.c | 4 ++-- 16 files changed, 37 insertions(+), 30 deletions(-) diff --git a/m4/virt-json.m4 b/m4/virt-json.m4 index 1179ff5bb..2f2125b56 100644 --- a/m4/virt-json.m4 +++ b/m4/virt-json.m4 @@ -48,6 +48,13 @@ AC_DEFUN([LIBVIRT_CHECK_JSON],[ [yajl_parse_complete], [yajl/yajl_common.h], [YAJL2], [yajl], [yajl_tree_parse], [yajl/yajl_common.h]) + + AM_CONDITIONAL([WITH_JSON], + [test "$with_yajl" = "yes"]) + if test "$with_yajl" = "yes"; then + AC_DEFINE([WITH_JSON], [1], [whether a JSON library is available]) + fi + ]) AC_DEFUN([LIBVIRT_RESULT_JSON],[ diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e54dde69a..3d8b83a41 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1385,7 +1385,7 @@ virQEMUCapsComputeCmdFlags(const char *help, * incomplete to contemplate using. The 0.13.0 release * is good enough to use, even though it lacks one or * two features. */ -#if WITH_YAJL +#if WITH_JSON if (version >= 13000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_MONITOR_JSON); #else diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 7bcc4936d..ffda906f3 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2122,7 +2122,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) */ if ((!useAgent) || (ret < 0 && (acpiRequested || !flags))) { -#if WITH_YAJL +#if WITH_JSON if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MONITOR_JSON)) { if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NO_SHUTDOWN)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -2134,7 +2134,7 @@ qemuDomainReboot(virDomainPtr dom, unsigned int flags) virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("ACPI reboot is not supported without the JSON monitor")); goto endjob; -#if WITH_YAJL +#if WITH_JSON } #endif qemuDomainSetFakeReboot(driver, vm, isReboot); diff --git a/tests/Makefile.am b/tests/Makefile.am index 289ef35bd..249c533dc 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -333,9 +333,9 @@ if WITH_CIL test_programs += objectlocking endif WITH_CIL -if WITH_YAJL +if WITH_JSON test_programs += virjsontest -endif WITH_YAJL +endif WITH_JSON test_programs += \ networkxml2xmltest \ @@ -1233,15 +1233,15 @@ virdeterministichashmock_la_LIBADD = $(MOCKLIBS_LIBS) test_libraries += virdeterministichashmock.la -if WITH_YAJL +if WITH_JSON virmacmaptest_SOURCES = \ virmacmaptest.c testutils.h testutils.c virmacmaptest_LDADD = $(LDADDS) test_programs += virmacmaptest -else ! WITH_YAJL +else ! WITH_JSON EXTRA_DIST += virmacmaptest.c -endif ! WITH_YAJL +endif ! WITH_JSON virnetdevtest_SOURCES = \ virnetdevtest.c testutils.h testutils.c diff --git a/tests/cputest.c b/tests/cputest.c index 1e79edbef..ab1054266 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_JSON # 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_JSON static virQEMUDriver driver; #endif @@ -479,7 +479,7 @@ typedef enum { JSON_MODELS_REQUIRED, } cpuTestCPUIDJson; -#if WITH_QEMU && WITH_YAJL +#if WITH_QEMU && WITH_JSON 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_JSON */ static int cpuTestGetCPUModels(const struct data *data, @@ -834,7 +834,7 @@ cpuTestUpdateLive(const void *arg) } -#if WITH_QEMU && WITH_YAJL +#if WITH_QEMU && WITH_JSON 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_JSON 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_JSON # define DO_TEST_JSON(arch, host, json) \ do { \ if (json == JSON_MODELS) { \ @@ -1204,7 +1204,7 @@ mymain(void) DO_TEST_CPUID(VIR_ARCH_X86_64, "Xeon-X5460", JSON_NONE); cleanup: -#if WITH_QEMU && WITH_YAJL +#if WITH_QEMU && WITH_JSON qemuTestDriverFree(&driver); #endif diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c index 6eec4c752..cafdf8e02 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_JSON) && defined(HAVE_LIBXL_DOMAIN_CONFIG_FROM_JSON) # include "internal.h" # include "viralloc.h" @@ -210,4 +210,4 @@ int main(void) return EXIT_AM_SKIP; } -#endif /* WITH_LIBXL && WITH_YAJL && HAVE_LIBXL_DOMAIN_CONFIG_FROM_JSON */ +#endif /* WITH_LIBXL && WITH_JSON && HAVE_LIBXL_DOMAIN_CONFIG_FROM_JSON */ diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index f214eb461..0af46bc0d 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -907,7 +907,7 @@ mymain(void) { int ret = 0; -#if !WITH_YAJL +#if !WITH_JSON fputs("libvirt not compiled with yajl, skipping this test\n", stderr); return EXIT_AM_SKIP; #endif diff --git a/tests/qemucapabilitiestest.c b/tests/qemucapabilitiestest.c index 0d136cc8b..7091d6093 100644 --- a/tests/qemucapabilitiestest.c +++ b/tests/qemucapabilitiestest.c @@ -139,7 +139,7 @@ mymain(void) virQEMUDriver driver; testQemuData data; -#if !WITH_YAJL +#if !WITH_JSON fputs("libvirt not compiled with yajl, skipping this test\n", stderr); return EXIT_AM_SKIP; #endif diff --git a/tests/qemucaps2xmltest.c b/tests/qemucaps2xmltest.c index f4838f798..705b5c187 100644 --- a/tests/qemucaps2xmltest.c +++ b/tests/qemucaps2xmltest.c @@ -165,7 +165,7 @@ mymain(void) testQemuData data; -#if !WITH_YAJL +#if !WITH_JSON fputs("libvirt not compiled with yajl, skipping this test\n", stderr); return EXIT_AM_SKIP; #endif diff --git a/tests/qemucommandutiltest.c b/tests/qemucommandutiltest.c index eb155e7e1..106f83969 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_JSON fputs("libvirt not compiled with yajl, skipping this test\n", stderr); return EXIT_AM_SKIP; #endif diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 1336eeef5..c21f24cec 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -67,7 +67,7 @@ static int testHelpStrParsing(const void *data) goto cleanup; } -# ifndef WITH_YAJL +# ifndef WITH_JSON if (virQEMUCapsGet(info->flags, QEMU_CAPS_MONITOR_JSON)) virQEMUCapsSet(flags, QEMU_CAPS_MONITOR_JSON); # endif diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c index 85e53653e..18809adcb 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_JSON fputs("libvirt not compiled with yajl, skipping this test\n", stderr); return EXIT_AM_SKIP; #endif diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 0afdc8003..6829236b7 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2886,7 +2886,7 @@ mymain(void) struct testQAPISchemaData qapiData; char *metaschema = NULL; -#if !WITH_YAJL +#if !WITH_JSON fputs("libvirt not compiled with yajl, skipping this test\n", stderr); return EXIT_AM_SKIP; #endif diff --git a/tests/virmocklibxl.c b/tests/virmocklibxl.c index 973595659..8429cbb6b 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_JSON) # include "virmock.h" # include <sys/stat.h> # include <unistd.h> @@ -104,4 +104,4 @@ VIR_MOCK_IMPL_RET_ARGS(stat, int, return real_stat(path, sb); } -#endif /* WITH_LIBXL && WITH_YAJL */ +#endif /* WITH_LIBXL && WITH_JSON */ diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index ef869b16e..e42e349aa 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_JSON) struct testClientPriv { int magic; }; diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index b032d8b93..99cf3bd25 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_JSON 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_JSON */ cleanup: /* Final cleanup */ -- 2.16.1
Just copy YAJL_CFLAGS and YAJL_LIBS at this time. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- m4/virt-json.m4 | 2 ++ src/Makefile.am | 8 ++++---- src/util/Makefile.inc.am | 4 ++-- tests/Makefile.am | 2 +- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/m4/virt-json.m4 b/m4/virt-json.m4 index 2f2125b56..9e026931e 100644 --- a/m4/virt-json.m4 +++ b/m4/virt-json.m4 @@ -54,6 +54,8 @@ AC_DEFUN([LIBVIRT_CHECK_JSON],[ if test "$with_yajl" = "yes"; then AC_DEFINE([WITH_JSON], [1], [whether a JSON library is available]) fi + AC_SUBST([JSON_CFLAGS], [$YAJL_CFLAGS]) + AC_SUBST([JSON_LIBS], [$YAJL_LIBS]) ]) diff --git a/src/Makefile.am b/src/Makefile.am index 8b1e4c8a4..fd09bfd17 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -545,7 +545,7 @@ libvirt_admin_la_CFLAGS = \ libvirt_admin_la_CFLAGS += \ $(XDR_CFLAGS) \ $(CAPNG_CFLAGS) \ - $(YAJL_CFLAGS) \ + $(JSON_CFLAGS) \ $(SSH2_CFLAGS) \ $(SASL_CFLAGS) \ $(GNUTLS_CFLAGS) \ @@ -553,7 +553,7 @@ libvirt_admin_la_CFLAGS += \ libvirt_admin_la_LIBADD += \ $(CAPNG_LIBS) \ - $(YAJL_LIBS) \ + $(JSON_LIBS) \ $(DEVMAPPER_LIBS) \ $(LIBXML_LIBS) \ $(SSH2_LIBS) \ @@ -993,14 +993,14 @@ libvirt_nss_la_SOURCES = \ libvirt_nss_la_CFLAGS = \ -DLIBVIRT_NSS \ $(AM_CFLAGS) \ - $(YAJL_CFLAGS) \ + $(JSON_CFLAGS) \ $(NULL) libvirt_nss_la_LDFLAGS = \ $(AM_LDFLAGS) \ $(NULL) libvirt_nss_la_LIBADD = \ - $(YAJL_LIBS) \ + $(JSON_LIBS) \ $(NULL) endif WITH_NSS diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am index a3c3b711f..5adc24fee 100644 --- a/src/util/Makefile.inc.am +++ b/src/util/Makefile.inc.am @@ -245,7 +245,7 @@ libvirt_util_la_SOURCES = \ $(NULL) libvirt_util_la_CFLAGS = \ $(CAPNG_CFLAGS) \ - $(YAJL_CFLAGS) \ + $(JSON_CFLAGS) \ $(LIBNL_CFLAGS) \ $(AM_CFLAGS) \ $(AUDIT_CFLAGS) \ @@ -258,7 +258,7 @@ libvirt_util_la_CFLAGS = \ $(NULL) libvirt_util_la_LIBADD = \ $(CAPNG_LIBS) \ - $(YAJL_LIBS) \ + $(JSON_LIBS) \ $(LIBNL_LIBS) \ $(THREAD_LIBS) \ $(AUDIT_LIBS) \ diff --git a/tests/Makefile.am b/tests/Makefile.am index 249c533dc..ed0cd60f4 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -46,7 +46,7 @@ AM_CFLAGS = \ $(SASL_CFLAGS) \ $(SELINUX_CFLAGS) \ $(APPARMOR_CFLAGS) \ - $(YAJL_CFLAGS) \ + $(JSON_CFLAGS) \ $(COVERAGE_CFLAGS) \ $(XDR_CFLAGS) \ $(WARN_CFLAGS) -- 2.16.1
The QEMU driver needs a JSON library, not necessarily yajl. Check for yajl first, then explicitly check if we found a JSON library, to allow use of a different one. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- m4/virt-json.m4 | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/m4/virt-json.m4 b/m4/virt-json.m4 index 9e026931e..5e4bcc7c9 100644 --- a/m4/virt-json.m4 +++ b/m4/virt-json.m4 @@ -22,21 +22,21 @@ AC_DEFUN([LIBVIRT_ARG_JSON],[ ]) AC_DEFUN([LIBVIRT_CHECK_JSON],[ - dnl YAJL JSON library http://lloyd.github.com/yajl/ + need_json=no if test "$with_qemu:$with_yajl" = yes:check; then - dnl Some versions of qemu require the use of yajl; try to detect them + dnl Some versions of qemu require the use of JSON; 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 + need_json=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 ;; + [[1-9]].* | 0.15.* ) need_json=yes ;; 0.* | '' ) ;; *) AC_MSG_ERROR([Unexpected qemu version string]) ;; esac @@ -44,6 +44,7 @@ AC_DEFUN([LIBVIRT_CHECK_JSON],[ fi fi + dnl YAJL JSON library http://lloyd.github.com/yajl/ LIBVIRT_CHECK_LIB_ALT([YAJL], [yajl], [yajl_parse_complete], [yajl/yajl_common.h], [YAJL2], [yajl], @@ -53,6 +54,8 @@ AC_DEFUN([LIBVIRT_CHECK_JSON],[ [test "$with_yajl" = "yes"]) if test "$with_yajl" = "yes"; then AC_DEFINE([WITH_JSON], [1], [whether a JSON library is available]) + elif "$need_json" = "yes"; then + AC_MSG_ERROR([QEMU needs JSON but no library is available]) fi AC_SUBST([JSON_CFLAGS], [$YAJL_CFLAGS]) AC_SUBST([JSON_LIBS], [$YAJL_LIBS]) -- 2.16.1
I have no idea how it builds without and switching to Jansson exposes this. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Makefile.am b/src/Makefile.am index fd09bfd17..2d2805818 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -743,6 +743,7 @@ libvirt_setuid_rpc_client_la_LDFLAGS = \ $(AM_LDFLAGS) \ $(LIBXML_LIBS) \ $(SECDRIVER_LIBS) \ + $(JSON_LIBS) \ $(NULL) libvirt_setuid_rpc_client_la_CFLAGS = \ -DLIBVIRT_SETUID_RPC_CLIENT \ -- 2.16.1
On Thu, Mar 29, 2018 at 01:09:56 +0200, Ján Tomko wrote:
I have no idea how it builds without and switching to Jansson exposes this.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/Makefile.am | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/Makefile.am b/src/Makefile.am index fd09bfd17..2d2805818 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -743,6 +743,7 @@ libvirt_setuid_rpc_client_la_LDFLAGS = \ $(AM_LDFLAGS) \ $(LIBXML_LIBS) \ $(SECDRIVER_LIBS) \ + $(JSON_LIBS) \
Hmmm, isn't the setuid client supposed to be very minimal, and thus should not have dependencies on most libraries? Danpb (cc'd) might add some insight.
On Sat, Mar 31, 2018 at 11:05:38AM +0200, Peter Krempa wrote:
On Thu, Mar 29, 2018 at 01:09:56 +0200, Ján Tomko wrote:
I have no idea how it builds without and switching to Jansson exposes this.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/Makefile.am | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/Makefile.am b/src/Makefile.am index fd09bfd17..2d2805818 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -743,6 +743,7 @@ libvirt_setuid_rpc_client_la_LDFLAGS = \ $(AM_LDFLAGS) \ $(LIBXML_LIBS) \ $(SECDRIVER_LIBS) \ + $(JSON_LIBS) \
Hmmm, isn't the setuid client supposed to be very minimal, and thus should not have dependencies on most libraries?
Danpb (cc'd) might add some insight.
I see it now, I overlooked the use of WITH_YAJL in config-post.h: config-post.h:# undef WITH_YAJL config-post.h:# undef WITH_YAJL2 So even though the sources include virjson.c, it does not actually use it. Jano
On Sat, Mar 31, 2018 at 03:16:55PM +0200, Ján Tomko wrote:
On Sat, Mar 31, 2018 at 11:05:38AM +0200, Peter Krempa wrote:
On Thu, Mar 29, 2018 at 01:09:56 +0200, Ján Tomko wrote:
I have no idea how it builds without and switching to Jansson exposes this.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/Makefile.am | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/Makefile.am b/src/Makefile.am index fd09bfd17..2d2805818 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -743,6 +743,7 @@ libvirt_setuid_rpc_client_la_LDFLAGS = \ $(AM_LDFLAGS) \ $(LIBXML_LIBS) \ $(SECDRIVER_LIBS) \ + $(JSON_LIBS) \
Hmmm, isn't the setuid client supposed to be very minimal, and thus should not have dependencies on most libraries?
Danpb (cc'd) might add some insight.
I see it now, I overlooked the use of WITH_YAJL in config-post.h: config-post.h:# undef WITH_YAJL config-post.h:# undef WITH_YAJL2
So even though the sources include virjson.c, it does not actually use it.
Yep, exactly this was a bit of a nasty hack I had todo to cut down the deps. 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 :|
Check for the presence of Jansson library and prefer it to yajl if possible. The minimum required version is 2.7. Internally, virJSONValue still stores numbers as strings even though Jansson uses numeric variables for them. The configure script is particularly hideous, but will hopefully go away after we stop aiming to support compiling on CentOS 6. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- configure.ac | 1 + m4/virt-json.m4 | 55 +++++++++++--- src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 264 insertions(+), 11 deletions(-) diff --git a/configure.ac b/configure.ac index bef10c0fb..fe93209d7 100644 --- a/configure.ac +++ b/configure.ac @@ -987,6 +987,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-json.m4 b/m4/virt-json.m4 index 5e4bcc7c9..a5ae3edcd 100644 --- a/m4/virt-json.m4 +++ b/m4/virt-json.m4 @@ -18,12 +18,24 @@ dnl <http://www.gnu.org/licenses/>. dnl AC_DEFUN([LIBVIRT_ARG_JSON],[ + LIBVIRT_ARG_WITH_FEATURE([JANSSON], [jansson], [check]) LIBVIRT_ARG_WITH_FEATURE([YAJL], [yajl], [check]) ]) AC_DEFUN([LIBVIRT_CHECK_JSON],[ + if test "$with_yajl:$with_jansson" = "yes:yes"; then + AC_MSG_ERROR("Compiling with both jansson and yajl is unsupported") + fi + + if test "$with_yajl" = "yes"; then + with_jansson=no + elif test "$with_jansson" = "yes"; then + with_yajl=no + fi + need_json=no - if test "$with_qemu:$with_yajl" = yes:check; then + if test "$with_qemu:$with_yajl" = yes:check or + test "$with_qemu:$with_jansson" = yes:check; then dnl Some versions of qemu require the use of JSON; 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 @@ -44,24 +56,45 @@ AC_DEFUN([LIBVIRT_CHECK_JSON],[ fi fi - dnl YAJL JSON library http://lloyd.github.com/yajl/ - LIBVIRT_CHECK_LIB_ALT([YAJL], [yajl], - [yajl_parse_complete], [yajl/yajl_common.h], - [YAJL2], [yajl], - [yajl_tree_parse], [yajl/yajl_common.h]) + dnl Jansson http://www.digip.org/jansson/ + LIBVIRT_CHECK_PKG([JANSSON], [jansson], [2.7]) + + if test "$with_jansson" = "no"; then + dnl YAJL JSON library http://lloyd.github.com/yajl/ + LIBVIRT_CHECK_LIB_ALT([YAJL], [yajl], + [yajl_parse_complete], [yajl/yajl_common.h], + [YAJL2], [yajl], + [yajl_tree_parse], [yajl/yajl_common.h]) + else + AM_CONDITIONAL([WITH_YAJL], 0) + AM_CONDITIONAL([WITH_YAJL2], 0) + fi AM_CONDITIONAL([WITH_JSON], - [test "$with_yajl" = "yes"]) - if test "$with_yajl" = "yes"; then + [test "$with_yajl" = "yes" || test "$with_jansson" = "yes"]) + if test "$with_yajl" = "yes" || test "$with_jansson" = "yes"; then AC_DEFINE([WITH_JSON], [1], [whether a JSON library is available]) elif "$need_json" = "yes"; then AC_MSG_ERROR([QEMU needs JSON but no library is available]) fi - AC_SUBST([JSON_CFLAGS], [$YAJL_CFLAGS]) - AC_SUBST([JSON_LIBS], [$YAJL_LIBS]) + + if test "$with_jansson" = "yes"; then + AC_SUBST([JSON_CFLAGS], [$JANSSON_CFLAGS]) + AC_SUBST([JSON_LIBS], [$JANSSON_LIBS]) + else + AC_SUBST([JSON_CFLAGS], [$YAJL_CFLAGS]) + AC_SUBST([JSON_LIBS], [$YAJL_LIBS]) + fi ]) AC_DEFUN([LIBVIRT_RESULT_JSON],[ - LIBVIRT_RESULT_LIB([YAJL]) + if test "$with_jansson" = "yes"; then + msg = "Jansson" + elif test "$with_yajl" = "yes"; then + msg = "yajl" + else + msg = "none" + fi + LIBVIRT_RESULT([JSON library:], [$msg]) ]) diff --git a/src/util/virjson.c b/src/util/virjson.c index 6a02ddf0c..86cbd6eef 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1951,6 +1951,225 @@ 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) + 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) + 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_error_t error; + json_t *ret = NULL; + size_t i; + + switch (object->type) { + case VIR_JSON_TYPE_OBJECT: + ret = json_object(); + if (!ret) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to create JSON object: %s"), + error.text); + 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) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to set JSON object: %s"), + error.text); + goto error; + } + } + break; + + case VIR_JSON_TYPE_ARRAY: + ret = json_array(); + if (!ret) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to create JSON array: %s"), + error.text); + 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) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to append array value: %s"), + error.text); + 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", + _("that don't seem like no kind of number to me")); + 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; + } + if (!ret) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("error creating JSON value: %s"), + error.text); + } + return ret; + + error: + json_decref(ret); + return NULL; +} + + +char * +virJSONValueToString(virJSONValuePtr object, + bool pretty) +{ + size_t flags = JSON_ENCODE_ANY; + json_t *json; + json_error_t error; + 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) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to format JSON %d:%d: %s"), + error.line, error.column, error.text); + } + json_decref(json); + return str; +} + + #else virJSONValuePtr virJSONValueFromString(const char *jsonstring ATTRIBUTE_UNUSED) -- 2.16.1
On Thu, Mar 29, 2018 at 01:09:57 +0200, Ján Tomko wrote:
Check for the presence of Jansson library and prefer it to yajl if possible.
The minimum required version is 2.7.
Internally, virJSONValue still stores numbers as strings even though Jansson uses numeric variables for them.
The configure script is particularly hideous, but will hopefully go away after we stop aiming to support compiling on CentOS 6.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- configure.ac | 1 + m4/virt-json.m4 | 55 +++++++++++--- src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 264 insertions(+), 11 deletions(-)
diff --git a/m4/virt-json.m4 b/m4/virt-json.m4 index 5e4bcc7c9..a5ae3edcd 100644 --- a/m4/virt-json.m4 +++ b/m4/virt-json.m4
diff --git a/src/util/virjson.c b/src/util/virjson.c index 6a02ddf0c..86cbd6eef 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1951,6 +1951,225 @@ 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) + goto error;
'val' will be leaked on failure
+ } + + 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) + goto error;
here too
+ } + 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;
After mi privatization of struct _virJSONValue it should be simple enough to add the same differetiation to our code. Not sure whether that's worth though.
+ + 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_error_t error; + json_t *ret = NULL; + size_t i; + + switch (object->type) { + case VIR_JSON_TYPE_OBJECT: + ret = json_object(); + if (!ret) {
So this would be the second copy of a similar function. I propose that we replace the formatter which in this case copies everything from our structs into structs of janson to format it with a formatter which directly uses virBuffer to do so. https://www.redhat.com/archives/libvir-list/2018-March/msg01935.html This will allow us to have a single copy of the formatter and additionally it will not depend on the library.
On Sat, Mar 31, 2018 at 11:13:13 +0200, Peter Krempa wrote:
On Thu, Mar 29, 2018 at 01:09:57 +0200, Ján Tomko wrote:
Check for the presence of Jansson library and prefer it to yajl if possible.
The minimum required version is 2.7.
Internally, virJSONValue still stores numbers as strings even though Jansson uses numeric variables for them.
The configure script is particularly hideous, but will hopefully go away after we stop aiming to support compiling on CentOS 6.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- configure.ac | 1 + m4/virt-json.m4 | 55 +++++++++++--- src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 264 insertions(+), 11 deletions(-)
[...]
+ + case JSON_INTEGER: + ret = virJSONValueNewNumberLong(json_integer_value(json)); + break; + + case JSON_REAL: + ret = virJSONValueNewNumberDouble(json_real_value(json)); + break;
After mi privatization of struct _virJSONValue it should be simple enough to add the same differetiation to our code.
As a side-note, the qemu QAPI schema differentiates these as well, thus this change would make our schema validator more strict. Also it would possibly have some test fallout
On Sat, Mar 31, 2018 at 11:13:13AM +0200, Peter Krempa wrote:
On Thu, Mar 29, 2018 at 01:09:57 +0200, Ján Tomko wrote:
Check for the presence of Jansson library and prefer it to yajl if possible.
The minimum required version is 2.7.
Internally, virJSONValue still stores numbers as strings even though Jansson uses numeric variables for them.
The configure script is particularly hideous, but will hopefully go away after we stop aiming to support compiling on CentOS 6.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- configure.ac | 1 + m4/virt-json.m4 | 55 +++++++++++--- src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 264 insertions(+), 11 deletions(-)
diff --git a/m4/virt-json.m4 b/m4/virt-json.m4 index 5e4bcc7c9..a5ae3edcd 100644 --- a/m4/virt-json.m4 +++ b/m4/virt-json.m4
diff --git a/src/util/virjson.c b/src/util/virjson.c index 6a02ddf0c..86cbd6eef 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1951,6 +1951,225 @@ 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) + goto error;
'val' will be leaked on failure
+ } + + 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) + goto error;
here too
+ } + 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;
After mi privatization of struct _virJSONValue it should be simple enough to add the same differetiation to our code.
Not sure whether that's worth though.
+ + 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_error_t error; + json_t *ret = NULL; + size_t i; + + switch (object->type) { + case VIR_JSON_TYPE_OBJECT: + ret = json_object(); + if (!ret) {
So this would be the second copy of a similar function. I propose that we replace the formatter which in this case copies everything from our structs into structs of janson to format it with a formatter which directly uses virBuffer to do so.
Note the second copy will drop back down to 1 copy when we later drop YAJL support, so this is not a long term problem.
https://www.redhat.com/archives/libvir-list/2018-March/msg01935.html
This will allow us to have a single copy of the formatter and additionally it will not depend on the library.
That means that we are basically reinventing JSON formatting & escaping rules in our code. I don't think that would be a step forward. I wish we could someday get rid of our use of virBuffer for formatting XML too and rely on a XML library for formatting just as we do for JSON. 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 :|
On Tue, Apr 03, 2018 at 11:39:20 +0100, Daniel Berrange wrote:
On Sat, Mar 31, 2018 at 11:13:13AM +0200, Peter Krempa wrote:
On Thu, Mar 29, 2018 at 01:09:57 +0200, Ján Tomko wrote:
Check for the presence of Jansson library and prefer it to yajl if possible.
The minimum required version is 2.7.
Internally, virJSONValue still stores numbers as strings even though Jansson uses numeric variables for them.
The configure script is particularly hideous, but will hopefully go away after we stop aiming to support compiling on CentOS 6.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- configure.ac | 1 + m4/virt-json.m4 | 55 +++++++++++--- src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 264 insertions(+), 11 deletions(-)
[...]
+static json_t * +virJSONValueToJansson(virJSONValuePtr object) +{ + json_error_t error; + json_t *ret = NULL; + size_t i; + + switch (object->type) { + case VIR_JSON_TYPE_OBJECT: + ret = json_object(); + if (!ret) {
So this would be the second copy of a similar function. I propose that we replace the formatter which in this case copies everything from our structs into structs of janson to format it with a formatter which directly uses virBuffer to do so.
Note the second copy will drop back down to 1 copy when we later drop YAJL support, so this is not a long term problem.
https://www.redhat.com/archives/libvir-list/2018-March/msg01935.html
This will allow us to have a single copy of the formatter and additionally it will not depend on the library.
That means that we are basically reinventing JSON formatting & escaping rules in our code. I don't think that would be a step forward. I wisha
What I don't find being a step forwar is that when we are formatting the JSON with current approach we are basically translating our internal virJSONValue tree into a second copy of that tree represented by the janson data types, which is then used to do the formatting. Since the escaping rules are simple enough in case of JSON and are fully tested I don't really see a problem with that.
we could someday get rid of our use of virBuffer for formatting XML too and rely on a XML library for formatting just as we do for JSON.
So are we going to ditch libxml2 eventually?
On Tue, Apr 03, 2018 at 12:45:28PM +0200, Peter Krempa wrote:
On Tue, Apr 03, 2018 at 11:39:20 +0100, Daniel Berrange wrote:
On Sat, Mar 31, 2018 at 11:13:13AM +0200, Peter Krempa wrote:
On Thu, Mar 29, 2018 at 01:09:57 +0200, Ján Tomko wrote:
Check for the presence of Jansson library and prefer it to yajl if possible.
The minimum required version is 2.7.
Internally, virJSONValue still stores numbers as strings even though Jansson uses numeric variables for them.
The configure script is particularly hideous, but will hopefully go away after we stop aiming to support compiling on CentOS 6.
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- configure.ac | 1 + m4/virt-json.m4 | 55 +++++++++++--- src/util/virjson.c | 219 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 264 insertions(+), 11 deletions(-)
[...]
+static json_t * +virJSONValueToJansson(virJSONValuePtr object) +{ + json_error_t error; + json_t *ret = NULL; + size_t i; + + switch (object->type) { + case VIR_JSON_TYPE_OBJECT: + ret = json_object(); + if (!ret) {
So this would be the second copy of a similar function. I propose that we replace the formatter which in this case copies everything from our structs into structs of janson to format it with a formatter which directly uses virBuffer to do so.
Note the second copy will drop back down to 1 copy when we later drop YAJL support, so this is not a long term problem.
https://www.redhat.com/archives/libvir-list/2018-March/msg01935.html
This will allow us to have a single copy of the formatter and additionally it will not depend on the library.
That means that we are basically reinventing JSON formatting & escaping rules in our code. I don't think that would be a step forward. I wisha
What I don't find being a step forwar is that when we are formatting the JSON with current approach we are basically translating our internal virJSONValue tree into a second copy of that tree represented by the janson data types, which is then used to do the formatting.
We could potentially change virJSONValue so that it is more of a wrapper for the janson data type, so we don't have the translation step in terms of data storage.
Since the escaping rules are simple enough in case of JSON and are fully tested I don't really see a problem with that.
we could someday get rid of our use of virBuffer for formatting XML too and rely on a XML library for formatting just as we do for JSON.
So are we going to ditch libxml2 eventually?
I doubt it, I just didn't want to imply that we must use libxml for formatting. We could use libxml, but we could use an alternate if there was one better suited 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 :|
If we build with QEMU, it very probably needs JSON. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- m4/virt-json.m4 | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/m4/virt-json.m4 b/m4/virt-json.m4 index a5ae3edcd..d87cf56eb 100644 --- a/m4/virt-json.m4 +++ b/m4/virt-json.m4 @@ -36,24 +36,10 @@ AC_DEFUN([LIBVIRT_CHECK_JSON],[ need_json=no if test "$with_qemu:$with_yajl" = yes:check or test "$with_qemu:$with_jansson" = yes:check; then - dnl Some versions of qemu require the use of JSON; 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 - need_json=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.* ) need_json=yes ;; - 0.* | '' ) ;; - *) AC_MSG_ERROR([Unexpected qemu version string]) ;; - esac - fi - fi + dnl Nearly all supported QEMU versions require JSON. + dnl Assume we need it by default, but still let the user + dnl shoot it the foot. + need_json=yes fi dnl Jansson http://www.digip.org/jansson/ -- 2.16.1
On Thu, Mar 29, 2018 at 01:09:49AM +0200, Ján Tomko wrote:
Prefer Jansson, but allow fallback/choice of yajl.
Support for yajl can hopefully be dropped after we ditch CentOS 6 which has no Jansson.
Seems like we can revisit this patch series given our discusions about supported platforms. 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 :|
On Thu, Apr 12, 2018 at 12:35:21PM +0100, Daniel P. Berrangé wrote:
On Thu, Mar 29, 2018 at 01:09:49AM +0200, Ján Tomko wrote:
Prefer Jansson, but allow fallback/choice of yajl.
Support for yajl can hopefully be dropped after we ditch CentOS 6 which has no Jansson.
Seems like we can revisit this patch series given our discusions about supported platforms.
Sadly, yes. I don't see Jansson in the list of SLES 12 packages: https://www.suse.com/LinuxPackages/packageRouter.jsp?product=server&version=12&service_pack=&architecture=x86_64&package_name=index_all But json-c does seem to be everywhere. Jano
On Thu, Apr 12, 2018 at 01:47:06PM +0200, Ján Tomko wrote:
On Thu, Apr 12, 2018 at 12:35:21PM +0100, Daniel P. Berrangé wrote:
On Thu, Mar 29, 2018 at 01:09:49AM +0200, Ján Tomko wrote:
Prefer Jansson, but allow fallback/choice of yajl.
Support for yajl can hopefully be dropped after we ditch CentOS 6 which has no Jansson.
Seems like we can revisit this patch series given our discusions about supported platforms.
Sadly, yes.
I don't see Jansson in the list of SLES 12 packages: https://www.suse.com/LinuxPackages/packageRouter.jsp?product=server&version=12&service_pack=&architecture=x86_64&package_name=index_all
But json-c does seem to be everywhere.
json-c header files pollute the global namespace with this: #undef FALSE #define FALSE ((json_bool)0) #undef TRUE #define TRUE ((json_bool)1) #define hexdigit(x) (((x) <= '9') ? (x) - '0' : ((x) & 7) + 9) #define error_ptr(error) ((void*)error) #define error_description(error) (json_tokener_errors[error]) #define is_error(ptr) (ptr == NULL) so I find json-c really unappealing to use. 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 :|
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Peter Krempa