Re: [PATCH 0/5] Testing libvirt XML -> libxl_domain_config conversion

[Jim Fehlig <jfehlig@suse.com>]

Daniel P. Berrange wrote:
> On Mon, Jun 02, 2014 at 01:41:58PM +0100, Ian Campbell wrote:
>   
>>>    I hacked around this, but it is a little dirty too. libvirt
>>>    already links to libyajl for the QEMU driver, but we  don't
>>>    really need the raw YAJL objects. It'd be nice to have a
>>>
>>>       char * libxl_domain_config_as_json(libxl_domain_config *p)
>>>
>>>    as a higher level wrapper around libxl_domain_config_gen_json
>>>    avoiding the pain of dealing with YAJL's different APIs.
>>>
>>>    Ian J mentioned to me that he thought there was already such a
>>>    method, but AFAICT, the only such code is in the 'xl' command
>>>    line tool itself (xl_cmdimpl.c - printf_info_one_json)
>>>       
>> That was me not Ian J I think.
>>     
>
> Opps, my bad, was talking to too many people to remember things clearly :-)
>  
>   
>> The function you need is libxl_domain_config_to_json (which is
>> autogenerated, so in _libxl_types.[hc]). I think the general
>> libxl_*_to_json support has been there since Xen 4.2, but IIRC
>> libxl_domain_config only got moved into the autogenerated/IDL layer in
>> 4.3.
>>     
>
> Ahhh, so I was looking in the wrong place - no wonder 'git grep' failed
> to find it :-)
>   

This patch becomes a bit smaller by using libxl_domain_config_to_json()
- see below diff.  It works with Xen 4.2-4.4, although the test fails on
all but 4.4 due to changes in the json noted earlier (e.g. additional
c_info fields added to the returned json).

Thanks for looking at this btw!  Great topic for the hackathon.  I was
groaning about the need for round-trip config testing last week while
reviewing some other patches, just before you posted this series!  Awesome!

Regards,
Jim


diff --git a/tests/libxlxml2jsontest.c b/tests/libxlxml2jsontest.c
index 02808b7..ed7c256 100644
--- a/tests/libxlxml2jsontest.c
+++ b/tests/libxlxml2jsontest.c
@@ -40,24 +40,11 @@
 # include "virmock.h"
 # include "testutilsxen.h"
 
-# ifdef WITH_YAJL2
-#  define HAVE_YAJL_V2
-# endif
-
-# include <yajl/yajl_gen.h>
-# include <libxl_json.h>
-
 # define VIR_FROM_THIS VIR_FROM_XEN
 
 static const char *abs_top_srcdir;
 static virCapsPtr xencaps;
 
-# ifdef WITH_YAJL2
-#  define yajl_size_t size_t
-# else
-#  define yajl_size_t unsigned int
-# endif
-
 static int testCompareXMLToJSONFiles(const char *xml,
                                      const char *cmdline)
 {
@@ -69,31 +56,10 @@ static int testCompareXMLToJSONFiles(const char *xml,
     libxl_domain_config config;
     xentoollog_logger *log = NULL;
     virDomainXMLOptionPtr xmlopt = NULL;
-    yajl_gen gen;
-# ifndef WITH_YAJL2
-    yajl_gen_config conf = { 1, "    " };
-# endif
-    const unsigned char *actualargv;
-    yajl_size_t actualargvlen;
+    char *actualargv = NULL;
 
     libxl_domain_config_init(&config);
 
-# ifdef WITH_YAJL2
-    gen = yajl_gen_alloc(NULL);
-    if (gen) {
-        yajl_gen_config(gen, yajl_gen_beautify, 1);
-        yajl_gen_config(gen, yajl_gen_indent_string, "    ");
-        yajl_gen_config(gen, yajl_gen_validate_utf8, 1);
-    }
-# else
-    gen = yajl_gen_alloc(&conf, NULL);
-# endif
-    if (!gen) {
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       "Unable to create JSON formatter");
-        goto cleanup;
-    }
-
     if (!(log = (xentoollog_logger
*)xtl_createlogger_stdiostream(stderr, XTL_DEBUG, 0)))
         goto cleanup;
 
@@ -114,24 +80,18 @@ static int testCompareXMLToJSONFiles(const char *xml,
     if (libxlBuildDomainConfig(gports, vmdef, ctx, &config) < 0)
         goto cleanup;
 
-    libxl_domain_config_gen_json(gen, &config);
-
-    if (yajl_gen_get_buf(gen, &actualargv, &actualargvlen) !=
yajl_gen_status_ok) {
-        virReportOOMError();
-        goto cleanup;
-    }
-
+    actualargv = libxl_domain_config_to_json(ctx, &config);
     virtTestLoadFile(cmdline, &expectargv);
 
-    if (STRNEQ(expectargv, (char *)actualargv)) {
-        virtTestDifference(stderr, expectargv, (char *)actualargv);
+    if (STRNEQ(expectargv, actualargv)) {
+        virtTestDifference(stderr, expectargv, actualargv);
         goto cleanup;
     }
 
     ret = 0;
 
  cleanup:
-    yajl_gen_free(gen);
+    VIR_FREE(actualargv);
     VIR_FREE(expectargv);
     virDomainDefFree(vmdef);
     virObjectUnref(gports);