Re: [PATCH V3 3/3] libxl: arm: Add grant_usage parameter for virtio devices

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Fri, Jun 02, 2023 at 11:19:09AM +0530, Viresh Kumar wrote:
> diff --git a/tools/libs/light/libxl_virtio.c b/tools/libs/light/libxl_virtio.c
> index f8a78e22d156..19d834984777 100644
> --- a/tools/libs/light/libxl_virtio.c
> +++ b/tools/libs/light/libxl_virtio.c
> @@ -48,11 +56,13 @@ static int libxl__set_xenstore_virtio(libxl__gc *gc, 
> uint32_t domid,
>                                        flexarray_t *ro_front)
>  {
>      const char *transport = 
> libxl_virtio_transport_to_string(virtio->transport);
> +    const char *grant_usage = libxl_defbool_to_string(virtio->grant_usage);
>  
>      flexarray_append_pair(back, "irq", GCSPRINTF("%u", virtio->irq));
>      flexarray_append_pair(back, "base", GCSPRINTF("%#"PRIx64, virtio->base));
>      flexarray_append_pair(back, "type", GCSPRINTF("%s", virtio->type));
>      flexarray_append_pair(back, "transport", GCSPRINTF("%s", transport));
> +    flexarray_append_pair(back, "grant_usage", GCSPRINTF("%s", grant_usage));

It doesn't seems like a good idea to write a string like "True" or
"False" in xenstore when a simple integer would work. Also I'm pretty
sure all other bool are written as "0" or "1", for false or true.
Could you change to write "0" or "1" instead of using
libxl_defbool_to_string() ?


Beside this, patch looks good to me.

Cheers,

-- 
Anthony PERARD