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(a)redhat.com&gt; --- 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

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(a)redhat.com&gt; --- 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

On Thu, Mar 29, 2018 at 01:09:56 +0200, Ján Tomko wrote: 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 03:16:55PM +0200, Ján Tomko wrote: 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 :|

On Thu, Mar 29, 2018 at 01:09:57 +0200, Ján Tomko wrote: 'val' will be leaked on failure here too 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. 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:13AM +0200, Peter Krempa wrote: 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. 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 :|