[XEN PATCH] libxl_nocpuid.c: fix build with json-c

Michael Young posted 1 patch 3 weeks, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/94ffdeb1-6826-4c3a-a9c1-3ccc8b129a61@durham.ac.uk
tools/libs/light/libxl_nocpuid.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
[XEN PATCH] libxl_nocpuid.c: fix build with json-c
Posted by Michael Young 3 weeks, 1 day ago
The build of xen-4.21.0 with json-c present and yajl not present
is failing with the error

libxl_nocpuid.c:43:1: error: unknown type name ‘yajl_gen_status’
    43 | yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
       | ^~~~~~~~~~~~~~~
libxl_nocpuid.c:43:50: error: unknown type name ‘yajl_gen’
    43 | yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
       |                                                  ^~~~~~~~
make[6]: *** 
[/builddir/build/BUILD/xen-4.21.0-build/xen-4.21.0/tools/libs/light/../../../tools/Rules.mk:178: libxl_nocpuid.o] Error 1

I tested it with the patch below based on libxl_nocpuid.c which did build.

Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
---
  tools/libs/light/libxl_nocpuid.c | 13 +++++++++++++
  1 file changed, 13 insertions(+)

diff --git a/tools/libs/light/libxl_nocpuid.c 
b/tools/libs/light/libxl_nocpuid.c
index 0630959e76..71ab49ed61 100644
--- a/tools/libs/light/libxl_nocpuid.c
+++ b/tools/libs/light/libxl_nocpuid.c
@@ -40,11 +40,24 @@ int libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t 
domid, bool restore,
      return 0;
  }

+#ifdef HAVE_LIBJSONC
+#ifndef _hidden
+#define _hidden
+#endif
+_hidden int libxl_cpuid_policy_list_gen_jso(json_object **jso_r,
+                                libxl_cpuid_policy_list *pcpuid)
+{
+    return 0;
+}
+#endif
+
+#if defined(HAVE_LIBYAJL)
  yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
                                  libxl_cpuid_policy_list *pcpuid)
  {
      return 0;
  }
+#endif

  int libxl__cpuid_policy_list_parse_json(libxl__gc *gc,
                                          const libxl__json_object *o,
-- 
2.51.1
Re: [XEN PATCH] libxl_nocpuid.c: fix build with json-c
Posted by Jan Beulich 2 weeks, 5 days ago
On 21.11.2025 22:09, Michael Young wrote:
> The build of xen-4.21.0 with json-c present and yajl not present
> is failing with the error
> 
> libxl_nocpuid.c:43:1: error: unknown type name ‘yajl_gen_status’
>     43 | yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
>        | ^~~~~~~~~~~~~~~
> libxl_nocpuid.c:43:50: error: unknown type name ‘yajl_gen’
>     43 | yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
>        |                                                  ^~~~~~~~
> make[6]: *** 
> [/builddir/build/BUILD/xen-4.21.0-build/xen-4.21.0/tools/libs/light/../../../tools/Rules.mk:178: libxl_nocpuid.o] Error 1
> 
> I tested it with the patch below based on libxl_nocpuid.c which did build.
> 
> Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
> ---
>   tools/libs/light/libxl_nocpuid.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/tools/libs/light/libxl_nocpuid.c 
> b/tools/libs/light/libxl_nocpuid.c
> index 0630959e76..71ab49ed61 100644
> --- a/tools/libs/light/libxl_nocpuid.c
> +++ b/tools/libs/light/libxl_nocpuid.c
> @@ -40,11 +40,24 @@ int libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t 
> domid, bool restore,
>       return 0;
>   }
> 
> +#ifdef HAVE_LIBJSONC
> +#ifndef _hidden
> +#define _hidden
> +#endif

Why would this be needed? libxl_internal provides a definition afaics.

> +_hidden int libxl_cpuid_policy_list_gen_jso(json_object **jso_r,

Nor should the attribute be needed here, as the function declaration ought
to be in scope.

> +                                libxl_cpuid_policy_list *pcpuid)
> +{
> +    return 0;
> +}
> +#endif
> +
> +#if defined(HAVE_LIBYAJL)
>   yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
>                                   libxl_cpuid_policy_list *pcpuid)
>   {
>       return 0;
>   }
> +#endif

Maybe unrelated to this build fix, I find it hard to believe that returning
0 (presumably meaning "success") here is appropriate without actually doing
anything. In particular for the new function you add, I think upon success
the caller can expect *jso_r to have got assigned a value. However, without
any commentary it's hard to tell whether there's some "agreement" that the
caller has to pre-set its variable (to, say, NULL).

Also why are the libxl_..._jso() all hidden, while their libxl_..._json()
counterparts aren't? And why would non-exported functions have their
declarations live in a non-private header?

Jan

Re: [XEN PATCH] libxl_nocpuid.c: fix build with json-c
Posted by Anthony PERARD 2 weeks, 2 days ago
On Mon, Nov 24, 2025 at 10:26:34AM +0100, Jan Beulich wrote:
> On 21.11.2025 22:09, Michael Young wrote:
> > diff --git a/tools/libs/light/libxl_nocpuid.c 
> > b/tools/libs/light/libxl_nocpuid.c
> > index 0630959e76..71ab49ed61 100644
> > --- a/tools/libs/light/libxl_nocpuid.c
> > +++ b/tools/libs/light/libxl_nocpuid.c
> > @@ -40,11 +40,24 @@ int libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t 
> > domid, bool restore,
> >       return 0;
> >   }
> > 
> > +#ifdef HAVE_LIBJSONC
> > +#ifndef _hidden
> > +#define _hidden
> > +#endif
> 
> Why would this be needed? libxl_internal provides a definition afaics.
> 
> > +_hidden int libxl_cpuid_policy_list_gen_jso(json_object **jso_r,
> 
> Nor should the attribute be needed here, as the function declaration ought
> to be in scope.

Yes, just mirroring the changes done to libxl_cpuid.c should be enough.

> > +                                libxl_cpuid_policy_list *pcpuid)
> > +{
> > +    return 0;
> > +}
> > +#endif
> > +
> > +#if defined(HAVE_LIBYAJL)
> >   yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
> >                                   libxl_cpuid_policy_list *pcpuid)
> >   {
> >       return 0;
> >   }
> > +#endif
> 
> Maybe unrelated to this build fix, I find it hard to believe that returning
> 0 (presumably meaning "success") here is appropriate without actually doing
> anything. In particular for the new function you add, I think upon success
> the caller can expect *jso_r to have got assigned a value. However, without
> any commentary it's hard to tell whether there's some "agreement" that the
> caller has to pre-set its variable (to, say, NULL).

For the YAJL function, this is correct, there's nothing to do with
`hand`. But for the json-c function, while the current caller does
initialise `*jso_r`, it would be best indeed to have the function set
`*jso_r` to NULL. (It's equivalent to return a NULL JSON object.)

> Also why are the libxl_..._jso() all hidden, while their libxl_..._json()
> counterparts aren't? And why would non-exported functions have their
> declarations live in a non-private header?

History, bad demission which expose libxl internals to libxl's users,
which make it impossible to use whatever json parsing/generating library that
libxl wants. The header `libxl_json.h` is only necessary if you want to
use the `*_json()` functions, and the only user (I hope) is `xl`.
I guess we could remove the installation of "libxl_json.h" header (and
"_libxl_types_json.h") when building libxl with json-c support.

Thanks,

-- 
Anthony PERARD