Re: [PATCH V6 2/3] xl: Add support to parse generic virtio device

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Tue, Nov 08, 2022 at 04:53:59PM +0530, Viresh Kumar wrote:
> diff --git a/tools/ocaml/libs/xl/genwrap.py b/tools/ocaml/libs/xl/genwrap.py
> index 7bf26bdcd831..b188104299b1 100644
> --- a/tools/ocaml/libs/xl/genwrap.py
> +++ b/tools/ocaml/libs/xl/genwrap.py
> @@ -36,6 +36,7 @@ DEVICE_LIST =      [ ("list",           ["ctx", "domid", "t 
> list"]),
>  functions = { # ( name , [type1,type2,....] )
>      "device_vfb":     DEVICE_FUNCTIONS,
>      "device_vkb":     DEVICE_FUNCTIONS,
> +    "device_virtio":     DEVICE_FUNCTIONS,
>      "device_disk":    DEVICE_FUNCTIONS + DEVICE_LIST +
>                        [ ("insert",         ["ctx", "t", "domid", 
> "?async:'a", "unit", "unit"]),
>                          ("of_vdev",        ["ctx", "domid", "string", "t"]),
> diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c 
> b/tools/ocaml/libs/xl/xenlight_stubs.c
> index 45b8af61c74a..8e54f95da7c7 100644
> --- a/tools/ocaml/libs/xl/xenlight_stubs.c
> +++ b/tools/ocaml/libs/xl/xenlight_stubs.c
> @@ -707,6 +707,7 @@ DEVICE_ADDREMOVE(disk)
>  DEVICE_ADDREMOVE(nic)
>  DEVICE_ADDREMOVE(vfb)
>  DEVICE_ADDREMOVE(vkb)
> +DEVICE_ADDREMOVE(virtio)
>  DEVICE_ADDREMOVE(pci)
>  _DEVICE_ADDREMOVE(disk, cdrom, insert)

I don't think these ocaml changes are necessary, because they don't
build. I'm guessing those adds the ability to hotplug devices which
virtio device don't have, so function for that are missing.

> diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> index 1b5381cef033..c6f35c069d2a 100644
> --- a/tools/xl/xl_parse.c
> +++ b/tools/xl/xl_parse.c
> @@ -2309,8 +2390,10 @@ void parse_config_data(const char *config_source,
>  
>      d_config->num_vfbs = 0;
>      d_config->num_vkbs = 0;
> +    d_config->num_virtios = 0;
>      d_config->vfbs = NULL;
>      d_config->vkbs = NULL;
> +    d_config->virtios = NULL;

These look a bit out of place, I think it would be fine to set
num_virtios and virtios just before calling parse_virtio_list(), as
array are usually initialised just before parsing the associated config
option in parse_config_data().

>      if (!xlu_cfg_get_list (config, "vfb", &cvfbs, 0, 0)) {
>          while ((buf = xlu_cfg_get_listitem (cvfbs, d_config->num_vfbs)) != 
> NULL) {
> @@ -2752,6 +2835,7 @@ void parse_config_data(const char *config_source,
>      }
>  
>      parse_vkb_list(config, d_config);
> +    parse_virtio_list(config, d_config);
>  
>      xlu_cfg_get_defbool(config, "xend_suspend_evtchn_compat",
>                          &c_info->xend_suspend_evtchn_compat, 0);

Thanks,

-- 
Anthony PERARD